c++/modules: Don't emit imported deduction guides [PR117397]

The ICE in the linked PR is caused because name lookup finds duplicate
copies of the deduction guides, causing a checking assert to fail.

This is ultimately because we're exporting an imported guide; when name
lookup processes 'dguide-5_b.H' it goes via the 'tt_entity' path and
just returns the entity from 'dguide-5_a.H'.  Because this doesn't ever
go through 'key_mergeable' we never set 'BINDING_VECTOR_GLOBAL_DUPS_P'
and so deduping is not engaged, allowing duplicate results.

Currently I believe this to be a perculiarity of the ANY_REACHABLE
handling for deduction guides; in no other case that I can find do we
emit bindings purely to imported entities.  As such, this patch fixes
this problem from that end, by ensuring that we simply do not emit any
imported deduction guides.  This avoids the ICE because no duplicates
need deduping to start with, and should otherwise have no functional
change because lookup of deduction guides will look at all reachable
modules (exported or not) regardless.

Since we're now deliberately not emitting imported deduction guides we
can use LOOK_want::NORMAL instead of LOOK_want::ANY_REACHABLE, since the
extra work to find as-yet undiscovered deduction guides in transitive
importers is not necessary here.

	PR c++/117397

gcc/cp/ChangeLog:

	* module.cc (depset::hash::add_deduction_guides): Don't emit
	imported deduction guides.
	(depset::hash::finalize_dependencies): Add check for any
	bindings referring to imported entities.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/dguide-5_a.H: New test.
	* g++.dg/modules/dguide-5_b.H: New test.
	* g++.dg/modules/dguide-5_c.H: New test.
	* g++.dg/modules/dguide-6.h: New test.
	* g++.dg/modules/dguide-6_a.C: New test.
	* g++.dg/modules/dguide-6_b.C: New test.
	* g++.dg/modules/dguide-6_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Jason Merrill <jason@redhat.com>

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 51990f36284717048c4d3824bfc7e6b67235469f..61116fe7669334f487a19d93e02531d44d7f3d1d 100644 (file)
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -14341,22 +14341,32 @@ depset::hash::add_deduction_guides (tree decl)
   if (find_binding (ns, name))
     return;
 
-  tree guides = lookup_qualified_name (ns, name, LOOK_want::ANY_REACHABLE,
+  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 *binding = nullptr;
+  for (tree t : lkp_range (guides))
+    {
+      gcc_checking_assert (!TREE_VISITED (t));
+      depset *dep = make_dependency (t, EK_FOR_BINDING);
 
-  depset **slot = binding_slot (ns, name, /*insert=*/true);
-  *slot = binding;
+      /* We don't want to create bindings for imported deduction guides, as
+        this would potentially cause name lookup to return duplicates.  */
+      if (dep->is_import ())
+       continue;
+
+      if (!binding)
+       {
+         /* We have bindings to add.  */
+         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);
     }
@@ -14592,6 +14602,11 @@ depset::hash::finalize_dependencies ()
          if (dep->deps.length () > 2)
            gcc_qsort (&dep->deps[1], dep->deps.length () - 1,
                       sizeof (dep->deps[1]), binding_cmp);
+
+         /* Bindings shouldn't refer to imported entities.  */
+         if (CHECKING_P)
+           for (depset *entity : dep->deps)
+             gcc_checking_assert (!entity->is_import ());
        }
       else if (dep->is_exposure () && !dep->is_tu_local ())
        {

diff --git a/gcc/testsuite/g++.dg/modules/dguide-5_a.H b/gcc/testsuite/g++.dg/modules/dguide-5_a.H
new file mode 100644 (file)
index 0000000..4242105
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-5_a.H
@@ -0,0 +1,6 @@
+// PR c++/117397
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+template <typename T> struct S;
+template <typename T> S(T) -> S<T>;

diff --git a/gcc/testsuite/g++.dg/modules/dguide-5_b.H b/gcc/testsuite/g++.dg/modules/dguide-5_b.H
new file mode 100644 (file)
index 0000000..d31f24d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-5_b.H
@@ -0,0 +1,6 @@
+// PR c++/117397
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+import "dguide-5_a.H";
+template <typename T> struct S;

diff --git a/gcc/testsuite/g++.dg/modules/dguide-5_c.H b/gcc/testsuite/g++.dg/modules/dguide-5_c.H
new file mode 100644 (file)
index 0000000..a9bf2dd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-5_c.H
@@ -0,0 +1,7 @@
+// PR c++/117397
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+template <typename T> struct S;
+import "dguide-5_b.H";
+S<int> foo();

diff --git a/gcc/testsuite/g++.dg/modules/dguide-6.h b/gcc/testsuite/g++.dg/modules/dguide-6.h
new file mode 100644 (file)
index 0000000..f204af4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-6.h
@@ -0,0 +1,4 @@
+template <typename T> struct S {
+  S(int);
+  S(int, int);
+};

diff --git a/gcc/testsuite/g++.dg/modules/dguide-6_a.C b/gcc/testsuite/g++.dg/modules/dguide-6_a.C
new file mode 100644 (file)
index 0000000..727336e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-6_a.C
@@ -0,0 +1,7 @@
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi M:a }
+
+module;
+#include "dguide-6.h"
+export module M:a;
+S(int) -> S<int>;

diff --git a/gcc/testsuite/g++.dg/modules/dguide-6_b.C b/gcc/testsuite/g++.dg/modules/dguide-6_b.C
new file mode 100644 (file)
index 0000000..b101d0e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-6_b.C
@@ -0,0 +1,8 @@
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi M }
+
+module;
+#include "dguide-6.h"
+export module M;
+export import :a;
+S(int, int) -> S<double>;

diff --git a/gcc/testsuite/g++.dg/modules/dguide-6_c.C b/gcc/testsuite/g++.dg/modules/dguide-6_c.C
new file mode 100644 (file)
index 0000000..5da934f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/dguide-6_c.C
@@ -0,0 +1,12 @@
+// { dg-additional-options "-fmodules" }
+
+#include "dguide-6.h"
+import M;
+
+int main() {
+  S a(1);
+  S<int> a_copy = a;
+
+  S b(2, 3);
+  S<double> b_copy = b;
+}