Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
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, except I'm not 100% sure what the impact of
skipping declarations that "do not appear in the TU containing the point
of lookup" is meant to result in, except maybe that we're not supposed
to consider exported functions later on in the current TU? So I don't
think there's anything to do there.
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-8_a.C: New test.
* g++.dg/modules/adl-8_b.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
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 | 12 +++
gcc/testsuite/g++.dg/modules/adl-7_b.C | 8 ++
gcc/testsuite/g++.dg/modules/adl-8_a.C | 23 ++++
gcc/testsuite/g++.dg/modules/adl-8_b.C | 8 ++
12 files changed, 233 insertions(+), 75 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/adl-6_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/adl-6_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/adl-6_c.C
create mode 100644 gcc/testsuite/g++.dg/modules/adl-7_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/adl-7_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/adl-8_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/adl-8_b.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 55e8e073627..b4900d367e1 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7746,7 +7746,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 9412f78ecd9..099e4f74811 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -20832,8 +20832,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 ba624677625..dca74e4821e 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 5d956c057e2..f00ee55e164 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 aa1396f7ce2..91c1d8a2047 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 00000000000..e9789036997
--- /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 00000000000..a373b4bebf8
--- /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 00000000000..99b6c4c043e
--- /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 00000000000..2595127a1cc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-7_a.C
@@ -0,0 +1,12 @@
+// 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 ns {
+ export struct S {};
+ /* not exported */ 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 00000000000..098902a0863
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-7_b.C
@@ -0,0 +1,8 @@
+// PR c++/117658
+// { dg-additional-options "-fmodules" }
+
+import A;
+
+int main() {
+ ::test(ns::S{});
+}
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 00000000000..800d9f8bd82
--- /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 00000000000..301350d5aa0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-8_b.C
@@ -0,0 +1,8 @@
+// { dg-additional-options "-fmodules -std=c++26" }
+
+import M;
+
+int main() {
+ test1();
+ test2(ns::S{});
+}
--
2.47.0
