diff mbox series

c++/modules: Handle transitive reachability for deduction guides [PR116403]

Message ID 66c15222.620a0220.48a2.fa9e@mx.google.com
State New
Headers show
Series c++/modules: Handle transitive reachability for deduction guides [PR116403] | expand

Commit Message

Nathaniel Shead Aug. 18, 2024, 1:45 a.m. UTC
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

Currently we implement [temp.deduct.guide] p1 by forcing all deduction
guides to be considered as exported.  However this is not sufficient:
for transitive non-exported imports we will still hide the deduction
guide from name lookup, causing errors.

This patch instead adjusts name lookup to have a new ANY_REACHABLE flag
to allow for this case.  Currently this is only used by deduction guides
but there are some other circumstances where this may be useful in the
future (e.g. finding existing temploid friends).

	PR c++/116403

gcc/cp/ChangeLog:

	* pt.cc (deduction_guides_for): Use ANY_REACHABLE for lookup of
	deduction guides.
	* module.cc (depset::hash::add_deduction_guides): Likewise.
	(module_state::write_cluster): No longer override deduction
	guides as exported.
	* name-lookup.cc (name_lookup::search_namespace_only): Ignore
	visibility when LOOK_want::ANY_REACHABLE is specified.
	(check_module_override): Ignore visibility when checking for
	ambiguating deduction guides.
	* name-lookup.h (LOOK_want): New flag 'ANY_REACHABLE'.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/dguide-4_a.C: New test.
	* g++.dg/modules/dguide-4_b.C: New test.
	* g++.dg/modules/dguide-4_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                          |  7 +----
 gcc/cp/name-lookup.cc                     | 38 ++++++++++++++++++-----
 gcc/cp/name-lookup.h                      |  5 ++-
 gcc/cp/pt.cc                              |  3 +-
 gcc/testsuite/g++.dg/modules/dguide-4_a.C | 18 +++++++++++
 gcc/testsuite/g++.dg/modules/dguide-4_b.C |  9 ++++++
 gcc/testsuite/g++.dg/modules/dguide-4_c.C | 15 +++++++++
 7 files changed, 79 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-4_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-4_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-4_c.C

Comments

Jason Merrill Aug. 19, 2024, 4:59 p.m. UTC | #1
On 8/17/24 9:45 PM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

OK.

> -- >8 --
> 
> Currently we implement [temp.deduct.guide] p1 by forcing all deduction
> guides to be considered as exported.  However this is not sufficient:
> for transitive non-exported imports we will still hide the deduction
> guide from name lookup, causing errors.
> 
> This patch instead adjusts name lookup to have a new ANY_REACHABLE flag
> to allow for this case.  Currently this is only used by deduction guides
> but there are some other circumstances where this may be useful in the
> future (e.g. finding existing temploid friends).
> 
> 	PR c++/116403
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (deduction_guides_for): Use ANY_REACHABLE for lookup of
> 	deduction guides.
> 	* module.cc (depset::hash::add_deduction_guides): Likewise.
> 	(module_state::write_cluster): No longer override deduction
> 	guides as exported.
> 	* name-lookup.cc (name_lookup::search_namespace_only): Ignore
> 	visibility when LOOK_want::ANY_REACHABLE is specified.
> 	(check_module_override): Ignore visibility when checking for
> 	ambiguating deduction guides.
> 	* name-lookup.h (LOOK_want): New flag 'ANY_REACHABLE'.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/dguide-4_a.C: New test.
> 	* g++.dg/modules/dguide-4_b.C: New test.
> 	* g++.dg/modules/dguide-4_c.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                          |  7 +----
>   gcc/cp/name-lookup.cc                     | 38 ++++++++++++++++++-----
>   gcc/cp/name-lookup.h                      |  5 ++-
>   gcc/cp/pt.cc                              |  3 +-
>   gcc/testsuite/g++.dg/modules/dguide-4_a.C | 18 +++++++++++
>   gcc/testsuite/g++.dg/modules/dguide-4_b.C |  9 ++++++
>   gcc/testsuite/g++.dg/modules/dguide-4_c.C | 15 +++++++++
>   7 files changed, 79 insertions(+), 16 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/dguide-4_a.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/dguide-4_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/dguide-4_c.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index f4d137b13a1..6eb4cbf2911 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -13640,7 +13640,7 @@ depset::hash::add_deduction_guides (tree decl)
>     if (find_binding (ns, name))
>       return;
>   
> -  tree guides = lookup_qualified_name (ns, name, LOOK_want::NORMAL,
> +  tree guides = lookup_qualified_name (ns, name, LOOK_want::ANY_REACHABLE,
>   				       /*complain=*/false);
>     if (guides == error_mark_node)
>       return;
> @@ -15223,11 +15223,6 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
>   		      flags |= cbf_hidden;
>   		    else if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (bound)))
>   		      flags |= cbf_export;
> -		    else if (deduction_guide_p (bound))
> -		      /* Deduction guides are always exported so that they are
> -			 visible to name lookup whenever their class template
> -			 is reachable.  */
> -		      flags |= cbf_export;
>   		  }
>   
>   		gcc_checking_assert (DECL_P (bound));
> diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
> index 872f1af0b2e..70ad4cbf3b5 100644
> --- a/gcc/cp/name-lookup.cc
> +++ b/gcc/cp/name-lookup.cc
> @@ -916,7 +916,8 @@ name_lookup::search_namespace_only (tree scope)
>   		if (unsigned base = cluster->indices[jx].base)
>   		  if (unsigned span = cluster->indices[jx].span)
>   		    do
> -		      if (bitmap_bit_p (imports, base))
> +		      if (bool (want & LOOK_want::ANY_REACHABLE)
> +			  || bitmap_bit_p (imports, base))
>   			goto found;
>   		    while (++base, --span);
>   		continue;
> @@ -960,9 +961,17 @@ name_lookup::search_namespace_only (tree scope)
>   			dup_detect |= dup;
>   		      }
>   
> -		    if (STAT_TYPE_VISIBLE_P (bind))
> -		      type = STAT_TYPE (bind);
> -		    bind = STAT_VISIBLE (bind);
> +		    if (bool (want & LOOK_want::ANY_REACHABLE))
> +		      {
> +			type = STAT_TYPE (bind);
> +			bind = STAT_DECL (bind);
> +		      }
> +		    else
> +		      {
> +			if (STAT_TYPE_VISIBLE_P (bind))
> +			  type = STAT_TYPE (bind);
> +			bind = STAT_VISIBLE (bind);
> +		      }
>   		  }
>   
>   		/* And process it.  */
> @@ -3761,6 +3770,10 @@ check_module_override (tree decl, tree mvec, bool hiding,
>     tree nontmpl = STRIP_TEMPLATE (decl);
>     bool attached = DECL_LANG_SPECIFIC (nontmpl) && DECL_MODULE_ATTACH_P (nontmpl);
>   
> +  /* For deduction guides we don't do normal name lookup, but rather consider
> +     any reachable declaration, so we should check for overriding here too.  */
> +  bool any_reachable = deduction_guide_p (decl);
> +
>     if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
>       {
>         cluster++;
> @@ -3775,7 +3788,8 @@ check_module_override (tree decl, tree mvec, bool hiding,
>   	  continue;
>   	if (!cluster->indices[jx].base)
>   	  continue;
> -	if (!bitmap_bit_p (imports, cluster->indices[jx].base))
> +	if (!any_reachable
> +	    && !bitmap_bit_p (imports, cluster->indices[jx].base))
>   	  continue;
>   	/* Is it loaded? */
>   	if (cluster->slots[jx].is_lazy ())
> @@ -3795,9 +3809,17 @@ check_module_override (tree decl, tree mvec, bool hiding,
>   	    /* If there was a matching STAT_TYPE here then xref_tag
>   	       should have found it, but we need to check anyway because
>   	       a conflicting using-declaration may exist.  */
> -	    if (STAT_TYPE_VISIBLE_P (bind))
> -	      type = STAT_TYPE (bind);
> -	    bind = STAT_VISIBLE (bind);
> +	    if (any_reachable)
> +	      {
> +		type = STAT_TYPE (bind);
> +		bind = STAT_DECL (bind);
> +	      }
> +	    else
> +	      {
> +		if (STAT_TYPE_VISIBLE_P (bind))
> +		  type = STAT_TYPE (bind);
> +		bind = STAT_VISIBLE (bind);
> +	      }
>   	  }
>   
>   	if (type)
> diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
> index 7c4193444dd..f39f5309150 100644
> --- a/gcc/cp/name-lookup.h
> +++ b/gcc/cp/name-lookup.h
> @@ -385,9 +385,12 @@ enum class LOOK_want
>     TYPE = 1 << 1,  /* We only want TYPE_DECLS.  */
>     NAMESPACE = 1 << 2,  /* We only want NAMESPACE_DECLS.  */
>   
> -  HIDDEN_FRIEND = 1 << 3, /* See hidden friends.  */
> +  HIDDEN_FRIEND = 1 << 3,  /* See hidden friends.  */
>     HIDDEN_LAMBDA = 1 << 4,  /* See lambda-ignored entities.  */
>   
> +  ANY_REACHABLE = 1 << 5,  /* Include reachable module declarations not
> +			      normally visible to name lookup.  */
> +
>     TYPE_NAMESPACE = TYPE | NAMESPACE,  /* Either NAMESPACE or TYPE.  */
>   };
>   constexpr LOOK_want operator| (LOOK_want a, LOOK_want b)
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 8725a5eeb3f..5c6f4e7f7ff 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -30731,7 +30731,8 @@ deduction_guides_for (tree tmpl, bool &any_dguides_p, tsubst_flags_t complain)
>       {
>         guides = lookup_qualified_name (CP_DECL_CONTEXT (tmpl),
>   				      dguide_name (tmpl),
> -				      LOOK_want::NORMAL, /*complain*/false);
> +				      LOOK_want::ANY_REACHABLE,
> +				      /*complain=*/false);
>         if (guides == error_mark_node)
>   	guides = NULL_TREE;
>         else
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-4_a.C b/gcc/testsuite/g++.dg/modules/dguide-4_a.C
> new file mode 100644
> index 00000000000..5a9da44cdd6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-4_a.C
> @@ -0,0 +1,18 @@
> +// PR c++/116403
> +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> +// { dg-module-cmi A }
> +
> +module;
> +
> +template <typename T> struct GMF {
> +  GMF(int);
> +};
> +GMF(int) -> GMF<int>;
> +
> +export module A;
> +export using ::GMF;
> +
> +export template <typename T> struct Attached {
> +  Attached(int);
> +};
> +Attached(int) -> Attached<int>;
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-4_b.C b/gcc/testsuite/g++.dg/modules/dguide-4_b.C
> new file mode 100644
> index 00000000000..9276ffbb3c8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-4_b.C
> @@ -0,0 +1,9 @@
> +// PR c++/116403
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi B }
> +
> +export module B;
> +import A;  // not exported
> +
> +export using ::GMF;
> +export using ::Attached;
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-4_c.C b/gcc/testsuite/g++.dg/modules/dguide-4_c.C
> new file mode 100644
> index 00000000000..898ffa6ac81
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-4_c.C
> @@ -0,0 +1,15 @@
> +// PR c++/116403
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import B;
> +
> +int main() {
> +  GMF gmf(123);
> +  GMF<int> test_gmf = gmf;
> +
> +  Attached attached(456);
> +  Attached<int> test_attached = attached;
> +}
> +
> +GMF(int) -> GMF<double>;  // { dg-error "ambiguating" }
> +Attached(int) -> Attached<double>;  // { dg-error "ambiguating" }
diff mbox series

Patch

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f4d137b13a1..6eb4cbf2911 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13640,7 +13640,7 @@  depset::hash::add_deduction_guides (tree decl)
   if (find_binding (ns, name))
     return;
 
-  tree guides = lookup_qualified_name (ns, name, LOOK_want::NORMAL,
+  tree guides = lookup_qualified_name (ns, name, LOOK_want::ANY_REACHABLE,
 				       /*complain=*/false);
   if (guides == error_mark_node)
     return;
@@ -15223,11 +15223,6 @@  module_state::write_cluster (elf_out *to, depset *scc[], unsigned size,
 		      flags |= cbf_hidden;
 		    else if (DECL_MODULE_EXPORT_P (STRIP_TEMPLATE (bound)))
 		      flags |= cbf_export;
-		    else if (deduction_guide_p (bound))
-		      /* Deduction guides are always exported so that they are
-			 visible to name lookup whenever their class template
-			 is reachable.  */
-		      flags |= cbf_export;
 		  }
 
 		gcc_checking_assert (DECL_P (bound));
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 872f1af0b2e..70ad4cbf3b5 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -916,7 +916,8 @@  name_lookup::search_namespace_only (tree scope)
 		if (unsigned base = cluster->indices[jx].base)
 		  if (unsigned span = cluster->indices[jx].span)
 		    do
-		      if (bitmap_bit_p (imports, base))
+		      if (bool (want & LOOK_want::ANY_REACHABLE)
+			  || bitmap_bit_p (imports, base))
 			goto found;
 		    while (++base, --span);
 		continue;
@@ -960,9 +961,17 @@  name_lookup::search_namespace_only (tree scope)
 			dup_detect |= dup;
 		      }
 
-		    if (STAT_TYPE_VISIBLE_P (bind))
-		      type = STAT_TYPE (bind);
-		    bind = STAT_VISIBLE (bind);
+		    if (bool (want & LOOK_want::ANY_REACHABLE))
+		      {
+			type = STAT_TYPE (bind);
+			bind = STAT_DECL (bind);
+		      }
+		    else
+		      {
+			if (STAT_TYPE_VISIBLE_P (bind))
+			  type = STAT_TYPE (bind);
+			bind = STAT_VISIBLE (bind);
+		      }
 		  }
 
 		/* And process it.  */
@@ -3761,6 +3770,10 @@  check_module_override (tree decl, tree mvec, bool hiding,
   tree nontmpl = STRIP_TEMPLATE (decl);
   bool attached = DECL_LANG_SPECIFIC (nontmpl) && DECL_MODULE_ATTACH_P (nontmpl);
 
+  /* For deduction guides we don't do normal name lookup, but rather consider
+     any reachable declaration, so we should check for overriding here too.  */
+  bool any_reachable = deduction_guide_p (decl);
+
   if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
     {
       cluster++;
@@ -3775,7 +3788,8 @@  check_module_override (tree decl, tree mvec, bool hiding,
 	  continue;
 	if (!cluster->indices[jx].base)
 	  continue;
-	if (!bitmap_bit_p (imports, cluster->indices[jx].base))
+	if (!any_reachable
+	    && !bitmap_bit_p (imports, cluster->indices[jx].base))
 	  continue;
 	/* Is it loaded? */
 	if (cluster->slots[jx].is_lazy ())
@@ -3795,9 +3809,17 @@  check_module_override (tree decl, tree mvec, bool hiding,
 	    /* If there was a matching STAT_TYPE here then xref_tag
 	       should have found it, but we need to check anyway because
 	       a conflicting using-declaration may exist.  */
-	    if (STAT_TYPE_VISIBLE_P (bind))
-	      type = STAT_TYPE (bind);
-	    bind = STAT_VISIBLE (bind);
+	    if (any_reachable)
+	      {
+		type = STAT_TYPE (bind);
+		bind = STAT_DECL (bind);
+	      }
+	    else
+	      {
+		if (STAT_TYPE_VISIBLE_P (bind))
+		  type = STAT_TYPE (bind);
+		bind = STAT_VISIBLE (bind);
+	      }
 	  }
 
 	if (type)
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 7c4193444dd..f39f5309150 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -385,9 +385,12 @@  enum class LOOK_want
   TYPE = 1 << 1,  /* We only want TYPE_DECLS.  */
   NAMESPACE = 1 << 2,  /* We only want NAMESPACE_DECLS.  */
 
-  HIDDEN_FRIEND = 1 << 3, /* See hidden friends.  */
+  HIDDEN_FRIEND = 1 << 3,  /* See hidden friends.  */
   HIDDEN_LAMBDA = 1 << 4,  /* See lambda-ignored entities.  */
 
+  ANY_REACHABLE = 1 << 5,  /* Include reachable module declarations not
+			      normally visible to name lookup.  */
+
   TYPE_NAMESPACE = TYPE | NAMESPACE,  /* Either NAMESPACE or TYPE.  */
 };
 constexpr LOOK_want operator| (LOOK_want a, LOOK_want b)
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 8725a5eeb3f..5c6f4e7f7ff 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -30731,7 +30731,8 @@  deduction_guides_for (tree tmpl, bool &any_dguides_p, tsubst_flags_t complain)
     {
       guides = lookup_qualified_name (CP_DECL_CONTEXT (tmpl),
 				      dguide_name (tmpl),
-				      LOOK_want::NORMAL, /*complain*/false);
+				      LOOK_want::ANY_REACHABLE,
+				      /*complain=*/false);
       if (guides == error_mark_node)
 	guides = NULL_TREE;
       else
diff --git a/gcc/testsuite/g++.dg/modules/dguide-4_a.C b/gcc/testsuite/g++.dg/modules/dguide-4_a.C
new file mode 100644
index 00000000000..5a9da44cdd6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-4_a.C
@@ -0,0 +1,18 @@ 
+// PR c++/116403
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi A }
+
+module;
+
+template <typename T> struct GMF {
+  GMF(int);
+};
+GMF(int) -> GMF<int>;
+
+export module A;
+export using ::GMF;
+
+export template <typename T> struct Attached {
+  Attached(int);
+};
+Attached(int) -> Attached<int>;
diff --git a/gcc/testsuite/g++.dg/modules/dguide-4_b.C b/gcc/testsuite/g++.dg/modules/dguide-4_b.C
new file mode 100644
index 00000000000..9276ffbb3c8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-4_b.C
@@ -0,0 +1,9 @@ 
+// PR c++/116403
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi B }
+
+export module B;
+import A;  // not exported
+
+export using ::GMF;
+export using ::Attached;
diff --git a/gcc/testsuite/g++.dg/modules/dguide-4_c.C b/gcc/testsuite/g++.dg/modules/dguide-4_c.C
new file mode 100644
index 00000000000..898ffa6ac81
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-4_c.C
@@ -0,0 +1,15 @@ 
+// PR c++/116403
+// { dg-additional-options "-fmodules-ts" }
+
+import B;
+
+int main() {
+  GMF gmf(123);
+  GMF<int> test_gmf = gmf;
+
+  Attached attached(456);
+  Attached<int> test_attached = attached;
+}
+
+GMF(int) -> GMF<double>;  // { dg-error "ambiguating" }
+Attached(int) -> Attached<double>;  // { dg-error "ambiguating" }