From 87ffd205fe0f4cbabd3d22cba815b02c0466d9ed Mon Sep 17 00:00:00 2001
From: Nathaniel Shead <nathanieloshead@gmail.com>
Date: Sun, 12 Jan 2025 04:00:56 +1100
Subject: [PATCH] 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>
---
gcc/cp/module.cc | 35 ++++++++++++++++-------
gcc/testsuite/g++.dg/modules/dguide-5_a.H | 6 ++++
gcc/testsuite/g++.dg/modules/dguide-5_b.H | 6 ++++
gcc/testsuite/g++.dg/modules/dguide-5_c.H | 7 +++++
gcc/testsuite/g++.dg/modules/dguide-6.h | 4 +++
gcc/testsuite/g++.dg/modules/dguide-6_a.C | 7 +++++
gcc/testsuite/g++.dg/modules/dguide-6_b.C | 8 ++++++
gcc/testsuite/g++.dg/modules/dguide-6_c.C | 12 ++++++++
8 files changed, 75 insertions(+), 10 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/dguide-5_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/dguide-5_b.H
create mode 100644 gcc/testsuite/g++.dg/modules/dguide-5_c.H
create mode 100644 gcc/testsuite/g++.dg/modules/dguide-6.h
create mode 100644 gcc/testsuite/g++.dg/modules/dguide-6_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/dguide-6_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/dguide-6_c.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 51990f362847..61116fe76693 100644
--- 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
index 000000000000..42421059c7f0
--- /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
index 000000000000..d31f24d54de9
--- /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
index 000000000000..a9bf2dd0583c
--- /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
index 000000000000..f204af49e8c7
--- /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
index 000000000000..727336edd90b
--- /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
index 000000000000..b101d0e16612
--- /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
index 000000000000..5da934fbade3
--- /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;
+}
--
GitLab