public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231]
@ 2024-06-16  2:29 Nathaniel Shead
  2024-06-16  5:41 ` Nathaniel Shead
  2024-07-23 20:17 ` Jason Merrill
  0 siblings, 2 replies; 9+ messages in thread
From: Nathaniel Shead @ 2024-06-16  2:29 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell

Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

This probably isn't the most efficient approach, since we need to do
name lookup to find deduction guides for a type which will also
potentially do a bunch of pointless lazy loading from imported modules,
but I wasn't able to work out a better approach without completely
reworking how deduction guides are stored and represented.  This way at
least is correct (I believe), and we can maybe revisit this in the
future.

-- >8 --

Deduction guides are represented as 'normal' functions currently, and
have no special handling in modules.  However, this causes some issues;
by [temp.deduct.guide] a deduction guide is not found by normal name
lookup and instead all reachable deduction guides for a class template
should be considered, but this does not happen currently.

To solve this, this patch ensures that all deduction guides are
considered exported to ensure that they are always visible to importers
if they are reachable.  Another alternative here would be to add a new
kind of "all reachable" flag to name lookup, but that is complicated by
some difficulties in handling GM entities; this may be a better way to
go if more kinds of entities end up needing this handling, however.

Another issue here is that because deduction guides are "unrelated"
functions, they will usually get discarded from the GMF, so this patch
ensures that when finding dependencies, GMF deduction guides will also
have bindings created.  We do this in find_dependencies so that we don't
unnecessarily create bindings for GMF deduction guides that are never
reached; for consistency we do this for *all* deduction guides, not just
GM ones.

Finally, when merging deduction guides from multiple modules, the name
lookup code may now return two-dimensional overload sets, so update
callers to match.

As a small drive-by improvement this patch also updates the error pretty
printing code to add a space before the '->' when printing a deduction
guide, so we get 'S(int) -> S<int>' instead of 'S(int)-> S<int>'.

	PR c++/115231

gcc/cp/ChangeLog:

	* error.cc (dump_function_decl): Add a space before '->' when
	printing deduction guides.
	* module.cc (depset::hash::add_binding_entity): Skip deduction
	guides here.
	(depset::hash::add_deduction_guides): New.
	(depset::hash::find_dependencies): Add deduction guide
	dependencies for a class template.
	(module_state::write_cluster): Always consider deduction guides
	as exported.
	* pt.cc (deduction_guides_for): Use 'lkp_iterator' instead of
	'ovl_iterator'.

gcc/testsuite/ChangeLog:

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

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/error.cc                           |  1 +
 gcc/cp/module.cc                          | 60 +++++++++++++++++++++++
 gcc/cp/pt.cc                              |  2 +-
 gcc/testsuite/g++.dg/modules/dguide-1_a.C | 44 +++++++++++++++++
 gcc/testsuite/g++.dg/modules/dguide-1_b.C | 20 ++++++++
 gcc/testsuite/g++.dg/modules/dguide-2_a.C | 24 +++++++++
 gcc/testsuite/g++.dg/modules/dguide-2_b.C | 19 +++++++
 gcc/testsuite/g++.dg/modules/dguide-3_a.C | 10 ++++
 gcc/testsuite/g++.dg/modules/dguide-3_b.C | 10 ++++
 gcc/testsuite/g++.dg/modules/dguide-3_c.C | 22 +++++++++
 10 files changed, 211 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_c.C

diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index 171a352c85f..2fb5084320e 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -1866,6 +1866,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags)
 	dump_type_suffix (pp, ret, flags);
       else if (deduction_guide_p (t))
 	{
+	  pp->set_padding (pp_before);
 	  pp_cxx_ws_string (pp, "->");
 	  dump_type (pp, TREE_TYPE (TREE_TYPE (t)), flags);
 	}
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 6d6044af199..a2c9e151fd5 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2589,6 +2589,9 @@ public:
     void add_partial_entities (vec<tree, va_gc> *);
     void add_class_entities (vec<tree, va_gc> *);
 
+  private:
+    void add_deduction_guides (tree decl);
+
   public:    
     void find_dependencies (module_state *);
     bool finalize_dependencies ();
@@ -13127,6 +13130,11 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
     {
       tree inner = decl;
 
+      if (deduction_guide_p (inner))
+	/* Ignore deduction guides here, they'll be handled within
+	   find_dependencies for a class template.  */
+	return false;
+
       if (TREE_CODE (inner) == CONST_DECL
 	  && TREE_CODE (DECL_CONTEXT (inner)) == ENUMERAL_TYPE)
 	inner = TYPE_NAME (DECL_CONTEXT (inner));
@@ -13590,6 +13598,50 @@ find_pending_key (tree decl, tree *decl_p = nullptr)
   return ns;
 }
 
+/* Creates bindings and dependencies for all deduction guides of
+   the given class template DECL as needed.  */
+
+void
+depset::hash::add_deduction_guides (tree decl)
+{
+  /* Alias templates never have deduction guides.  */
+  if (DECL_ALIAS_TEMPLATE_P (decl))
+    return;
+
+  /* We don't need to do anything for class-scope deduction guides,
+     as they will be added as members anyway.  */
+  if (!DECL_NAMESPACE_SCOPE_P (decl))
+    return;
+
+  tree ns = CP_DECL_CONTEXT (decl);
+  tree name = dguide_name (decl);
+
+  /* We always add all deduction guides with a given name at once,
+     so if there's already a binding there's nothing to do.  */
+  if (find_binding (ns, name))
+    return;
+
+  tree guides = lookup_qualified_name (ns, name, LOOK_want::NORMAL,
+				       /*complain=*/false);
+  if (guides == error_mark_node)
+    return;
+
+  /* We have bindings to add.  */
+  depset *binding = make_binding (ns, name);
+  add_namespace_context (binding, ns);
+
+  depset **slot = binding_slot (ns, name, /*insert=*/true);
+  *slot = binding;
+
+  for (lkp_iterator it (guides); it; ++it)
+    {
+      gcc_checking_assert (!TREE_VISITED (*it));
+      depset *dep = make_dependency (*it, EK_FOR_BINDING);
+      binding->deps.safe_push (dep);
+      dep->deps.safe_push (binding);
+    }
+}
+
 /* Iteratively find dependencies.  During the walk we may find more
    entries on the same binding that need walking.  */
 
@@ -13649,6 +13701,10 @@ depset::hash::find_dependencies (module_state *module)
 		}
 	      walker.end ();
 
+	      if (!walker.is_key_order ()
+		  && DECL_CLASS_TEMPLATE_P (decl))
+		add_deduction_guides (decl);
+
 	      if (!walker.is_key_order ()
 		  && TREE_CODE (decl) == TEMPLATE_DECL
 		  && !DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl))
@@ -15145,6 +15201,10 @@ 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
+			 reachable whenever their class template is.  */
+		      flags |= cbf_export;
 		  }
 
 		gcc_checking_assert (DECL_P (bound));
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 607753ae6b7..8e007a571a5 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -30784,7 +30784,7 @@ deduction_guides_for (tree tmpl, bool &any_dguides_p, tsubst_flags_t complain)
   else
     {
       cands = ctor_deduction_guides_for (tmpl, complain);
-      for (ovl_iterator it (guides); it; ++it)
+      for (lkp_iterator it (guides); it; ++it)
 	cands = lookup_add (*it, cands);
     }
 
diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_a.C b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
new file mode 100644
index 00000000000..834e033eae3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
@@ -0,0 +1,44 @@
+// PR c++/115231
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi M }
+
+module;
+
+template <typename T>
+struct A {
+  template <typename U> A(U);
+};
+
+template <typename T> A(T) -> A<T>;
+
+export module M;
+
+// Exporting a GMF entity should make the deduction guides reachable.
+export using ::A;
+
+
+export template <typename T>
+struct B {
+  template <typename U> B(U);
+};
+
+// Not exported, but should still be reachable by [temp.deduct.guide] p1.
+B(int) -> B<double>;
+
+
+// Class-scope deduction guides should be reachable as well, even if
+// the class body was not exported.
+export template <typename T> struct C;
+
+template <typename T>
+struct C {
+  template <typename U>
+  struct I {
+    template <typename V> I(V);
+  };
+
+  I(int) -> I<int>;
+
+  template <typename P>
+  I(const P*) -> I<P>;
+};
diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_b.C b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
new file mode 100644
index 00000000000..97266986d8f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
@@ -0,0 +1,20 @@
+// PR c++/115231
+// { dg-additional-options "-fmodules-ts" }
+
+import M;
+
+int main() {
+  // Check that deduction guides are reachable,
+  // and that they declared the right type.
+  A a(1);
+  A<int> a2 = a;
+
+  B b(2);
+  B<double> b2 = b;
+
+  C<int>::I x(10);
+  C<int>::I<int> x2 = x;
+
+  C<int>::I y("xyz");
+  C<int>::I<char> y2 = y;
+}
diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_a.C b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
new file mode 100644
index 00000000000..fcd6c579813
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
@@ -0,0 +1,24 @@
+// PR c++/115231
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi M }
+
+module;
+
+template <typename T>
+struct A {
+  template <typename U> A(U);
+};
+template <typename T> A(T) -> A<T>;
+
+export module M;
+
+template <typename T>
+struct B {
+  template <typename U> B(U);
+};
+B(int) -> B<int>;
+
+// Accessing deduction guides should be possible,
+// even if we can't name the type directly.
+export A<void> f();
+export B<void> g();
diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_b.C b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
new file mode 100644
index 00000000000..ca31306aea3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
@@ -0,0 +1,19 @@
+// PR c++/115231
+// { dg-additional-options "-fmodules-ts" }
+
+import M;
+
+template <typename>
+struct U;
+
+template <template <typename> typename TT, typename Inner>
+struct U<TT<Inner>> {
+  void go() {
+    TT t(10);
+  }
+};
+
+int main() {
+  U<decltype(f())>{}.go();
+  U<decltype(g())>{}.go();
+}
diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_a.C b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
new file mode 100644
index 00000000000..33350ce9804
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi A }
+
+export module A;
+
+extern "C++" {
+  template <typename T> struct S;
+  S(int) -> S<int>;
+  S(double) -> S<double>;
+}
diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_b.C b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
new file mode 100644
index 00000000000..d23696c74bd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi B }
+
+export module B;
+
+extern "C++" {
+  template <typename T> struct S;
+  S(int) -> S<int>;
+  S(char) -> S<char>;
+}
diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_c.C b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
new file mode 100644
index 00000000000..39ae8f4aa58
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
@@ -0,0 +1,22 @@
+// { dg-additional-options "-fmodules-ts" }
+// Test merging deduction guides.
+
+import A;
+import B;
+
+template <typename T> struct S {
+  template <typename U> S(U);
+};
+
+int main() {
+  S x(123);  // { dg-bogus "incomplete" "" { xfail *-*-* } }
+  S<int> x2 = x;
+
+  S y('c');  // { dg-bogus "incomplete" "" { xfail *-*-* } }
+  S<char> y2 = y;
+
+  S z(0.5);  // { dg-bogus "incomplete" "" { xfail *-*-* } }
+  S<double> z2 = z;
+}
+
+S(char) -> S<double>;  // { dg-error "ambiguating" }
-- 
2.43.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231]
  2024-06-16  2:29 [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231] Nathaniel Shead
@ 2024-06-16  5:41 ` Nathaniel Shead
  2024-07-07  2:21   ` Nathaniel Shead
  2024-07-23 20:17 ` Jason Merrill
  1 sibling, 1 reply; 9+ messages in thread
From: Nathaniel Shead @ 2024-06-16  5:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell

On Sun, Jun 16, 2024 at 12:29:23PM +1000, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> This probably isn't the most efficient approach, since we need to do
> name lookup to find deduction guides for a type which will also
> potentially do a bunch of pointless lazy loading from imported modules,
> but I wasn't able to work out a better approach without completely
> reworking how deduction guides are stored and represented.  This way at
> least is correct (I believe), and we can maybe revisit this in the
> future.
> 
> -- >8 --
> 
> Deduction guides are represented as 'normal' functions currently, and
> have no special handling in modules.  However, this causes some issues;
> by [temp.deduct.guide] a deduction guide is not found by normal name
> lookup and instead all reachable deduction guides for a class template
> should be considered, but this does not happen currently.
> 
> To solve this, this patch ensures that all deduction guides are
> considered exported to ensure that they are always visible to importers
> if they are reachable.  Another alternative here would be to add a new
> kind of "all reachable" flag to name lookup, but that is complicated by
> some difficulties in handling GM entities; this may be a better way to
> go if more kinds of entities end up needing this handling, however.
> 
> Another issue here is that because deduction guides are "unrelated"
> functions, they will usually get discarded from the GMF, so this patch
> ensures that when finding dependencies, GMF deduction guides will also
> have bindings created.  We do this in find_dependencies so that we don't
> unnecessarily create bindings for GMF deduction guides that are never
> reached; for consistency we do this for *all* deduction guides, not just
> GM ones.
> 
> Finally, when merging deduction guides from multiple modules, the name
> lookup code may now return two-dimensional overload sets, so update
> callers to match.
> 
> As a small drive-by improvement this patch also updates the error pretty
> printing code to add a space before the '->' when printing a deduction
> guide, so we get 'S(int) -> S<int>' instead of 'S(int)-> S<int>'.
> 
> 	PR c++/115231
> 
> gcc/cp/ChangeLog:
> 
> 	* error.cc (dump_function_decl): Add a space before '->' when
> 	printing deduction guides.
> 	* module.cc (depset::hash::add_binding_entity): Skip deduction
> 	guides here.
> 	(depset::hash::add_deduction_guides): New.
> 	(depset::hash::find_dependencies): Add deduction guide
> 	dependencies for a class template.
> 	(module_state::write_cluster): Always consider deduction guides
> 	as exported.
> 	* pt.cc (deduction_guides_for): Use 'lkp_iterator' instead of
> 	'ovl_iterator'.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/dguide-1_a.C: New test.
> 	* g++.dg/modules/dguide-1_b.C: New test.
> 	* g++.dg/modules/dguide-2_a.C: New test.
> 	* g++.dg/modules/dguide-2_b.C: New test.
> 	* g++.dg/modules/dguide-3_a.C: New test.
> 	* g++.dg/modules/dguide-3_b.C: New test.
> 	* g++.dg/modules/dguide-3_c.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/error.cc                           |  1 +
>  gcc/cp/module.cc                          | 60 +++++++++++++++++++++++
>  gcc/cp/pt.cc                              |  2 +-
>  gcc/testsuite/g++.dg/modules/dguide-1_a.C | 44 +++++++++++++++++
>  gcc/testsuite/g++.dg/modules/dguide-1_b.C | 20 ++++++++
>  gcc/testsuite/g++.dg/modules/dguide-2_a.C | 24 +++++++++
>  gcc/testsuite/g++.dg/modules/dguide-2_b.C | 19 +++++++
>  gcc/testsuite/g++.dg/modules/dguide-3_a.C | 10 ++++
>  gcc/testsuite/g++.dg/modules/dguide-3_b.C | 10 ++++
>  gcc/testsuite/g++.dg/modules/dguide-3_c.C | 22 +++++++++
>  10 files changed, 211 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_b.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_b.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_b.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_c.C
> 
> diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
> index 171a352c85f..2fb5084320e 100644
> --- a/gcc/cp/error.cc
> +++ b/gcc/cp/error.cc
> @@ -1866,6 +1866,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags)
>  	dump_type_suffix (pp, ret, flags);
>        else if (deduction_guide_p (t))
>  	{
> +	  pp->set_padding (pp_before);
>  	  pp_cxx_ws_string (pp, "->");
>  	  dump_type (pp, TREE_TYPE (TREE_TYPE (t)), flags);
>  	}
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 6d6044af199..a2c9e151fd5 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -2589,6 +2589,9 @@ public:
>      void add_partial_entities (vec<tree, va_gc> *);
>      void add_class_entities (vec<tree, va_gc> *);
>  
> +  private:
> +    void add_deduction_guides (tree decl);
> +
>    public:    
>      void find_dependencies (module_state *);
>      bool finalize_dependencies ();
> @@ -13127,6 +13130,11 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
>      {
>        tree inner = decl;
>  
> +      if (deduction_guide_p (inner))
> +	/* Ignore deduction guides here, they'll be handled within
> +	   find_dependencies for a class template.  */
> +	return false;
> +
>        if (TREE_CODE (inner) == CONST_DECL
>  	  && TREE_CODE (DECL_CONTEXT (inner)) == ENUMERAL_TYPE)
>  	inner = TYPE_NAME (DECL_CONTEXT (inner));
> @@ -13590,6 +13598,50 @@ find_pending_key (tree decl, tree *decl_p = nullptr)
>    return ns;
>  }
>  
> +/* Creates bindings and dependencies for all deduction guides of
> +   the given class template DECL as needed.  */
> +
> +void
> +depset::hash::add_deduction_guides (tree decl)
> +{
> +  /* Alias templates never have deduction guides.  */
> +  if (DECL_ALIAS_TEMPLATE_P (decl))
> +    return;
> +
> +  /* We don't need to do anything for class-scope deduction guides,
> +     as they will be added as members anyway.  */
> +  if (!DECL_NAMESPACE_SCOPE_P (decl))
> +    return;
> +
> +  tree ns = CP_DECL_CONTEXT (decl);
> +  tree name = dguide_name (decl);
> +
> +  /* We always add all deduction guides with a given name at once,
> +     so if there's already a binding there's nothing to do.  */
> +  if (find_binding (ns, name))
> +    return;
> +
> +  tree guides = lookup_qualified_name (ns, name, LOOK_want::NORMAL,
> +				       /*complain=*/false);
> +  if (guides == error_mark_node)
> +    return;
> +
> +  /* We have bindings to add.  */
> +  depset *binding = make_binding (ns, name);
> +  add_namespace_context (binding, ns);
> +
> +  depset **slot = binding_slot (ns, name, /*insert=*/true);
> +  *slot = binding;
> +
> +  for (lkp_iterator it (guides); it; ++it)
> +    {
> +      gcc_checking_assert (!TREE_VISITED (*it));
> +      depset *dep = make_dependency (*it, EK_FOR_BINDING);
> +      binding->deps.safe_push (dep);
> +      dep->deps.safe_push (binding);
> +    }
> +}
> +
>  /* Iteratively find dependencies.  During the walk we may find more
>     entries on the same binding that need walking.  */
>  
> @@ -13649,6 +13701,10 @@ depset::hash::find_dependencies (module_state *module)
>  		}
>  	      walker.end ();
>  
> +	      if (!walker.is_key_order ()
> +		  && DECL_CLASS_TEMPLATE_P (decl))
> +		add_deduction_guides (decl);
> +
>  	      if (!walker.is_key_order ()
>  		  && TREE_CODE (decl) == TEMPLATE_DECL
>  		  && !DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl))
> @@ -15145,6 +15201,10 @@ 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
> +			 reachable whenever their class template is.  */
> +		      flags |= cbf_export;
>  		  }
>  
>  		gcc_checking_assert (DECL_P (bound));
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 607753ae6b7..8e007a571a5 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -30784,7 +30784,7 @@ deduction_guides_for (tree tmpl, bool &any_dguides_p, tsubst_flags_t complain)
>    else
>      {
>        cands = ctor_deduction_guides_for (tmpl, complain);
> -      for (ovl_iterator it (guides); it; ++it)
> +      for (lkp_iterator it (guides); it; ++it)
>  	cands = lookup_add (*it, cands);
>      }
>  
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_a.C b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
> new file mode 100644
> index 00000000000..834e033eae3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
> @@ -0,0 +1,44 @@
> +// PR c++/115231
> +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> +// { dg-module-cmi M }
> +
> +module;
> +
> +template <typename T>
> +struct A {
> +  template <typename U> A(U);
> +};
> +
> +template <typename T> A(T) -> A<T>;
> +
> +export module M;
> +
> +// Exporting a GMF entity should make the deduction guides reachable.
> +export using ::A;
> +
> +
> +export template <typename T>
> +struct B {
> +  template <typename U> B(U);
> +};
> +
> +// Not exported, but should still be reachable by [temp.deduct.guide] p1.
> +B(int) -> B<double>;
> +
> +
> +// Class-scope deduction guides should be reachable as well, even if
> +// the class body was not exported.
> +export template <typename T> struct C;
> +
> +template <typename T>
> +struct C {
> +  template <typename U>
> +  struct I {
> +    template <typename V> I(V);
> +  };
> +
> +  I(int) -> I<int>;
> +
> +  template <typename P>
> +  I(const P*) -> I<P>;
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_b.C b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
> new file mode 100644
> index 00000000000..97266986d8f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
> @@ -0,0 +1,20 @@
> +// PR c++/115231
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import M;
> +
> +int main() {
> +  // Check that deduction guides are reachable,
> +  // and that they declared the right type.
> +  A a(1);
> +  A<int> a2 = a;
> +
> +  B b(2);
> +  B<double> b2 = b;
> +
> +  C<int>::I x(10);
> +  C<int>::I<int> x2 = x;
> +
> +  C<int>::I y("xyz");
> +  C<int>::I<char> y2 = y;
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_a.C b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
> new file mode 100644
> index 00000000000..fcd6c579813
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
> @@ -0,0 +1,24 @@
> +// PR c++/115231
> +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> +// { dg-module-cmi M }
> +
> +module;
> +
> +template <typename T>
> +struct A {
> +  template <typename U> A(U);
> +};
> +template <typename T> A(T) -> A<T>;
> +
> +export module M;
> +
> +template <typename T>
> +struct B {
> +  template <typename U> B(U);
> +};
> +B(int) -> B<int>;
> +
> +// Accessing deduction guides should be possible,
> +// even if we can't name the type directly.
> +export A<void> f();
> +export B<void> g();
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_b.C b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
> new file mode 100644
> index 00000000000..ca31306aea3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
> @@ -0,0 +1,19 @@
> +// PR c++/115231
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import M;
> +
> +template <typename>
> +struct U;
> +
> +template <template <typename> typename TT, typename Inner>
> +struct U<TT<Inner>> {
> +  void go() {
> +    TT t(10);
> +  }
> +};
> +
> +int main() {
> +  U<decltype(f())>{}.go();
> +  U<decltype(g())>{}.go();
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_a.C b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
> new file mode 100644
> index 00000000000..33350ce9804
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
> @@ -0,0 +1,10 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi A }
> +
> +export module A;
> +
> +extern "C++" {
> +  template <typename T> struct S;
> +  S(int) -> S<int>;
> +  S(double) -> S<double>;
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_b.C b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
> new file mode 100644
> index 00000000000..d23696c74bd
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
> @@ -0,0 +1,10 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi B }
> +
> +export module B;
> +
> +extern "C++" {
> +  template <typename T> struct S;
> +  S(int) -> S<int>;
> +  S(char) -> S<char>;
> +}
> diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_c.C b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
> new file mode 100644
> index 00000000000..39ae8f4aa58
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
> @@ -0,0 +1,22 @@
> +// { dg-additional-options "-fmodules-ts" }
> +// Test merging deduction guides.
> +
> +import A;
> +import B;
> +
> +template <typename T> struct S {
> +  template <typename U> S(U);
> +};
> +
> +int main() {
> +  S x(123);  // { dg-bogus "incomplete" "" { xfail *-*-* } }
> +  S<int> x2 = x;
> +
> +  S y('c');  // { dg-bogus "incomplete" "" { xfail *-*-* } }
> +  S<char> y2 = y;
> +
> +  S z(0.5);  // { dg-bogus "incomplete" "" { xfail *-*-* } }
> +  S<double> z2 = z;
> +}
> +
> +S(char) -> S<double>;  // { dg-error "ambiguating" }
> -- 
> 2.43.2
> 

I just realised I forgot to explain these xfails in the above commit:
this is because the return types declare a new "specialisation" that is
treated as a separate type on stream-in, rather than merging with the
existing template on instantiation.

A simplified example of this error is:

  // a.cpp
  export module A;
  extern "C++" template <typename T> struct S;
  export S<int> foo();

  // main.cpp
  import A;
  template <typename T> struct S {};
  int main() { foo(); }

I suspect this is related to PR c++/113814.  I'm pretty sure this should
be valid code ([temp.inst] p2 doesn't seem to require instantiation in A
because a function return type doesn't require a completely-defined
object type and completeness won't affect the semantics of the program)
but I haven't looked much into how to fix this yet.

Nathaniel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231]
  2024-06-16  5:41 ` Nathaniel Shead
@ 2024-07-07  2:21   ` Nathaniel Shead
  0 siblings, 0 replies; 9+ messages in thread
From: Nathaniel Shead @ 2024-07-07  2:21 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell

Ping for https://gcc.gnu.org/pipermail/gcc-patches/2024-June/654792.html

On Sun, Jun 16, 2024 at 3:41 PM Nathaniel Shead
<nathanieloshead@gmail.com> wrote:
>
> On Sun, Jun 16, 2024 at 12:29:23PM +1000, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> >
> > This probably isn't the most efficient approach, since we need to do
> > name lookup to find deduction guides for a type which will also
> > potentially do a bunch of pointless lazy loading from imported modules,
> > but I wasn't able to work out a better approach without completely
> > reworking how deduction guides are stored and represented.  This way at
> > least is correct (I believe), and we can maybe revisit this in the
> > future.
> >
> > -- >8 --
> >
> > Deduction guides are represented as 'normal' functions currently, and
> > have no special handling in modules.  However, this causes some issues;
> > by [temp.deduct.guide] a deduction guide is not found by normal name
> > lookup and instead all reachable deduction guides for a class template
> > should be considered, but this does not happen currently.
> >
> > To solve this, this patch ensures that all deduction guides are
> > considered exported to ensure that they are always visible to importers
> > if they are reachable.  Another alternative here would be to add a new
> > kind of "all reachable" flag to name lookup, but that is complicated by
> > some difficulties in handling GM entities; this may be a better way to
> > go if more kinds of entities end up needing this handling, however.
> >
> > Another issue here is that because deduction guides are "unrelated"
> > functions, they will usually get discarded from the GMF, so this patch
> > ensures that when finding dependencies, GMF deduction guides will also
> > have bindings created.  We do this in find_dependencies so that we don't
> > unnecessarily create bindings for GMF deduction guides that are never
> > reached; for consistency we do this for *all* deduction guides, not just
> > GM ones.
> >
> > Finally, when merging deduction guides from multiple modules, the name
> > lookup code may now return two-dimensional overload sets, so update
> > callers to match.
> >
> > As a small drive-by improvement this patch also updates the error pretty
> > printing code to add a space before the '->' when printing a deduction
> > guide, so we get 'S(int) -> S<int>' instead of 'S(int)-> S<int>'.
> >
> >       PR c++/115231
> >
> > gcc/cp/ChangeLog:
> >
> >       * error.cc (dump_function_decl): Add a space before '->' when
> >       printing deduction guides.
> >       * module.cc (depset::hash::add_binding_entity): Skip deduction
> >       guides here.
> >       (depset::hash::add_deduction_guides): New.
> >       (depset::hash::find_dependencies): Add deduction guide
> >       dependencies for a class template.
> >       (module_state::write_cluster): Always consider deduction guides
> >       as exported.
> >       * pt.cc (deduction_guides_for): Use 'lkp_iterator' instead of
> >       'ovl_iterator'.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * g++.dg/modules/dguide-1_a.C: New test.
> >       * g++.dg/modules/dguide-1_b.C: New test.
> >       * g++.dg/modules/dguide-2_a.C: New test.
> >       * g++.dg/modules/dguide-2_b.C: New test.
> >       * g++.dg/modules/dguide-3_a.C: New test.
> >       * g++.dg/modules/dguide-3_b.C: New test.
> >       * g++.dg/modules/dguide-3_c.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> >  gcc/cp/error.cc                           |  1 +
> >  gcc/cp/module.cc                          | 60 +++++++++++++++++++++++
> >  gcc/cp/pt.cc                              |  2 +-
> >  gcc/testsuite/g++.dg/modules/dguide-1_a.C | 44 +++++++++++++++++
> >  gcc/testsuite/g++.dg/modules/dguide-1_b.C | 20 ++++++++
> >  gcc/testsuite/g++.dg/modules/dguide-2_a.C | 24 +++++++++
> >  gcc/testsuite/g++.dg/modules/dguide-2_b.C | 19 +++++++
> >  gcc/testsuite/g++.dg/modules/dguide-3_a.C | 10 ++++
> >  gcc/testsuite/g++.dg/modules/dguide-3_b.C | 10 ++++
> >  gcc/testsuite/g++.dg/modules/dguide-3_c.C | 22 +++++++++
> >  10 files changed, 211 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_a.C
> >  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_b.C
> >  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_a.C
> >  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_b.C
> >  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_a.C
> >  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_b.C
> >  create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_c.C
> >
> > diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
> > index 171a352c85f..2fb5084320e 100644
> > --- a/gcc/cp/error.cc
> > +++ b/gcc/cp/error.cc
> > @@ -1866,6 +1866,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags)
> >       dump_type_suffix (pp, ret, flags);
> >        else if (deduction_guide_p (t))
> >       {
> > +       pp->set_padding (pp_before);
> >         pp_cxx_ws_string (pp, "->");
> >         dump_type (pp, TREE_TYPE (TREE_TYPE (t)), flags);
> >       }
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 6d6044af199..a2c9e151fd5 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -2589,6 +2589,9 @@ public:
> >      void add_partial_entities (vec<tree, va_gc> *);
> >      void add_class_entities (vec<tree, va_gc> *);
> >
> > +  private:
> > +    void add_deduction_guides (tree decl);
> > +
> >    public:
> >      void find_dependencies (module_state *);
> >      bool finalize_dependencies ();
> > @@ -13127,6 +13130,11 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
> >      {
> >        tree inner = decl;
> >
> > +      if (deduction_guide_p (inner))
> > +     /* Ignore deduction guides here, they'll be handled within
> > +        find_dependencies for a class template.  */
> > +     return false;
> > +
> >        if (TREE_CODE (inner) == CONST_DECL
> >         && TREE_CODE (DECL_CONTEXT (inner)) == ENUMERAL_TYPE)
> >       inner = TYPE_NAME (DECL_CONTEXT (inner));
> > @@ -13590,6 +13598,50 @@ find_pending_key (tree decl, tree *decl_p = nullptr)
> >    return ns;
> >  }
> >
> > +/* Creates bindings and dependencies for all deduction guides of
> > +   the given class template DECL as needed.  */
> > +
> > +void
> > +depset::hash::add_deduction_guides (tree decl)
> > +{
> > +  /* Alias templates never have deduction guides.  */
> > +  if (DECL_ALIAS_TEMPLATE_P (decl))
> > +    return;
> > +
> > +  /* We don't need to do anything for class-scope deduction guides,
> > +     as they will be added as members anyway.  */
> > +  if (!DECL_NAMESPACE_SCOPE_P (decl))
> > +    return;
> > +
> > +  tree ns = CP_DECL_CONTEXT (decl);
> > +  tree name = dguide_name (decl);
> > +
> > +  /* We always add all deduction guides with a given name at once,
> > +     so if there's already a binding there's nothing to do.  */
> > +  if (find_binding (ns, name))
> > +    return;
> > +
> > +  tree guides = lookup_qualified_name (ns, name, LOOK_want::NORMAL,
> > +                                    /*complain=*/false);
> > +  if (guides == error_mark_node)
> > +    return;
> > +
> > +  /* We have bindings to add.  */
> > +  depset *binding = make_binding (ns, name);
> > +  add_namespace_context (binding, ns);
> > +
> > +  depset **slot = binding_slot (ns, name, /*insert=*/true);
> > +  *slot = binding;
> > +
> > +  for (lkp_iterator it (guides); it; ++it)
> > +    {
> > +      gcc_checking_assert (!TREE_VISITED (*it));
> > +      depset *dep = make_dependency (*it, EK_FOR_BINDING);
> > +      binding->deps.safe_push (dep);
> > +      dep->deps.safe_push (binding);
> > +    }
> > +}
> > +
> >  /* Iteratively find dependencies.  During the walk we may find more
> >     entries on the same binding that need walking.  */
> >
> > @@ -13649,6 +13701,10 @@ depset::hash::find_dependencies (module_state *module)
> >               }
> >             walker.end ();
> >
> > +           if (!walker.is_key_order ()
> > +               && DECL_CLASS_TEMPLATE_P (decl))
> > +             add_deduction_guides (decl);
> > +
> >             if (!walker.is_key_order ()
> >                 && TREE_CODE (decl) == TEMPLATE_DECL
> >                 && !DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl))
> > @@ -15145,6 +15201,10 @@ 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
> > +                      reachable whenever their class template is.  */
> > +                   flags |= cbf_export;
> >                 }
> >
> >               gcc_checking_assert (DECL_P (bound));
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 607753ae6b7..8e007a571a5 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -30784,7 +30784,7 @@ deduction_guides_for (tree tmpl, bool &any_dguides_p, tsubst_flags_t complain)
> >    else
> >      {
> >        cands = ctor_deduction_guides_for (tmpl, complain);
> > -      for (ovl_iterator it (guides); it; ++it)
> > +      for (lkp_iterator it (guides); it; ++it)
> >       cands = lookup_add (*it, cands);
> >      }
> >
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_a.C b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
> > new file mode 100644
> > index 00000000000..834e033eae3
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
> > @@ -0,0 +1,44 @@
> > +// PR c++/115231
> > +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> > +// { dg-module-cmi M }
> > +
> > +module;
> > +
> > +template <typename T>
> > +struct A {
> > +  template <typename U> A(U);
> > +};
> > +
> > +template <typename T> A(T) -> A<T>;
> > +
> > +export module M;
> > +
> > +// Exporting a GMF entity should make the deduction guides reachable.
> > +export using ::A;
> > +
> > +
> > +export template <typename T>
> > +struct B {
> > +  template <typename U> B(U);
> > +};
> > +
> > +// Not exported, but should still be reachable by [temp.deduct.guide] p1.
> > +B(int) -> B<double>;
> > +
> > +
> > +// Class-scope deduction guides should be reachable as well, even if
> > +// the class body was not exported.
> > +export template <typename T> struct C;
> > +
> > +template <typename T>
> > +struct C {
> > +  template <typename U>
> > +  struct I {
> > +    template <typename V> I(V);
> > +  };
> > +
> > +  I(int) -> I<int>;
> > +
> > +  template <typename P>
> > +  I(const P*) -> I<P>;
> > +};
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_b.C b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
> > new file mode 100644
> > index 00000000000..97266986d8f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
> > @@ -0,0 +1,20 @@
> > +// PR c++/115231
> > +// { dg-additional-options "-fmodules-ts" }
> > +
> > +import M;
> > +
> > +int main() {
> > +  // Check that deduction guides are reachable,
> > +  // and that they declared the right type.
> > +  A a(1);
> > +  A<int> a2 = a;
> > +
> > +  B b(2);
> > +  B<double> b2 = b;
> > +
> > +  C<int>::I x(10);
> > +  C<int>::I<int> x2 = x;
> > +
> > +  C<int>::I y("xyz");
> > +  C<int>::I<char> y2 = y;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_a.C b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
> > new file mode 100644
> > index 00000000000..fcd6c579813
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
> > @@ -0,0 +1,24 @@
> > +// PR c++/115231
> > +// { dg-additional-options "-fmodules-ts -Wno-global-module" }
> > +// { dg-module-cmi M }
> > +
> > +module;
> > +
> > +template <typename T>
> > +struct A {
> > +  template <typename U> A(U);
> > +};
> > +template <typename T> A(T) -> A<T>;
> > +
> > +export module M;
> > +
> > +template <typename T>
> > +struct B {
> > +  template <typename U> B(U);
> > +};
> > +B(int) -> B<int>;
> > +
> > +// Accessing deduction guides should be possible,
> > +// even if we can't name the type directly.
> > +export A<void> f();
> > +export B<void> g();
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_b.C b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
> > new file mode 100644
> > index 00000000000..ca31306aea3
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
> > @@ -0,0 +1,19 @@
> > +// PR c++/115231
> > +// { dg-additional-options "-fmodules-ts" }
> > +
> > +import M;
> > +
> > +template <typename>
> > +struct U;
> > +
> > +template <template <typename> typename TT, typename Inner>
> > +struct U<TT<Inner>> {
> > +  void go() {
> > +    TT t(10);
> > +  }
> > +};
> > +
> > +int main() {
> > +  U<decltype(f())>{}.go();
> > +  U<decltype(g())>{}.go();
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_a.C b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
> > new file mode 100644
> > index 00000000000..33350ce9804
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
> > @@ -0,0 +1,10 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi A }
> > +
> > +export module A;
> > +
> > +extern "C++" {
> > +  template <typename T> struct S;
> > +  S(int) -> S<int>;
> > +  S(double) -> S<double>;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_b.C b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
> > new file mode 100644
> > index 00000000000..d23696c74bd
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
> > @@ -0,0 +1,10 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi B }
> > +
> > +export module B;
> > +
> > +extern "C++" {
> > +  template <typename T> struct S;
> > +  S(int) -> S<int>;
> > +  S(char) -> S<char>;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_c.C b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
> > new file mode 100644
> > index 00000000000..39ae8f4aa58
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
> > @@ -0,0 +1,22 @@
> > +// { dg-additional-options "-fmodules-ts" }
> > +// Test merging deduction guides.
> > +
> > +import A;
> > +import B;
> > +
> > +template <typename T> struct S {
> > +  template <typename U> S(U);
> > +};
> > +
> > +int main() {
> > +  S x(123);  // { dg-bogus "incomplete" "" { xfail *-*-* } }
> > +  S<int> x2 = x;
> > +
> > +  S y('c');  // { dg-bogus "incomplete" "" { xfail *-*-* } }
> > +  S<char> y2 = y;
> > +
> > +  S z(0.5);  // { dg-bogus "incomplete" "" { xfail *-*-* } }
> > +  S<double> z2 = z;
> > +}
> > +
> > +S(char) -> S<double>;  // { dg-error "ambiguating" }
> > --
> > 2.43.2
> >
>
> I just realised I forgot to explain these xfails in the above commit:
> this is because the return types declare a new "specialisation" that is
> treated as a separate type on stream-in, rather than merging with the
> existing template on instantiation.
>
> A simplified example of this error is:
>
>   // a.cpp
>   export module A;
>   extern "C++" template <typename T> struct S;
>   export S<int> foo();
>
>   // main.cpp
>   import A;
>   template <typename T> struct S {};
>   int main() { foo(); }
>
> I suspect this is related to PR c++/113814.  I'm pretty sure this should
> be valid code ([temp.inst] p2 doesn't seem to require instantiation in A
> because a function return type doesn't require a completely-defined
> object type and completeness won't affect the semantics of the program)
> but I haven't looked much into how to fix this yet.
>
> Nathaniel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231]
  2024-06-16  2:29 [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231] Nathaniel Shead
  2024-06-16  5:41 ` Nathaniel Shead
@ 2024-07-23 20:17 ` Jason Merrill
  2024-07-26  4:52   ` Nathaniel Shead
  1 sibling, 1 reply; 9+ messages in thread
From: Jason Merrill @ 2024-07-23 20:17 UTC (permalink / raw)
  To: Nathaniel Shead, gcc-patches; +Cc: Nathan Sidwell

On 6/15/24 10:29 PM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> This probably isn't the most efficient approach, since we need to do
> name lookup to find deduction guides for a type which will also
> potentially do a bunch of pointless lazy loading from imported modules,
> but I wasn't able to work out a better approach without completely
> reworking how deduction guides are stored and represented.

Indeed.  We likely want to find them more directly from the template; 
it's not clear to me that DECL_INITIAL is used for TEMPLATE_DECL, or we 
could put them in an internal attribute or a separate hash table.

> -- >8 --
> 
> Deduction guides are represented as 'normal' functions currently, and
> have no special handling in modules.  However, this causes some issues;
> by [temp.deduct.guide] a deduction guide is not found by normal name
> lookup and instead all reachable deduction guides for a class template
> should be considered, but this does not happen currently.
> 
> To solve this, this patch ensures that all deduction guides are
> considered exported to ensure that they are always visible to importers
> if they are reachable.  Another alternative here would be to add a new
> kind of "all reachable" flag to name lookup, but that is complicated by
> some difficulties in handling GM entities; this may be a better way to
> go if more kinds of entities end up needing this handling, however.
> 
> Another issue here is that because deduction guides are "unrelated"
> functions, they will usually get discarded from the GMF, so this patch
> ensures that when finding dependencies, GMF deduction guides will also
> have bindings created.  We do this in find_dependencies so that we don't
> unnecessarily create bindings for GMF deduction guides that are never
> reached; for consistency we do this for *all* deduction guides, not just
> GM ones.

If you fixed the dependency calculation, why do they also need to be 
exported?

Jason


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231]
  2024-07-23 20:17 ` Jason Merrill
@ 2024-07-26  4:52   ` Nathaniel Shead
  2024-07-26 17:17     ` Jason Merrill
  0 siblings, 1 reply; 9+ messages in thread
From: Nathaniel Shead @ 2024-07-26  4:52 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Nathan Sidwell

On Tue, Jul 23, 2024 at 04:17:22PM -0400, Jason Merrill wrote:
> On 6/15/24 10:29 PM, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > 
> > This probably isn't the most efficient approach, since we need to do
> > name lookup to find deduction guides for a type which will also
> > potentially do a bunch of pointless lazy loading from imported modules,
> > but I wasn't able to work out a better approach without completely
> > reworking how deduction guides are stored and represented.
> 
> Indeed.  We likely want to find them more directly from the template; it's
> not clear to me that DECL_INITIAL is used for TEMPLATE_DECL, or we could put
> them in an internal attribute or a separate hash table.
> 
> > -- >8 --
> > 
> > Deduction guides are represented as 'normal' functions currently, and
> > have no special handling in modules.  However, this causes some issues;
> > by [temp.deduct.guide] a deduction guide is not found by normal name
> > lookup and instead all reachable deduction guides for a class template
> > should be considered, but this does not happen currently.
> > 
> > To solve this, this patch ensures that all deduction guides are
> > considered exported to ensure that they are always visible to importers
> > if they are reachable.  Another alternative here would be to add a new
> > kind of "all reachable" flag to name lookup, but that is complicated by
> > some difficulties in handling GM entities; this may be a better way to
> > go if more kinds of entities end up needing this handling, however.
> > 
> > Another issue here is that because deduction guides are "unrelated"
> > functions, they will usually get discarded from the GMF, so this patch
> > ensures that when finding dependencies, GMF deduction guides will also
> > have bindings created.  We do this in find_dependencies so that we don't
> > unnecessarily create bindings for GMF deduction guides that are never
> > reached; for consistency we do this for *all* deduction guides, not just
> > GM ones.
> 
> If you fixed the dependency calculation, why do they also need to be
> exported?
> 
> Jason
> 

Deduction guides aren't found using normal name lookup, but any
reachable deduction guide must be considered.  This means that even if
the module interface exports no declarations whatsoever, a deduction
guide declared in the module purview must still be considered by
importers.

The other option I've considered is adding a new "ANY_REACHABLE" flag to
name lookup which would also consider non-exported reachable decls.  On
further consideration I might actually go this way; I've been thinking
about how to resolve some issues adjacent to supporting textual
redefinitions that I believe this will be necessary for anyway, and we
can probably use this in tsubst_friend_class as well rather than the
current relatively ad-hoc solution.

That said, I've realised that this patch isn't completely sufficient
anyway; consider:

  // m.cpp
  module;
  template <typename T> struct S;
  export module M;
  S(int) -> S<double>;

  // x.cpp
  template <typename T> struct S { S(int); };
  import M;
  int main() {
    S s(10);  // should be S<double> s;
  }

This patch doesn't correctly handle this case yet, we need to also
consider cases where only the deduction guide is in purview.

Nathaniel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231]
  2024-07-26  4:52   ` Nathaniel Shead
@ 2024-07-26 17:17     ` Jason Merrill
  2024-07-27  1:41       ` Nathaniel Shead
  2024-08-06 12:03       ` [PATCH v2] " Nathaniel Shead
  0 siblings, 2 replies; 9+ messages in thread
From: Jason Merrill @ 2024-07-26 17:17 UTC (permalink / raw)
  To: Nathaniel Shead; +Cc: gcc-patches, Nathan Sidwell

On 7/26/24 12:52 AM, Nathaniel Shead wrote:
> On Tue, Jul 23, 2024 at 04:17:22PM -0400, Jason Merrill wrote:
>> On 6/15/24 10:29 PM, Nathaniel Shead wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>>>
>>> This probably isn't the most efficient approach, since we need to do
>>> name lookup to find deduction guides for a type which will also
>>> potentially do a bunch of pointless lazy loading from imported modules,
>>> but I wasn't able to work out a better approach without completely
>>> reworking how deduction guides are stored and represented.
>>
>> Indeed.  We likely want to find them more directly from the template; it's
>> not clear to me that DECL_INITIAL is used for TEMPLATE_DECL, or we could put
>> them in an internal attribute or a separate hash table.
>>
>>> -- >8 --
>>>
>>> Deduction guides are represented as 'normal' functions currently, and
>>> have no special handling in modules.  However, this causes some issues;
>>> by [temp.deduct.guide] a deduction guide is not found by normal name
>>> lookup and instead all reachable deduction guides for a class template
>>> should be considered, but this does not happen currently.
>>>
>>> To solve this, this patch ensures that all deduction guides are
>>> considered exported to ensure that they are always visible to importers
>>> if they are reachable.  Another alternative here would be to add a new
>>> kind of "all reachable" flag to name lookup, but that is complicated by
>>> some difficulties in handling GM entities; this may be a better way to
>>> go if more kinds of entities end up needing this handling, however.
>>>
>>> Another issue here is that because deduction guides are "unrelated"
>>> functions, they will usually get discarded from the GMF, so this patch
>>> ensures that when finding dependencies, GMF deduction guides will also
>>> have bindings created.  We do this in find_dependencies so that we don't
>>> unnecessarily create bindings for GMF deduction guides that are never
>>> reached; for consistency we do this for *all* deduction guides, not just
>>> GM ones.
>>
>> If you fixed the dependency calculation, why do they also need to be
>> exported?
> 
> Deduction guides aren't found using normal name lookup, but any
> reachable deduction guide must be considered.  This means that even if
> the module interface exports no declarations whatsoever, a deduction
> guide declared in the module purview must still be considered by
> importers.

Ah, I was missing the name lookup issue.

> The other option I've considered is adding a new "ANY_REACHABLE" flag to
> name lookup which would also consider non-exported reachable decls.  On
> further consideration I might actually go this way; I've been thinking
> about how to resolve some issues adjacent to supporting textual
> redefinitions that I believe this will be necessary for anyway, and we
> can probably use this in tsubst_friend_class as well rather than the
> current relatively ad-hoc solution.

There's also my hack in lookup_elaborated_type for ABI namespace types.

I'm not sure that it should be necessary to do this for redefinitions, 
though; what's the advantage over merging in check_module_override 
(apart from that needing to be fixed)?

> That said, I've realised that this patch isn't completely sufficient
> anyway; consider:
> 
>    // m.cpp
>    module;
>    template <typename T> struct S;
>    export module M;
>    S(int) -> S<double>;
> 
>    // x.cpp
>    template <typename T> struct S { S(int); };
>    import M;
>    int main() {
>      S s(10);  // should be S<double> s;
>    }
> 
> This patch doesn't correctly handle this case yet, we need to also
> consider cases where only the deduction guide is in purview.

Indeed.

Jason


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231]
  2024-07-26 17:17     ` Jason Merrill
@ 2024-07-27  1:41       ` Nathaniel Shead
  2024-08-06 12:03       ` [PATCH v2] " Nathaniel Shead
  1 sibling, 0 replies; 9+ messages in thread
From: Nathaniel Shead @ 2024-07-27  1:41 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Nathan Sidwell

On Fri, Jul 26, 2024 at 01:17:57PM -0400, Jason Merrill wrote:
> On 7/26/24 12:52 AM, Nathaniel Shead wrote:
> > On Tue, Jul 23, 2024 at 04:17:22PM -0400, Jason Merrill wrote:
> > > On 6/15/24 10:29 PM, Nathaniel Shead wrote:
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > > 
> > > > This probably isn't the most efficient approach, since we need to do
> > > > name lookup to find deduction guides for a type which will also
> > > > potentially do a bunch of pointless lazy loading from imported modules,
> > > > but I wasn't able to work out a better approach without completely
> > > > reworking how deduction guides are stored and represented.
> > > 
> > > Indeed.  We likely want to find them more directly from the template; it's
> > > not clear to me that DECL_INITIAL is used for TEMPLATE_DECL, or we could put
> > > them in an internal attribute or a separate hash table.
> > > 
> > > > -- >8 --
> > > > 
> > > > Deduction guides are represented as 'normal' functions currently, and
> > > > have no special handling in modules.  However, this causes some issues;
> > > > by [temp.deduct.guide] a deduction guide is not found by normal name
> > > > lookup and instead all reachable deduction guides for a class template
> > > > should be considered, but this does not happen currently.
> > > > 
> > > > To solve this, this patch ensures that all deduction guides are
> > > > considered exported to ensure that they are always visible to importers
> > > > if they are reachable.  Another alternative here would be to add a new
> > > > kind of "all reachable" flag to name lookup, but that is complicated by
> > > > some difficulties in handling GM entities; this may be a better way to
> > > > go if more kinds of entities end up needing this handling, however.
> > > > 
> > > > Another issue here is that because deduction guides are "unrelated"
> > > > functions, they will usually get discarded from the GMF, so this patch
> > > > ensures that when finding dependencies, GMF deduction guides will also
> > > > have bindings created.  We do this in find_dependencies so that we don't
> > > > unnecessarily create bindings for GMF deduction guides that are never
> > > > reached; for consistency we do this for *all* deduction guides, not just
> > > > GM ones.
> > > 
> > > If you fixed the dependency calculation, why do they also need to be
> > > exported?
> > 
> > Deduction guides aren't found using normal name lookup, but any
> > reachable deduction guide must be considered.  This means that even if
> > the module interface exports no declarations whatsoever, a deduction
> > guide declared in the module purview must still be considered by
> > importers.
> 
> Ah, I was missing the name lookup issue.
> 
> > The other option I've considered is adding a new "ANY_REACHABLE" flag to
> > name lookup which would also consider non-exported reachable decls.  On
> > further consideration I might actually go this way; I've been thinking
> > about how to resolve some issues adjacent to supporting textual
> > redefinitions that I believe this will be necessary for anyway, and we
> > can probably use this in tsubst_friend_class as well rather than the
> > current relatively ad-hoc solution.
> 
> There's also my hack in lookup_elaborated_type for ABI namespace types.
> 
> I'm not sure that it should be necessary to do this for redefinitions,
> though; what's the advantage over merging in check_module_override (apart
> from that needing to be fixed)?
> 

My thought had been because currently type redefinitions don't go
through 'check_module_override' at all, if the existing type name is
visible, and so I was thinking to fix it at that end instead.  This was
also to fix things that aren't strictly redefinitions but also fail
currently, e.g.

  module;
  template <typename T> struct S;
  export module M;
  export S<int> foo();  // declares a specialisation of S

  // ...

  import M;
  template <typename T> struct S {};  // definition of template
  int main() {
    foo();  // error: invalid use of incomplete type, because
    // we don't ever go through redeclare_class_template in xref_tag
  }

But on further thought maybe it would be more consistent to ensure that
check_module_override handles this, since I suppose we still need to
handle things like the following anyway:

  export module M;
  struct S {};  // not exported

  // ...

  import M;
  template <typename T> struct S {};

Though the names correspond ([basic.scope.scope] p4), they don't declare
the same entity ([basic.link] p8) since they have different attachment,
and they don't potentially conflict ([basic.scope.scope] p6) because S@M
doesn't precede the template decl because it's not visible to name
lookup.  So an "any reachable" flag would be incorrect here regardless.

(Personally, I now even wonder whether it'd be make sense to be more
consistent here, by type declarations always being a new type that is
then merged with an existing one where necessary, rather than xref_tag
always returning an existing type if it finds one, which would help with
compiler errors in the presence of using-decls and help avoid the
duplication of checks for redefining existing names spread across
various places in parser.cc, decl.cc, and name-lookup.cc?)

> > That said, I've realised that this patch isn't completely sufficient
> > anyway; consider:
> > 
> >    // m.cpp
> >    module;
> >    template <typename T> struct S;
> >    export module M;
> >    S(int) -> S<double>;
> > 
> >    // x.cpp
> >    template <typename T> struct S { S(int); };
> >    import M;
> >    int main() {
> >      S s(10);  // should be S<double> s;
> >    }
> > 
> > This patch doesn't correctly handle this case yet, we need to also
> > consider cases where only the deduction guide is in purview.
> 
> Indeed.
> 
> Jason
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] c++/modules: Ensure deduction guides are always reachable [PR115231]
  2024-07-26 17:17     ` Jason Merrill
  2024-07-27  1:41       ` Nathaniel Shead
@ 2024-08-06 12:03       ` Nathaniel Shead
  2024-08-06 21:39         ` Jason Merrill
  1 sibling, 1 reply; 9+ messages in thread
From: Nathaniel Shead @ 2024-08-06 12:03 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Nathan Sidwell

On Fri, Jul 26, 2024 at 01:17:57PM -0400, Jason Merrill wrote:
> On 7/26/24 12:52 AM, Nathaniel Shead wrote:
> > On Tue, Jul 23, 2024 at 04:17:22PM -0400, Jason Merrill wrote:
> > > On 6/15/24 10:29 PM, Nathaniel Shead wrote:
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > > 
> > > > This probably isn't the most efficient approach, since we need to do
> > > > name lookup to find deduction guides for a type which will also
> > > > potentially do a bunch of pointless lazy loading from imported modules,
> > > > but I wasn't able to work out a better approach without completely
> > > > reworking how deduction guides are stored and represented.
> > > 
> > > Indeed.  We likely want to find them more directly from the template; it's
> > > not clear to me that DECL_INITIAL is used for TEMPLATE_DECL, or we could put
> > > them in an internal attribute or a separate hash table.
> > > 

I had a go at exploring some other representations of deduction guides
but I didn't really land anywhere; most things I tried would require
reimplementing a lot of the merging logic handled by name lookup
currently.  Potentially just having an additional hash table for
deduction guides maintained by pushdecl would work but that felt like
unnecessary duplication, I'm not sure that the benefits outweight the
cost of the name lookup.  (But I haven't measured this.)

> > > > -- >8 --
> > > > 
> > > > Deduction guides are represented as 'normal' functions currently, and
> > > > have no special handling in modules.  However, this causes some issues;
> > > > by [temp.deduct.guide] a deduction guide is not found by normal name
> > > > lookup and instead all reachable deduction guides for a class template
> > > > should be considered, but this does not happen currently.
> > > > 
> > > > To solve this, this patch ensures that all deduction guides are
> > > > considered exported to ensure that they are always visible to importers
> > > > if they are reachable.  Another alternative here would be to add a new
> > > > kind of "all reachable" flag to name lookup, but that is complicated by
> > > > some difficulties in handling GM entities; this may be a better way to
> > > > go if more kinds of entities end up needing this handling, however.
> > > > 
> > > > Another issue here is that because deduction guides are "unrelated"
> > > > functions, they will usually get discarded from the GMF, so this patch
> > > > ensures that when finding dependencies, GMF deduction guides will also
> > > > have bindings created.  We do this in find_dependencies so that we don't
> > > > unnecessarily create bindings for GMF deduction guides that are never
> > > > reached; for consistency we do this for *all* deduction guides, not just
> > > > GM ones.
> > > 
> > > If you fixed the dependency calculation, why do they also need to be
> > > exported?
> > 
> > Deduction guides aren't found using normal name lookup, but any
> > reachable deduction guide must be considered.  This means that even if
> > the module interface exports no declarations whatsoever, a deduction
> > guide declared in the module purview must still be considered by
> > importers.
> 
> Ah, I was missing the name lookup issue.
> 
> > The other option I've considered is adding a new "ANY_REACHABLE" flag to
> > name lookup which would also consider non-exported reachable decls.  On
> > further consideration I might actually go this way; I've been thinking
> > about how to resolve some issues adjacent to supporting textual
> > redefinitions that I believe this will be necessary for anyway, and we
> > can probably use this in tsubst_friend_class as well rather than the
> > current relatively ad-hoc solution.
> 
> There's also my hack in lookup_elaborated_type for ABI namespace types.
> 
> I'm not sure that it should be necessary to do this for redefinitions,
> though; what's the advantage over merging in check_module_override (apart
> from that needing to be fixed)?
> 
> > That said, I've realised that this patch isn't completely sufficient
> > anyway; consider:
> > 
> >    // m.cpp
> >    module;
> >    template <typename T> struct S;
> >    export module M;
> >    S(int) -> S<double>;
> > 
> >    // x.cpp
> >    template <typename T> struct S { S(int); };
> >    import M;
> >    int main() {
> >      S s(10);  // should be S<double> s;
> >    }
> > 
> > This patch doesn't correctly handle this case yet, we need to also
> > consider cases where only the deduction guide is in purview.
> 
> Indeed.
> 
> Jason
> 

Here's a small update to the patch which fixes the above bug by ensuring
that dependencies are created for purview deduction guides rather than
just being skipped entirely, and includes that testcase with dguide-3.

This patch is still using force-exported bindings rather than attempting
to do anything more clever with name lookup just yet.

Bootstrapped and regtested (so far just modules.exp) on
x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?

-- >8 --

Deduction guides are represented as 'normal' functions currently, and
have no special handling in modules.  However, this causes some issues;
by [temp.deduct.guide] a deduction guide is not found by normal name
lookup and instead all reachable deduction guides for a class template
should be considered, but this does not happen currently.

To solve this, this patch ensures that all deduction guides are
considered exported to ensure that they are always visible to importers
if they are reachable.  Another alternative here would be to add a new
kind of "all reachable" flag to name lookup, but that is complicated by
some difficulties in handling GM entities; this may be a better way to
go if more kinds of entities end up needing this handling, however.

Another issue here is that because deduction guides are "unrelated"
functions, they will usually get discarded from the GMF, so this patch
ensures that when finding dependencies, GMF deduction guides will also
have bindings created.  We do this in find_dependencies so that we don't
unnecessarily create bindings for GMF deduction guides that are never
reached; for consistency we do this for *all* deduction guides, not just
GM ones.  We also make sure that the opposite (a deduction guide being
the only purview reference to a GMF template) correctly marks it as
reachable.

Finally, when merging deduction guides from multiple modules, the name
lookup code may now return two-dimensional overload sets, so update
callers to match.

As a small drive-by improvement this patch also updates the error pretty
printing code to add a space before the '->' when printing a deduction
guide, so we get 'S(int) -> S<int>' instead of 'S(int)-> S<int>'.

	PR c++/115231

gcc/cp/ChangeLog:

	* error.cc (dump_function_decl): Add a space before '->' when
	printing deduction guides.
	* module.cc (depset::hash::add_binding_entity): Don't create
	bindings for guides here, only mark dependencies.
	(depset::hash::add_deduction_guides): New.
	(depset::hash::find_dependencies): Add deduction guide
	dependencies for a class template.
	(module_state::write_cluster): Always consider deduction guides
	as exported.
	* pt.cc (deduction_guides_for): Use 'lkp_iterator' instead of
	'ovl_iterator'.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/dguide-1_a.C: New test.
	* g++.dg/modules/dguide-1_b.C: New test.
	* g++.dg/modules/dguide-2_a.C: New test.
	* g++.dg/modules/dguide-2_b.C: New test.
	* g++.dg/modules/dguide-3_a.C: New test.
	* g++.dg/modules/dguide-3_b.C: New test.
	* g++.dg/modules/dguide-3_c.C: New test.
	* g++.dg/modules/dguide-3_d.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/error.cc                           |  1 +
 gcc/cp/module.cc                          | 64 +++++++++++++++++++++++
 gcc/cp/pt.cc                              |  2 +-
 gcc/testsuite/g++.dg/modules/dguide-1_a.C | 44 ++++++++++++++++
 gcc/testsuite/g++.dg/modules/dguide-1_b.C | 20 +++++++
 gcc/testsuite/g++.dg/modules/dguide-2_a.C | 24 +++++++++
 gcc/testsuite/g++.dg/modules/dguide-2_b.C | 19 +++++++
 gcc/testsuite/g++.dg/modules/dguide-3_a.C | 10 ++++
 gcc/testsuite/g++.dg/modules/dguide-3_b.C | 10 ++++
 gcc/testsuite/g++.dg/modules/dguide-3_c.C |  6 +++
 gcc/testsuite/g++.dg/modules/dguide-3_d.C | 30 +++++++++++
 11 files changed, 229 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-1_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-2_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_a.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_c.C
 create mode 100644 gcc/testsuite/g++.dg/modules/dguide-3_d.C

diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index d80bac822ba..601e5672174 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -1871,6 +1871,7 @@ dump_function_decl (cxx_pretty_printer *pp, tree t, int flags)
 	dump_type_suffix (pp, ret, flags);
       else if (deduction_guide_p (t))
 	{
+	  pp->set_padding (pp_before);
 	  pp_cxx_ws_string (pp, "->");
 	  dump_type (pp, TREE_TYPE (TREE_TYPE (t)), flags);
 	}
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d1607a06757..aef010e91d1 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2589,6 +2589,9 @@ public:
     void add_partial_entities (vec<tree, va_gc> *);
     void add_class_entities (vec<tree, va_gc> *);
 
+  private:
+    void add_deduction_guides (tree decl);
+
   public:    
     void find_dependencies (module_state *);
     bool finalize_dependencies ();
@@ -13172,6 +13175,15 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
 	/* Ignore NTTP objects.  */
 	return false;
 
+      if (deduction_guide_p (decl))
+	{
+	  /* Ignore deduction guides, bindings for them will be created within
+	     find_dependencies for their class template.  But still build a dep
+	     for them so that we don't discard them.  */
+	  data->hash->make_dependency (decl, EK_FOR_BINDING);
+	  return false;
+	}
+
       if (!(flags & WMB_Using) && CP_DECL_CONTEXT (decl) != data->ns)
 	{
 	  /* An unscoped enum constant implicitly brought into the containing
@@ -13600,6 +13612,50 @@ find_pending_key (tree decl, tree *decl_p = nullptr)
   return ns;
 }
 
+/* Creates bindings and dependencies for all deduction guides of
+   the given class template DECL as needed.  */
+
+void
+depset::hash::add_deduction_guides (tree decl)
+{
+  /* Alias templates never have deduction guides.  */
+  if (DECL_ALIAS_TEMPLATE_P (decl))
+    return;
+
+  /* We don't need to do anything for class-scope deduction guides,
+     as they will be added as members anyway.  */
+  if (!DECL_NAMESPACE_SCOPE_P (decl))
+    return;
+
+  tree ns = CP_DECL_CONTEXT (decl);
+  tree name = dguide_name (decl);
+
+  /* We always add all deduction guides with a given name at once,
+     so if there's already a binding there's nothing to do.  */
+  if (find_binding (ns, name))
+    return;
+
+  tree guides = lookup_qualified_name (ns, name, LOOK_want::NORMAL,
+				       /*complain=*/false);
+  if (guides == error_mark_node)
+    return;
+
+  /* We have bindings to add.  */
+  depset *binding = make_binding (ns, name);
+  add_namespace_context (binding, ns);
+
+  depset **slot = binding_slot (ns, name, /*insert=*/true);
+  *slot = binding;
+
+  for (lkp_iterator it (guides); it; ++it)
+    {
+      gcc_checking_assert (!TREE_VISITED (*it));
+      depset *dep = make_dependency (*it, EK_FOR_BINDING);
+      binding->deps.safe_push (dep);
+      dep->deps.safe_push (binding);
+    }
+}
+
 /* Iteratively find dependencies.  During the walk we may find more
    entries on the same binding that need walking.  */
 
@@ -13659,6 +13715,10 @@ depset::hash::find_dependencies (module_state *module)
 		}
 	      walker.end ();
 
+	      if (!walker.is_key_order ()
+		  && DECL_CLASS_TEMPLATE_P (decl))
+		add_deduction_guides (decl);
+
 	      if (!walker.is_key_order ()
 		  && TREE_CODE (decl) == TEMPLATE_DECL
 		  && !DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl))
@@ -15158,6 +15218,10 @@ 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
+			 reachable whenever their class template is.  */
+		      flags |= cbf_export;
 		  }
 
 		gcc_checking_assert (DECL_P (bound));
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 77fa5907c3d..53917cb6dc6 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -30739,7 +30739,7 @@ deduction_guides_for (tree tmpl, bool &any_dguides_p, tsubst_flags_t complain)
   else
     {
       cands = ctor_deduction_guides_for (tmpl, complain);
-      for (ovl_iterator it (guides); it; ++it)
+      for (lkp_iterator it (guides); it; ++it)
 	cands = lookup_add (*it, cands);
     }
 
diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_a.C b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
new file mode 100644
index 00000000000..834e033eae3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-1_a.C
@@ -0,0 +1,44 @@
+// PR c++/115231
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi M }
+
+module;
+
+template <typename T>
+struct A {
+  template <typename U> A(U);
+};
+
+template <typename T> A(T) -> A<T>;
+
+export module M;
+
+// Exporting a GMF entity should make the deduction guides reachable.
+export using ::A;
+
+
+export template <typename T>
+struct B {
+  template <typename U> B(U);
+};
+
+// Not exported, but should still be reachable by [temp.deduct.guide] p1.
+B(int) -> B<double>;
+
+
+// Class-scope deduction guides should be reachable as well, even if
+// the class body was not exported.
+export template <typename T> struct C;
+
+template <typename T>
+struct C {
+  template <typename U>
+  struct I {
+    template <typename V> I(V);
+  };
+
+  I(int) -> I<int>;
+
+  template <typename P>
+  I(const P*) -> I<P>;
+};
diff --git a/gcc/testsuite/g++.dg/modules/dguide-1_b.C b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
new file mode 100644
index 00000000000..97266986d8f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-1_b.C
@@ -0,0 +1,20 @@
+// PR c++/115231
+// { dg-additional-options "-fmodules-ts" }
+
+import M;
+
+int main() {
+  // Check that deduction guides are reachable,
+  // and that they declared the right type.
+  A a(1);
+  A<int> a2 = a;
+
+  B b(2);
+  B<double> b2 = b;
+
+  C<int>::I x(10);
+  C<int>::I<int> x2 = x;
+
+  C<int>::I y("xyz");
+  C<int>::I<char> y2 = y;
+}
diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_a.C b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
new file mode 100644
index 00000000000..fcd6c579813
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-2_a.C
@@ -0,0 +1,24 @@
+// PR c++/115231
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+// { dg-module-cmi M }
+
+module;
+
+template <typename T>
+struct A {
+  template <typename U> A(U);
+};
+template <typename T> A(T) -> A<T>;
+
+export module M;
+
+template <typename T>
+struct B {
+  template <typename U> B(U);
+};
+B(int) -> B<int>;
+
+// Accessing deduction guides should be possible,
+// even if we can't name the type directly.
+export A<void> f();
+export B<void> g();
diff --git a/gcc/testsuite/g++.dg/modules/dguide-2_b.C b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
new file mode 100644
index 00000000000..ca31306aea3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-2_b.C
@@ -0,0 +1,19 @@
+// PR c++/115231
+// { dg-additional-options "-fmodules-ts" }
+
+import M;
+
+template <typename>
+struct U;
+
+template <template <typename> typename TT, typename Inner>
+struct U<TT<Inner>> {
+  void go() {
+    TT t(10);
+  }
+};
+
+int main() {
+  U<decltype(f())>{}.go();
+  U<decltype(g())>{}.go();
+}
diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_a.C b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
new file mode 100644
index 00000000000..33350ce9804
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-3_a.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi A }
+
+export module A;
+
+extern "C++" {
+  template <typename T> struct S;
+  S(int) -> S<int>;
+  S(double) -> S<double>;
+}
diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_b.C b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
new file mode 100644
index 00000000000..d23696c74bd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-3_b.C
@@ -0,0 +1,10 @@
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi B }
+
+export module B;
+
+extern "C++" {
+  template <typename T> struct S;
+  S(int) -> S<int>;
+  S(char) -> S<char>;
+}
diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_c.C b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
new file mode 100644
index 00000000000..95d29db724d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-3_c.C
@@ -0,0 +1,6 @@
+// { dg-additional-options "-fmodules-ts -Wno-global-module" }
+
+module;
+template <typename T> struct S;
+export module C;
+S(const char*) -> S<const char*>;
diff --git a/gcc/testsuite/g++.dg/modules/dguide-3_d.C b/gcc/testsuite/g++.dg/modules/dguide-3_d.C
new file mode 100644
index 00000000000..0d98517b7da
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-3_d.C
@@ -0,0 +1,30 @@
+// { dg-additional-options "-fmodules-ts" }
+// Test merging deduction guides.
+
+template <typename T> struct S {
+  template <typename U> S(U);
+};
+
+import A;
+import B;
+import C;
+
+int main() {
+  // declared in A and B
+  S x(123);
+  S<int> x2 = x;
+
+  // declared only in A
+  S y(0.5);
+  S<double> y2 = y;
+
+  // declared only in B
+  S z('c');
+  S<char> z2 = z;
+
+  // declared only in C (and attached to named module)
+  S w("hello");
+  S<const char*> w2 = w;
+}
+
+S(char) -> S<double>;  // { dg-error "ambiguating" }
-- 
2.43.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] c++/modules: Ensure deduction guides are always reachable [PR115231]
  2024-08-06 12:03       ` [PATCH v2] " Nathaniel Shead
@ 2024-08-06 21:39         ` Jason Merrill
  0 siblings, 0 replies; 9+ messages in thread
From: Jason Merrill @ 2024-08-06 21:39 UTC (permalink / raw)
  To: Nathaniel Shead; +Cc: gcc-patches, Nathan Sidwell

On 8/6/24 8:03 AM, Nathaniel Shead wrote:
> On Fri, Jul 26, 2024 at 01:17:57PM -0400, Jason Merrill wrote:
>> On 7/26/24 12:52 AM, Nathaniel Shead wrote:
>>> On Tue, Jul 23, 2024 at 04:17:22PM -0400, Jason Merrill wrote:
>>>> On 6/15/24 10:29 PM, Nathaniel Shead wrote:
>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>>>>>
>>>>> This probably isn't the most efficient approach, since we need to do
>>>>> name lookup to find deduction guides for a type which will also
>>>>> potentially do a bunch of pointless lazy loading from imported modules,
>>>>> but I wasn't able to work out a better approach without completely
>>>>> reworking how deduction guides are stored and represented.
>>>>
>>>> Indeed.  We likely want to find them more directly from the template; it's
>>>> not clear to me that DECL_INITIAL is used for TEMPLATE_DECL, or we could put
>>>> them in an internal attribute or a separate hash table.
>>>>
> 
> I had a go at exploring some other representations of deduction guides
> but I didn't really land anywhere; most things I tried would require
> reimplementing a lot of the merging logic handled by name lookup
> currently.  Potentially just having an additional hash table for
> deduction guides maintained by pushdecl would work but that felt like
> unnecessary duplication, I'm not sure that the benefits outweight the
> cost of the name lookup.  (But I haven't measured this.)
> 
>>>>> -- >8 --
>>>>>
>>>>> Deduction guides are represented as 'normal' functions currently, and
>>>>> have no special handling in modules.  However, this causes some issues;
>>>>> by [temp.deduct.guide] a deduction guide is not found by normal name
>>>>> lookup and instead all reachable deduction guides for a class template
>>>>> should be considered, but this does not happen currently.
>>>>>
>>>>> To solve this, this patch ensures that all deduction guides are
>>>>> considered exported to ensure that they are always visible to importers
>>>>> if they are reachable.  Another alternative here would be to add a new
>>>>> kind of "all reachable" flag to name lookup, but that is complicated by
>>>>> some difficulties in handling GM entities; this may be a better way to
>>>>> go if more kinds of entities end up needing this handling, however.
>>>>>
>>>>> Another issue here is that because deduction guides are "unrelated"
>>>>> functions, they will usually get discarded from the GMF, so this patch
>>>>> ensures that when finding dependencies, GMF deduction guides will also
>>>>> have bindings created.  We do this in find_dependencies so that we don't
>>>>> unnecessarily create bindings for GMF deduction guides that are never
>>>>> reached; for consistency we do this for *all* deduction guides, not just
>>>>> GM ones.
>>>>
>>>> If you fixed the dependency calculation, why do they also need to be
>>>> exported?
>>>
>>> Deduction guides aren't found using normal name lookup, but any
>>> reachable deduction guide must be considered.  This means that even if
>>> the module interface exports no declarations whatsoever, a deduction
>>> guide declared in the module purview must still be considered by
>>> importers.
>>
>> Ah, I was missing the name lookup issue.
>>
>>> The other option I've considered is adding a new "ANY_REACHABLE" flag to
>>> name lookup which would also consider non-exported reachable decls.  On
>>> further consideration I might actually go this way; I've been thinking
>>> about how to resolve some issues adjacent to supporting textual
>>> redefinitions that I believe this will be necessary for anyway, and we
>>> can probably use this in tsubst_friend_class as well rather than the
>>> current relatively ad-hoc solution.
>>
>> There's also my hack in lookup_elaborated_type for ABI namespace types.
>>
>> I'm not sure that it should be necessary to do this for redefinitions,
>> though; what's the advantage over merging in check_module_override (apart
>> from that needing to be fixed)?
>>
>>> That said, I've realised that this patch isn't completely sufficient
>>> anyway; consider:
>>>
>>>     // m.cpp
>>>     module;
>>>     template <typename T> struct S;
>>>     export module M;
>>>     S(int) -> S<double>;
>>>
>>>     // x.cpp
>>>     template <typename T> struct S { S(int); };
>>>     import M;
>>>     int main() {
>>>       S s(10);  // should be S<double> s;
>>>     }
>>>
>>> This patch doesn't correctly handle this case yet, we need to also
>>> consider cases where only the deduction guide is in purview.
>>
>> Indeed.
>>
>> Jason
>>
> 
> Here's a small update to the patch which fixes the above bug by ensuring
> that dependencies are created for purview deduction guides rather than
> just being skipped entirely, and includes that testcase with dguide-3.
> 
> This patch is still using force-exported bindings rather than attempting
> to do anything more clever with name lookup just yet.
> 
> Bootstrapped and regtested (so far just modules.exp) on
> x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?
> 
> -- >8 --
> 
> Deduction guides are represented as 'normal' functions currently, and
> have no special handling in modules.  However, this causes some issues;
> by [temp.deduct.guide] a deduction guide is not found by normal name
> lookup and instead all reachable deduction guides for a class template
> should be considered, but this does not happen currently.
> 
> To solve this, this patch ensures that all deduction guides are
> considered exported to ensure that they are always visible to importers
> if they are reachable.  Another alternative here would be to add a new
> kind of "all reachable" flag to name lookup, but that is complicated by
> some difficulties in handling GM entities; this may be a better way to
> go if more kinds of entities end up needing this handling, however.
> 
> Another issue here is that because deduction guides are "unrelated"
> functions, they will usually get discarded from the GMF, so this patch
> ensures that when finding dependencies, GMF deduction guides will also
> have bindings created.  We do this in find_dependencies so that we don't
> unnecessarily create bindings for GMF deduction guides that are never
> reached; for consistency we do this for *all* deduction guides, not just
> GM ones.  We also make sure that the opposite (a deduction guide being
> the only purview reference to a GMF template) correctly marks it as
> reachable.
> 
> Finally, when merging deduction guides from multiple modules, the name
> lookup code may now return two-dimensional overload sets, so update
> callers to match.
> 
> As a small drive-by improvement this patch also updates the error pretty
> printing code to add a space before the '->' when printing a deduction
> guide, so we get 'S(int) -> S<int>' instead of 'S(int)-> S<int>'.
> 
> @@ -15158,6 +15218,10 @@ 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
> +			 reachable whenever their class template is.  */

I'd adjust "reachable" to "visible to name lookup"; they don't need to 
be exported to be reachable.  OK with that change.

Jason


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-08-06 21:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-16  2:29 [PATCH] c++/modules: Ensure deduction guides are always reachable [PR115231] Nathaniel Shead
2024-06-16  5:41 ` Nathaniel Shead
2024-07-07  2:21   ` Nathaniel Shead
2024-07-23 20:17 ` Jason Merrill
2024-07-26  4:52   ` Nathaniel Shead
2024-07-26 17:17     ` Jason Merrill
2024-07-27  1:41       ` Nathaniel Shead
2024-08-06 12:03       ` [PATCH v2] " Nathaniel Shead
2024-08-06 21:39         ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).