https://gcc.gnu.org/g:c2e567a6edb563677107c40ce2ce67f78d294013
commit r16-3559-gc2e567a6edb563677107c40ce2ce67f78d294013
Author: Nathaniel Shead <nathanielosh...@gmail.com>
Date: Sun Aug 24 01:56:32 2025 +1000
c++/modules: Fix ADL [PR117658]
On looking again at [basic.lookup.argdep] p4, I believe GCC hasn't fully
implemented the wording here for ADL. This patch fixes two issues.
First, 4.3 indicates that a function exported from a named module should
be visible to ADL regardless of whether it's visible to normal name
lookup, as long as some restrictions are followed.
This patch implements this; for skipping declarations that "do not
appear in the TU containing the point of lookup" I don't think there's
anything special we need to do, as any declarations before the point of
lookup will be found in other ways anyway, and any remaining
declarations from the current TU cannot be seen regardless.
Secondly, currently we only add the exported functions along the
instantiation path of a lookup. But I don't think this is intended by
the current wording, so this patch adjusts that. I also clean up the
logic to do all different module processing in adl_namespace_fns so that
we don't duplicate work in traversing the module binding list
unnecessarily.
This new handling means we need to do some extra work to properly error
on overload sets containing TU-local entities (as this might actually
come up now!) but I'm leaving that for a later patch.
As a drive-by fix this also fixes an ICE for C++26 expansion statements
with finding the instantiation path.
PR c++/117658
gcc/cp/ChangeLog:
* cp-tree.h (get_originating_module): Adjust parameter names.
* module.cc (path_of_instantiation): Handle C++26 expansion
statements.
* name-lookup.cc (name_lookup::adl_namespace_fns): Handle
exported declarations attached to the same module of an
associated entity with the same innermost non-inline namespace,
and non-exported functions on the instantiation path.
(name_lookup::search_adl): Build mapping of namespace to modules
that associated entities are attached to; remove now-unneeded
instantiation path handling.
gcc/testsuite/ChangeLog:
* g++.dg/modules/adl-4_a.C: Test should pass.
* g++.dg/modules/adl-4_b.C: Test should pass.
* g++.dg/modules/adl-6_a.C: New test.
* g++.dg/modules/adl-6_b.C: New test.
* g++.dg/modules/adl-6_c.C: New test.
* g++.dg/modules/adl-7_a.C: New test.
* g++.dg/modules/adl-7_b.C: New test.
* g++.dg/modules/adl-7_c.C: New test.
* g++.dg/modules/adl-8_a.C: New test.
* g++.dg/modules/adl-8_b.C: New test.
* g++.dg/modules/adl-8_c.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Reviewed-by: Jason Merrill <ja...@redhat.com>
Diff:
---
gcc/cp/cp-tree.h | 2 +-
gcc/cp/module.cc | 3 +-
gcc/cp/name-lookup.cc | 143 +++++++++++++++++----------------
gcc/testsuite/g++.dg/modules/adl-4_a.C | 2 +-
gcc/testsuite/g++.dg/modules/adl-4_b.C | 7 +-
gcc/testsuite/g++.dg/modules/adl-6_a.C | 38 +++++++++
gcc/testsuite/g++.dg/modules/adl-6_b.C | 26 ++++++
gcc/testsuite/g++.dg/modules/adl-6_c.C | 36 +++++++++
gcc/testsuite/g++.dg/modules/adl-7_a.C | 18 +++++
gcc/testsuite/g++.dg/modules/adl-7_b.C | 8 ++
gcc/testsuite/g++.dg/modules/adl-7_c.C | 9 +++
gcc/testsuite/g++.dg/modules/adl-8_a.C | 23 ++++++
gcc/testsuite/g++.dg/modules/adl-8_b.C | 14 ++++
gcc/testsuite/g++.dg/modules/adl-8_c.C | 9 +++
14 files changed, 263 insertions(+), 75 deletions(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8520ca05556c..d531bcd833b3 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7745,7 +7745,7 @@ extern void module_add_import_initializers ();
/* Where the namespace-scope decl was originally declared. */
extern void set_originating_module (tree, bool friend_p = false);
extern tree get_originating_module_decl (tree) ATTRIBUTE_PURE;
-extern int get_originating_module (tree, bool for_mangle = false)
ATTRIBUTE_PURE;
+extern int get_originating_module (tree, bool global_m1 = false)
ATTRIBUTE_PURE;
extern unsigned get_importing_module (tree, bool = false) ATTRIBUTE_PURE;
extern void check_module_decl_linkage (tree);
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 0404eae6ce36..ee76dd863d71 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -20833,8 +20833,9 @@ path_of_instantiation (tinst_level *tinst, bitmap
*path_map_p)
{
gcc_checking_assert (modules_p ());
- if (!tinst)
+ if (!tinst || TREE_CODE (tinst->tldcl) == TEMPLATE_FOR_STMT)
{
+ gcc_assert (!tinst || !tinst->next);
/* Not inside an instantiation, just the regular case. */
*path_map_p = nullptr;
return get_import_bitmap ();
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index f5e80503678e..9a2d0747ba48 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -550,7 +550,7 @@ private:
void adl_class_only (tree);
void adl_namespace (tree);
void adl_class_fns (tree);
- void adl_namespace_fns (tree, bitmap);
+ void adl_namespace_fns (tree, bitmap, bitmap, bitmap);
public:
/* Search namespace + inlines + maybe usings as qualified lookup. */
@@ -1205,10 +1205,16 @@ name_lookup::add_fns (tree fns)
add_overload (fns);
}
-/* Add the overloaded fns of SCOPE. */
+/* Add the overloaded fns of SCOPE. IMPORTS is the list of visible modules
+ for this lookup. INST_PATH for dependent (2nd phase) ADL is the list of
+ modules on the instantiation context for this lookup, or otherwise NULL.
+ ASSOCS is the list of modules where this namespace shares an innermost
+ non-inline namespace with an associated entity attached to said module,
+ or NULL if there are none. */
void
-name_lookup::adl_namespace_fns (tree scope, bitmap imports)
+name_lookup::adl_namespace_fns (tree scope, bitmap imports,
+ bitmap inst_path, bitmap assocs)
{
if (tree *binding = find_namespace_slot (scope, name))
{
@@ -1257,19 +1263,22 @@ name_lookup::adl_namespace_fns (tree scope, bitmap
imports)
for (; ix--; cluster++)
for (unsigned jx = 0; jx != BINDING_VECTOR_SLOTS_PER_CLUSTER; jx++)
{
+ int mod = cluster->indices[jx].base;
+
/* Functions are never on merged slots. */
- if (!cluster->indices[jx].base
- || cluster->indices[jx].span != 1)
+ if (!mod || cluster->indices[jx].span != 1)
continue;
- /* Is this slot visible? */
- if (!bitmap_bit_p (imports, cluster->indices[jx].base))
+ /* Is this slot accessible here? */
+ bool visible = bitmap_bit_p (imports, mod);
+ bool on_inst_path = inst_path && bitmap_bit_p (inst_path, mod);
+ if (!visible && !on_inst_path
+ && !(assocs && bitmap_bit_p (assocs, mod)))
continue;
- /* Is it loaded. */
+ /* Is it loaded? */
if (cluster->slots[jx].is_lazy ())
- lazy_load_binding (cluster->indices[jx].base,
- scope, name, &cluster->slots[jx]);
+ lazy_load_binding (mod, scope, name, &cluster->slots[jx]);
tree bind = cluster->slots[jx];
if (!bind)
@@ -1294,10 +1303,26 @@ name_lookup::adl_namespace_fns (tree scope, bitmap
imports)
dup_detect |= dup;
}
- bind = STAT_VISIBLE (bind);
+ /* For lookups on the instantiation path we can see any
+ module-linkage declaration; otherwise we should only
+ see exported decls. */
+ if (!on_inst_path)
+ bind = STAT_VISIBLE (bind);
}
- add_fns (bind);
+ if (on_inst_path || visible)
+ add_fns (bind);
+ else
+ {
+ /* We're only accessible because we're the same module as
+ an associated entity with module attachment: only add
+ functions actually attached to this module. */
+ for (tree fn : ovl_range (bind))
+ if (DECL_DECLARES_FUNCTION_P (fn)
+ && DECL_LANG_SPECIFIC (STRIP_TEMPLATE (fn))
+ && DECL_MODULE_ATTACH_P (STRIP_TEMPLATE (fn)))
+ add_overload (fn);
+ }
}
}
}
@@ -1632,6 +1657,32 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc>
*args)
if (fns)
dedup (true);
+ /* First get the attached modules for each innermost non-inline
+ namespace of an associated entity. */
+ bitmap_obstack_initialize (NULL);
+ hash_map<tree, bitmap> ns_mod_assocs;
+ if (modules_p ())
+ {
+ for (tree scope : scopes)
+ if (TYPE_P (scope))
+ {
+ int mod = get_originating_module (TYPE_NAME (scope),
+ /*global_m1=*/true);
+ if (mod > 0)
+ {
+ tree ctx = decl_namespace_context (scope);
+ while (DECL_NAMESPACE_INLINE_P (ctx))
+ ctx = CP_DECL_CONTEXT (ctx);
+
+ bool existed = false;
+ bitmap &b = ns_mod_assocs.get_or_insert (ctx, &existed);
+ if (!existed)
+ b = BITMAP_ALLOC (NULL);
+ bitmap_set_bit (b, mod);
+ }
+ }
+ }
+
/* INST_PATH will be NULL, if this is /not/ 2nd-phase ADL. */
bitmap inst_path = NULL;
/* VISIBLE is the regular import bitmap. */
@@ -1641,66 +1692,22 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc>
*args)
{
tree scope = (*scopes)[ix];
if (TREE_CODE (scope) == NAMESPACE_DECL)
- adl_namespace_fns (scope, visible);
- else
{
- if (RECORD_OR_UNION_TYPE_P (scope))
- adl_class_fns (scope);
-
- /* During 2nd phase ADL: Any exported declaration D in N
- declared within the purview of a named module M
- (10.2) is visible if there is an associated entity
- attached to M with the same innermost enclosing
- non-inline namespace as D.
- [basic.lookup.argdep]/4.4 */
-
- if (!inst_path)
- /* Not 2nd phase. */
- continue;
-
- tree ctx = CP_DECL_CONTEXT (TYPE_NAME (scope));
- if (TREE_CODE (ctx) != NAMESPACE_DECL)
- /* Not namespace-scope class. */
- continue;
-
- tree origin = get_originating_module_decl (TYPE_NAME (scope));
- tree not_tmpl = STRIP_TEMPLATE (origin);
- if (!DECL_LANG_SPECIFIC (not_tmpl)
- || !DECL_MODULE_IMPORT_P (not_tmpl))
- /* Not imported. */
- continue;
-
- unsigned module = get_importing_module (origin);
-
- if (!bitmap_bit_p (inst_path, module))
- /* Not on path of instantiation. */
- continue;
-
- if (bitmap_bit_p (visible, module))
- /* If the module was in the visible set, we'll look at
- its namespace partition anyway. */
- continue;
-
- if (tree *slot = find_namespace_slot (ctx, name, false))
- if (binding_slot *mslot = search_imported_binding_slot (slot,
module))
- {
- if (mslot->is_lazy ())
- lazy_load_binding (module, ctx, name, mslot);
-
- if (tree bind = *mslot)
- {
- /* We must turn on deduping, because some other class
- from this module might also be in this namespace. */
- dedup (true);
-
- /* Add the exported fns */
- if (STAT_HACK_P (bind))
- add_fns (STAT_VISIBLE (bind));
- }
- }
+ tree ctx = scope;
+ while (DECL_NAMESPACE_INLINE_P (ctx))
+ ctx = CP_DECL_CONTEXT (ctx);
+ bitmap *assocs = ns_mod_assocs.get (ctx);
+ adl_namespace_fns (scope, visible, inst_path,
+ assocs ? *assocs : NULL);
}
+ else if (RECORD_OR_UNION_TYPE_P (scope))
+ adl_class_fns (scope);
}
+ for (auto refs : ns_mod_assocs)
+ BITMAP_FREE (refs.second);
+ bitmap_obstack_release (NULL);
+
fns = value;
dedup (false);
}
diff --git a/gcc/testsuite/g++.dg/modules/adl-4_a.C
b/gcc/testsuite/g++.dg/modules/adl-4_a.C
index 5d956c057e20..f00ee55e1640 100644
--- a/gcc/testsuite/g++.dg/modules/adl-4_a.C
+++ b/gcc/testsuite/g++.dg/modules/adl-4_a.C
@@ -3,7 +3,7 @@ export module inter;
// { dg-module-cmi inter }
namespace hidden {
-// not found via ADL
+
int fn (int x);
}
diff --git a/gcc/testsuite/g++.dg/modules/adl-4_b.C
b/gcc/testsuite/g++.dg/modules/adl-4_b.C
index aa1396f7ce2b..91c1d8a20478 100644
--- a/gcc/testsuite/g++.dg/modules/adl-4_b.C
+++ b/gcc/testsuite/g++.dg/modules/adl-4_b.C
@@ -25,12 +25,11 @@ int main ()
{
hidden::Y y(2);
- // unexported hidden::fn@inter is not visible from TPL@inter
+ // unexported hidden::fn@inter is visible from TPL@inter because it's a
+ // dependent name and so lookup is performed at each point on the
+ // instantiation context, including the point at the end of inter.
if (TPL (y) != -2)
return 2;
return 0;
}
-
-// ADL fails
-// { dg-regexp {[^\n]*/adl-4_a.C:14:[0-9]*: error: 'fn' was not declared in
this scope\n} }
diff --git a/gcc/testsuite/g++.dg/modules/adl-6_a.C
b/gcc/testsuite/g++.dg/modules/adl-6_a.C
new file mode 100644
index 000000000000..e97890369973
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-6_a.C
@@ -0,0 +1,38 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi M }
+
+export module M;
+
+namespace R {
+ export struct X {};
+ export void f(X);
+}
+
+namespace S {
+ export void f(R::X, R::X);
+}
+
+namespace I {
+ inline namespace A {
+ export struct Y {};
+ }
+ inline namespace B {
+ export void f(Y);
+ export extern "C++" void f(Y, int);
+ }
+ inline namespace C {
+ export struct f {} f;
+ }
+}
+
+namespace O {
+ export void g(I::Y);
+ export extern "C++" void h(I::Y);
+}
+namespace I {
+ inline namespace D {
+ export using O::g;
+ export using O::h;
+ }
+}
diff --git a/gcc/testsuite/g++.dg/modules/adl-6_b.C
b/gcc/testsuite/g++.dg/modules/adl-6_b.C
new file mode 100644
index 000000000000..a373b4bebf8c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-6_b.C
@@ -0,0 +1,26 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi N }
+
+export module N;
+import M;
+
+export R::X make();
+
+namespace R {
+ static int g(X);
+}
+
+export
+template<typename T, typename U>
+void apply_ok(T t, U u) {
+ f(t, u);
+}
+
+export
+template<typename T>
+void apply_err(T t) {
+ g(t);
+}
+
+export I::Y make_Y();
diff --git a/gcc/testsuite/g++.dg/modules/adl-6_c.C
b/gcc/testsuite/g++.dg/modules/adl-6_c.C
new file mode 100644
index 000000000000..99b6c4c043e8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-6_c.C
@@ -0,0 +1,36 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+
+import N;
+
+namespace S {
+ struct Z { template<typename T> operator T(); };
+}
+
+void test() {
+ auto x = make(); // OK, decltype(x) is R::X in module M
+
+ R::f(x); // error: R and R::f are not visible here
+ // { dg-error "" "" { target *-*-* } .-1 }
+
+ f(x); // OK, calls R::f from interface of M
+
+ f(x, S::Z()); // error: S::f in module M not considered even though S is an
associated namespace
+ // { dg-error "" "" { target *-*-* } .-1 }
+
+ apply_ok(x, S::Z()); // OK, S::f is visible in instantiation context
+
+ apply_err(x); // error: R::g has internal linkage and cannot be used
outside N
+ // { dg-message "here" "" { target *-*-* } .-1 }
+ // { dg-error "'g'" "" { target *-*-* } 0 }
+
+ auto y = make_Y();
+ f(y); // OK, I::B::f and I::A::Y have matching innermost non-inline
namespace
+ g(y); // OK, O::g is accessible through I::D::g
+
+ f(y, 0); // error: I::B::f(Y, int) is not attached to M
+ // { dg-error "" "" { target *-*-* } .-1 }
+
+ h(y); // error: O::h is also not attached to M
+ // { dg-error "" "" { target *-*-* } .-1 }
+}
diff --git a/gcc/testsuite/g++.dg/modules/adl-7_a.C
b/gcc/testsuite/g++.dg/modules/adl-7_a.C
new file mode 100644
index 000000000000..50c2432bc132
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-7_a.C
@@ -0,0 +1,18 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi A }
+
+export module A;
+
+export template <typename T> void test(T t) { foo(t); }
+
+namespace ns1 {
+ export struct S {};
+ /* not exported */ void foo(S) {}
+}
+
+namespace ns2 {
+ export template <typename T> struct S {
+ friend void foo(S) {}
+ };
+}
diff --git a/gcc/testsuite/g++.dg/modules/adl-7_b.C
b/gcc/testsuite/g++.dg/modules/adl-7_b.C
new file mode 100644
index 000000000000..5c615c0cfac9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-7_b.C
@@ -0,0 +1,8 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+// { dg-module-cmi B }
+
+export module B;
+export import A;
+
+export using bar = ns2::S<int>;
diff --git a/gcc/testsuite/g++.dg/modules/adl-7_c.C
b/gcc/testsuite/g++.dg/modules/adl-7_c.C
new file mode 100644
index 000000000000..18d3915bd03b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-7_c.C
@@ -0,0 +1,9 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+
+import B;
+
+int main() {
+ ::test(ns1::S{});
+ ::test(bar{});
+}
diff --git a/gcc/testsuite/g++.dg/modules/adl-8_a.C
b/gcc/testsuite/g++.dg/modules/adl-8_a.C
new file mode 100644
index 000000000000..800d9f8bd820
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-8_a.C
@@ -0,0 +1,23 @@
+// C++26 expansion statements should not ICE
+// { dg-additional-options "-fmodules -std=c++26" }
+// { dg-module-cmi M }
+
+export module M;
+
+namespace ns {
+ export struct S {};
+ void foo(S) {}
+}
+
+export void test1() {
+ template for (auto x : { ns::S{} }) {
+ foo(x);
+ }
+}
+
+export template <typename T>
+void test2(T t) {
+ template for (auto x : { t, t, t }) {
+ foo(x);
+ }
+}
diff --git a/gcc/testsuite/g++.dg/modules/adl-8_b.C
b/gcc/testsuite/g++.dg/modules/adl-8_b.C
new file mode 100644
index 000000000000..77049a200b98
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-8_b.C
@@ -0,0 +1,14 @@
+// { dg-additional-options "-fmodules -std=c++26" }
+
+export module X;
+export import M;
+
+export template <typename T>
+void test3(T t) {
+ test2(t);
+}
+
+namespace other {
+ export struct S {};
+ void foo(S) {}
+}
diff --git a/gcc/testsuite/g++.dg/modules/adl-8_c.C
b/gcc/testsuite/g++.dg/modules/adl-8_c.C
new file mode 100644
index 000000000000..0d64ac3be470
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-8_c.C
@@ -0,0 +1,9 @@
+// { dg-additional-options "-fmodules -std=c++26" }
+
+import X;
+
+int main() {
+ test1();
+ test2(ns::S{});
+ test3(other::S{});
+}
