https://gcc.gnu.org/g:28749321d32e513b9da3ab2f7ff51df8f9d3811f
commit r16-6311-g28749321d32e513b9da3ab2f7ff51df8f9d3811f Author: Nathaniel Shead <[email protected]> Date: Sat Nov 15 10:34:36 2025 +1100 c++: Implement dependent ADL for use with modules [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. PR c++/122712 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]> Reviewed-by: Jason Merrill <[email protected]> Diff: --- gcc/cp/module.cc | 129 ++++++++++++++++++++++++++++++++ gcc/cp/name-lookup.cc | 41 ++++++++-- gcc/cp/name-lookup.h | 3 +- gcc/testsuite/g++.dg/modules/adl-12_a.C | 94 +++++++++++++++++++++++ gcc/testsuite/g++.dg/modules/adl-12_b.C | 29 +++++++ 5 files changed, 287 insertions(+), 9 deletions(-) diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 85ace9c9b1d1..2ca381140ea2 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); @@ -9803,6 +9820,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. */ @@ -14980,6 +15000,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. */ @@ -15341,6 +15445,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 8fa5a0c246d7..8a7a81a9e324 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,12 @@ name_lookup::adl_class (tree type) if (found_p (type)) return; - complete_type (type); + /* Don't instantiate if we don't have to so we don't unnecessarily error + on incomplete types during modules streaming. This does however mean + we incorrectly miss some decl-reachable entities (PR c++/123235). */ + if (!tentative) + complete_type (type); + adl_bases (type); mark_found (type); @@ -1679,7 +1693,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 +1707,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 +1750,10 @@ 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) + /* We don't need to look at friends when searching for + new decl-reachable entities as they will already be + considered reachable by importers. */ adl_class_fns (scope); } @@ -1756,13 +1777,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 da277c49b1a6..2421446cb603 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 000000000000..c07ee2b99758 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/adl-12_a.C @@ -0,0 +1,94 @@ +// { dg-additional-options "-fmodules -Wno-global-module -fdump-lang-module" } +// { dg-module-cmi M } + +// Check we implement [module.global.frag] p2.3: +// 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. + +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*); +} +namespace C { + struct G {}; + void qux(int, void*); +} +template <typename T> struct H : C::G {}; +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 +} + +export template <typename T> +void needs_completion(T t) { + H<int>* holder; + // C::qux should be found via H<T>'s base, C::G. + // But we don't see this because we don't attempt to complete the type. + qux(t, holder); + // { dg-final { scan-lang-dump "Bindings '::C::qux'" module { xfail *-*-* } } } +} 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 000000000000..cde8c35b20e5 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/adl-12_b.C @@ -0,0 +1,29 @@ +// { 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 + needs_completion(0); // { dg-bogus "required from here" "PR123235" { xfail *-*-* } } + // { dg-prune-output "not declared in this scope" } + +#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" "PR122609" { xfail *-*-* } } +#endif +}
