On Sat, Nov 15, 2025 at 12:11:39AM +1100, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>
> An alternative implementation would be to do the additional lookups in
> modules.cc during tree walk at the point we know we need to check for
> any CALL_EXPR with KOENIG_LOOKUP_P or DEPENDENT_OPERATOR_TYPE, and then
> discard the results.
>
> I felt this was the slightly neater approach but that would possibly be
> less risky; any thoughts?
Here's the other approach doing it just during module walk. I actually
think I prefer this approach.
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
-- >8 --
[module.global.frag] p3.3 says "A declaration D is decl-reachable from a
declaration S in the same translation unit if ... S contains a dependent
call E ([temp.dep]) and D is found by any name lookup performed for an
expression synthesized from E by replacing each type-dependent argument
or operand with a value of a placeholder type with no associated
namespaces or entities".
This requires doing partial ADL ondependent calls, in case there are
non-dependent arguments that would cause new functions to become
decl-reachable. This patch implements this with an additional lookup
during modules streaming to find any such entities.
This causes us to do ADL in more circumstances; this means also that we
might instantiate templates in cases we didn't use to. This could cause
issues given we have already started our modules walk at this point, or
break any otherwise valid existing code. To fix this patch adds a flag
to do a "tentative" ADL pass which doesn't attempt to complete any types
(and hence cause instantiations to occur); this means that we might miss
some associated entities however. During a tentative walk we can also
skip entities that we know won't contribute to the missing
decl-reachable set, as an optimisation.
One implementation limitation is that both modules tree walking and
name lookup marks tree nodes as TREE_VISITED for different purposes; to
avoid conflicts this patch caches calls that will require lookup in a
separate worklist to be processed after the walk is done.
gcc/cp/ChangeLog:
* module.cc (depset::hash::dep_adl_info): New type.
(depset::hash::dep_adl_entity_list): New work list.
(depset::hash::hash): Create it.
(depset::hash::~hash): Release it.
(trees_out::tree_value): Cache possibly dependent
calls during tree walk.
(depset::hash::add_dependent_adl_entities): New function.
(depset::hash::find_dependencies): Process cached entities.
* name-lookup.cc (name_lookup::tentative): New member.
(name_lookup::name_lookup): Initialize it.
(name_lookup::preserve_state): Propagate tentative from previous
lookup.
(name_lookup::adl_namespace_fns): Don't search imported bindings
during tentative lookup.
(name_lookup::adl_class): Don't attempt to complete class types
during tentative lookup.
(name_lookup::search_adl): Skip type-dependent args and avoid
unnecessary work during tentative lookup.
(lookup_arg_dependent): Add tentative parameter.
* name-lookup.h (lookup_arg_dependent): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/modules/adl-12_a.C: New test.
* g++.dg/modules/adl-12_b.C: New test.
Signed-off-by: Nathaniel Shead <[email protected]>
---
gcc/cp/module.cc | 129 ++++++++++++++++++++++++
gcc/cp/name-lookup.cc | 36 +++++--
gcc/cp/name-lookup.h | 3 +-
gcc/testsuite/g++.dg/modules/adl-12_a.C | 74 ++++++++++++++
gcc/testsuite/g++.dg/modules/adl-12_b.C | 27 +++++
5 files changed, 260 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/adl-12_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/adl-12_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ccabd640757..8889735c730 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2628,6 +2628,20 @@ public:
bool ignore_tu_local; /* In a context where referencing a TU-local
entity is not an exposure. */
+ private:
+ /* Information needed to do dependent ADL for discovering
+ more decl-reachable entities. Cached during walking to
+ prevent tree marking from interfering with lookup. */
+ struct dep_adl_info {
+ /* The name of the call or operator. */
+ tree name = NULL_TREE;
+ /* If not ERROR_MARK, a rewrite candidate for this operator. */
+ tree_code rewrite = ERROR_MARK;
+ /* Argument list for the call. */
+ vec<tree, va_gc>* args = make_tree_vector ();
+ };
+ vec<dep_adl_info> dep_adl_entity_list;
+
public:
hash (size_t size, hash *c = NULL)
: parent (size), chain (c), current (NULL), section (0),
@@ -2635,10 +2649,12 @@ public:
ignore_tu_local (false)
{
worklist.create (size);
+ dep_adl_entity_list.create (16);
}
~hash ()
{
worklist.release ();
+ dep_adl_entity_list.release ();
}
public:
@@ -2671,6 +2687,7 @@ public:
void add_specializations (bool decl_p);
void add_partial_entities (vec<tree, va_gc> *);
void add_class_entities (vec<tree, va_gc> *);
+ void add_dependent_adl_entities (tree expr);
private:
void add_deduction_guides (tree decl);
@@ -9771,6 +9788,9 @@ trees_out::tree_value (tree t)
&& !DECL_CONTEXT (t)))
&& TREE_CODE (t) != FUNCTION_DECL));
+ if (is_initial_scan () && EXPR_P (t))
+ dep_hash->add_dependent_adl_entities (t);
+
if (streaming_p ())
{
/* A new node -> tt_node. */
@@ -14913,6 +14933,90 @@ depset::hash::add_class_entities (vec<tree, va_gc>
*class_members)
}
}
+/* Add any entities found via dependent ADL. */
+
+void
+depset::hash::add_dependent_adl_entities (tree expr)
+{
+ gcc_checking_assert (!is_key_order ());
+ if (TREE_CODE (current->get_entity ()) != TEMPLATE_DECL)
+ return;
+
+ dep_adl_info info;
+ switch (TREE_CODE (expr))
+ {
+ case CALL_EXPR:
+ if (!KOENIG_LOOKUP_P (expr))
+ return;
+ info.name = CALL_EXPR_FN (expr);
+ if (!info.name)
+ return;
+ if (TREE_CODE (info.name) == TEMPLATE_ID_EXPR)
+ info.name = TREE_OPERAND (info.name, 0);
+ if (TREE_CODE (info.name) == TU_LOCAL_ENTITY)
+ return;
+ if (!identifier_p (info.name))
+ info.name = OVL_NAME (info.name);
+ for (int ix = 0; ix < call_expr_nargs (expr); ix++)
+ vec_safe_push (info.args, CALL_EXPR_ARG (expr, ix));
+ break;
+
+ case LE_EXPR:
+ case GE_EXPR:
+ case LT_EXPR:
+ case GT_EXPR:
+ info.rewrite = SPACESHIP_EXPR;
+ goto overloadable_expr;
+
+ case NE_EXPR:
+ info.rewrite = EQ_EXPR;
+ goto overloadable_expr;
+
+ case EQ_EXPR:
+ /* Not strictly a rewrite candidate, but we need to ensure
+ that lookup of a matching NE_EXPR can succeed if that
+ would inhibit a rewrite with reversed parameters. */
+ info.rewrite = NE_EXPR;
+ goto overloadable_expr;
+
+ case COMPOUND_EXPR:
+ case MEMBER_REF:
+ case MULT_EXPR:
+ case TRUNC_DIV_EXPR:
+ case TRUNC_MOD_EXPR:
+ case PLUS_EXPR:
+ case MINUS_EXPR:
+ case LSHIFT_EXPR:
+ case RSHIFT_EXPR:
+ case SPACESHIP_EXPR:
+ case BIT_AND_EXPR:
+ case BIT_XOR_EXPR:
+ case BIT_IOR_EXPR:
+ case TRUTH_ANDIF_EXPR:
+ case TRUTH_ORIF_EXPR:
+ overloadable_expr:
+ info.name = ovl_op_identifier (TREE_CODE (expr));
+ gcc_checking_assert (tree_operand_length (expr) == 2);
+ vec_safe_push (info.args, TREE_OPERAND (expr, 0));
+ vec_safe_push (info.args, TREE_OPERAND (expr, 1));
+ break;
+
+ default:
+ return;
+ }
+
+ /* If all arguments are type-dependent we don't need to do
+ anything further, we won't find new entities. */
+ processing_template_decl_sentinel ptds;
+ ++processing_template_decl;
+ if (!any_type_dependent_arguments_p (info.args))
+ return;
+
+ /* We need to defer name lookup until after walking, otherwise
+ we get confused by stray TREE_VISITEDs. */
+ dep_adl_entity_list.safe_push (info);
+}
+
/* We add the partial & explicit specializations, and the explicit
instantiations. */
@@ -15274,6 +15378,31 @@ depset::hash::find_dependencies (module_state *module)
&& deduction_guide_p (decl))
add_deduction_guides (TYPE_NAME (TREE_TYPE (TREE_TYPE (decl))));
+ /* Handle dependent ADL for [module.global.frag] p3.3. */
+ if (!is_key_order () && !dep_adl_entity_list.is_empty ())
+ {
+ processing_template_decl_sentinel ptds;
+ ++processing_template_decl;
+ for (auto &info : dep_adl_entity_list)
+ {
+ tree lookup = lookup_arg_dependent (info.name, NULL_TREE,
+ info.args, true);
+ for (tree fn : lkp_range (lookup))
+ add_dependency (make_dependency (fn, EK_DECL));
+
+ if (info.rewrite)
+ {
+ tree rewrite_name = ovl_op_identifier (info.rewrite);
+ lookup = lookup_arg_dependent (rewrite_name,
NULL_TREE,
+ info.args, true);
+ for (tree fn : lkp_range (lookup))
+ add_dependency (make_dependency (fn, EK_DECL));
+ }
+ release_tree_vector (info.args);
+ }
+ dep_adl_entity_list.truncate (0);
+ }
+
if (!is_key_order ()
&& TREE_CODE (decl) == TEMPLATE_DECL
&& !DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (decl))
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 984d37c2089..54743ad1761 100644
--- a/gcc/cp/name-lookup.cc
+++ b/gcc/cp/name-lookup.cc
@@ -449,6 +449,8 @@ public:
LOOK_want want; /* What kind of entity we want. */
+ bool tentative; /* The lookup is just to find additional decl-reachable
+ entities in this TU during modules streaming. */
bool deduping; /* Full deduping is needed because using declarations
are in play. */
vec<tree, va_heap, vl_embed> *scopes;
@@ -463,7 +465,7 @@ protected:
public:
name_lookup (tree n, LOOK_want w = LOOK_want::NORMAL)
: name (n), value (NULL_TREE), type (NULL_TREE),
- want (w),
+ want (w), tentative (false),
deduping (false), scopes (NULL), previous (NULL)
{
preserve_state ();
@@ -602,6 +604,8 @@ name_lookup::preserve_state ()
}
}
+ tentative = previous->tentative;
+
/* Unmark the outer partial lookup. */
if (previous->deduping)
lookup_mark (previous->value, false);
@@ -1252,6 +1256,11 @@ name_lookup::adl_namespace_fns (tree scope, bitmap
imports,
add_fns (ovl_skip_hidden (MAYBE_STAT_DECL (bind)));
}
+ /* When doing tentative name lookup we only care about entities
+ in the current TU. */
+ if (tentative)
+ return;
+
/* Scan the imported bindings. */
unsigned ix = BINDING_VECTOR_NUM_CLUSTERS (val);
if (BINDING_VECTOR_SLOTS_PER_CLUSTER == BINDING_SLOTS_FIXED)
@@ -1484,7 +1493,10 @@ name_lookup::adl_class (tree type)
if (found_p (type))
return;
- complete_type (type);
+ /* Don't instantiate if we don't have to. */
+ if (!tentative)
+ complete_type (type);
+
adl_bases (type);
mark_found (type);
@@ -1679,7 +1691,10 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc>
*args)
first arg. */
if (TYPE_P (arg))
adl_type (arg);
- else
+ /* When processing a module CMI we might get a type-dependent
+ argument: treat as a placeholder with no associated namespace
+ or entities. */
+ else if (!tentative || !type_dependent_expression_p (arg))
adl_expr (arg);
if (vec_safe_length (scopes))
@@ -1690,10 +1705,11 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc>
*args)
dedup (true);
/* First get the attached modules for each innermost non-inline
- namespace of an associated entity. */
+ namespace of an associated entity. This isn't needed for
+ tentative lookup, as we're only interested in the current TU. */
bitmap_obstack_initialize (NULL);
hash_map<tree, bitmap> ns_mod_assocs;
- if (modules_p ())
+ if (modules_p () && !tentative)
{
for (tree scope : scopes)
if (TYPE_P (scope))
@@ -1732,7 +1748,7 @@ name_lookup::search_adl (tree fns, vec<tree, va_gc> *args)
adl_namespace_fns (scope, visible, inst_path,
assocs ? *assocs : NULL);
}
- else if (RECORD_OR_UNION_TYPE_P (scope))
+ else if (RECORD_OR_UNION_TYPE_P (scope) && !tentative)
adl_class_fns (scope);
}
@@ -1756,13 +1772,17 @@ static void consider_binding_level (tree name,
/* ADL lookup of NAME. FNS is the result of regular lookup, and we
don't add duplicates to it. ARGS is the vector of call
- arguments (which will not be empty). */
+ arguments (which will not be empty). TENTATIVE is true when
+ this is early lookup only for the purpose of finding more
+ decl-reachable declarations. */
tree
-lookup_arg_dependent (tree name, tree fns, vec<tree, va_gc> *args)
+lookup_arg_dependent (tree name, tree fns, vec<tree, va_gc> *args,
+ bool tentative/*=false*/)
{
auto_cond_timevar tv (TV_NAME_LOOKUP);
name_lookup lookup (name);
+ lookup.tentative = tentative;
return lookup.search_adl (fns, args);
}
diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h
index 3815b8c1c96..46cbf8e1016 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -461,7 +461,8 @@ extern void push_decl_namespace (tree);
extern void pop_decl_namespace (void);
extern void do_namespace_alias (location_t, tree, tree);
extern tree do_class_using_decl (tree, tree);
-extern tree lookup_arg_dependent (tree, tree, vec<tree, va_gc> *);
+extern tree lookup_arg_dependent (tree, tree, vec<tree, va_gc> *,
+ bool tentative = false);
extern tree search_anon_aggr (tree, tree, bool = false);
extern tree get_class_binding_direct (tree, tree, bool want_type = false);
extern tree get_class_binding (tree, tree, bool want_type = false);
diff --git a/gcc/testsuite/g++.dg/modules/adl-12_a.C
b/gcc/testsuite/g++.dg/modules/adl-12_a.C
new file mode 100644
index 00000000000..47c3c21a2a5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-12_a.C
@@ -0,0 +1,74 @@
+// { dg-additional-options "-fmodules -Wno-global-module -fdump-lang-module" }
+// { dg-module-cmi M }
+// Check we implement [module.global.frag] p3.3.
+
+module;
+namespace Q {
+ struct X {};
+ void g_impl(X, X);
+ void operator-(X, X);
+ void go_partial(X, int);
+ void operator/(X, int);
+}
+namespace P {
+ struct X {};
+}
+#if __cpp_impl_three_way_comparison >= 201907L
+namespace ops1 {
+ struct Y {};
+ int operator<=>(Y, int);
+ bool operator==(Y, int);
+}
+namespace ops2 {
+ struct Y {};
+ bool operator==(Y, int);
+ bool operator!=(Y, int);
+}
+#endif
+namespace Incomplete {
+ template <typename T> struct Holder { T t; };
+ struct Z;
+ void go(int, void*);
+}
+export module M;
+
+export template <typename T>
+void g(T t) {
+ g_impl(t, Q::X{}); // ADL in definition finds Q::g_impl, g_impl not
discarded
+ // { dg-final { scan-lang-dump "Bindings '::Q::g_impl'" module } }
+
+ t - Q::X{}; // Similarly for Q::operator-
+ // { dg-final { scan-lang-dump "Bindings '::Q::operator-'" module } }
+}
+
+export template <typename T> struct Partial {
+ template <typename U> static decltype(go_partial(T{}, U{})) f();
+ template <typename U> static decltype(T{} / U{}) o();
+};
+// The instantantiation of Partial<Q::X> should emit go_partial and operator/
+template struct Partial<Q::X>;
+// { dg-final { scan-lang-dump "Bindings '::Q::go_partial'" module } }
+// { dg-final { scan-lang-dump "Bindings '::Q::operator/'" module } }
+
+#if __cpp_impl_three_way_comparison >= 201907L
+export template <typename T>
+void rewrite_ops(T t) {
+ // Rewritten operators should also look for the ops they rewrite to.
+ t < ops1::Y{};
+ t != ops1::Y{};
+ // { dg-final { scan-lang-dump {Bindings '::ops1::operator<=>'} module {
target c++20 } } }
+ // { dg-final { scan-lang-dump {Bindings '::ops1::operator=='} module {
target c++20 } } }
+}
+export template <typename T>
+void rewrite_ops_error(T t) {
+ // Test we bind != to prevent considering == as a rewrite target.
+ t == ops2::Y{};
+ // { dg-final { scan-lang-dump "Bindings '::ops2::operator!='" module {
target c++20 } } }
+}
+#endif
+
+export template <typename T>
+void incomplete(T t) {
+ Incomplete::Holder<Incomplete::Z>* holder;
+ go(t, holder); // Shouldn't attempt to instantiate unnecessarily here
+}
diff --git a/gcc/testsuite/g++.dg/modules/adl-12_b.C
b/gcc/testsuite/g++.dg/modules/adl-12_b.C
new file mode 100644
index 00000000000..fd960c9ae42
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-12_b.C
@@ -0,0 +1,27 @@
+// { dg-additional-options "-fmodules" }
+// Example from [temp.dep.candidate]
+
+namespace Q {
+ struct X {};
+}
+namespace Incomplete {
+ struct Z {};
+};
+
+import M;
+
+void test(Q::X x) {
+ g(x); // OK
+ Partial<Q::X>::f<int>();
+ Partial<Q::X>::o<int>();
+ incomplete(0); // OK
+
+#if __cpp_impl_three_way_comparison >= 201907L
+ rewrite_ops(0); // OK
+
+ // This should error, but lookup_qualified_name in add_candidates
+ // doesn't look in the instantiation context of this call, so
+ // we don't see the operator!= and think we can validly rewrite.
+ rewrite_ops_error(0); // { dg-error "required from here" "" { xfail *-*-* }
}
+#endif
+}
--
2.51.0
