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?
-- >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 adds the result of the lookup to the
dependent nodes that are built, where modules streaming will look into
to find the dependencies on the relevant functions and mark as
reachable.
This causes us to do ADL in more circumstances; this means also that we
instantiate templates in cases we didn't use to. To prevent breaking
any existing code 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.
gcc/cp/ChangeLog:
* call.cc (add_operator_candidates): Mark non-tentative.
* cp-tree.h (build_dependent_operator_type): Add parameters.
* decl.cc (omp_declare_variant_finalize_one): Perform koenig
lookup in all cases.
* name-lookup.cc (name_lookup::preserve_state): Use the previous
state's tentative flag.
(name_lookup::adl_class): Don't perform instantiations during
tentative lookup.
(name_lookup::search_adl): Skip type-dependent expressions.
(lookup_arg_dependent): Add tentative parameter.
* name-lookup.h (lookup_arg_dependent): Add parameter.
* parser.cc (cp_parser_postfix_expression): Perform koenig
lookup in all cases.
* pt.cc (tsubst_expr): Likewise.
* semantics.cc (perform_koenig_lookup): Do ADL even for
dependent ops if producing a module CMI.
* typeck.cc (op_adl_lookup): New function.
(build_dependent_operator_type): Add parameters; store results
of ADL lookup.
(build_x_binary_op): Pass arguments.
(build_x_compound_expr): Likewise.
libcc1/ChangeLog:
* libcp1plugin.cc (plugin_build_call_expr): Perform koenig
lookup in all cases.
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/call.cc | 2 +-
gcc/cp/cp-tree.h | 4 +-
gcc/cp/decl.cc | 10 +--
gcc/cp/name-lookup.cc | 24 +++--
gcc/cp/name-lookup.h | 2 +-
gcc/cp/parser.cc | 14 ++-
gcc/cp/pt.cc | 6 +-
gcc/cp/semantics.cc | 19 ++--
gcc/cp/typeck.cc | 114 ++++++++++++++++++++++--
gcc/testsuite/g++.dg/modules/adl-12_a.C | 76 ++++++++++++++++
gcc/testsuite/g++.dg/modules/adl-12_b.C | 24 +++++
libcc1/libcp1plugin.cc | 2 +-
12 files changed, 252 insertions(+), 45 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/call.cc b/gcc/cp/call.cc
index f80d597b339..be194f252cb 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -7146,7 +7146,7 @@ add_operator_candidates (z_candidate **candidates,
fns = TREE_VALUE (found);
else
fns = NULL_TREE;
- fns = lookup_arg_dependent (fnname, fns, arglist);
+ fns = lookup_arg_dependent (fnname, fns, arglist, /*tentative=*/false);
add_candidates (fns, NULL_TREE, arglist, NULL_TREE,
NULL_TREE, false, NULL_TREE, NULL_TREE,
flags, candidates, complain);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 5e8d1c9644c..52206f0906c 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8543,7 +8543,9 @@ extern tree build_class_member_access_expr (cp_expr,
tree, tree, bool,
extern tree finish_class_member_access_expr (cp_expr, tree, bool,
tsubst_flags_t);
extern tree lookup_destructor (tree, tree, tree,
tsubst_flags_t);
-extern tree build_dependent_operator_type (tree, enum tree_code, bool);
+extern tree build_dependent_operator_type (tree, enum tree_code, bool,
+ tree = NULL_TREE,
+ tree = NULL_TREE);
extern tree build_x_indirect_ref (location_t, tree,
ref_operator, tree,
tsubst_flags_t);
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 5f990ea56b2..12cd26502ac 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -9049,9 +9049,8 @@ omp_declare_variant_finalize_one (tree decl, tree attr)
if (!args->is_empty ())
{
koenig_p = true;
- if (!any_type_dependent_arguments_p (args))
- variant = perform_koenig_lookup (variant, args,
- tf_warning_or_error);
+ variant = perform_koenig_lookup (variant, args,
+ tf_warning_or_error);
}
else
variant = unqualified_fn_lookup_error (variant);
@@ -9065,9 +9064,8 @@ omp_declare_variant_finalize_one (tree decl, tree attr)
|| DECL_LOCAL_DECL_P (fn)))
{
koenig_p = true;
- if (!any_type_dependent_arguments_p (args))
- variant = perform_koenig_lookup (variant, args,
- tf_warning_or_error);
+ variant = perform_koenig_lookup (variant, args,
+ tf_warning_or_error);
}
}
}
diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc
index 984d37c2089..02ce35d00cc 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; /* If this is just to find additional decl-reachable
+ entities. */
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);
@@ -1484,7 +1488,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 +1686,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 (!type_dependent_expression_p (arg))
adl_expr (arg);
if (vec_safe_length (scopes))
@@ -1756,13 +1766,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)
{
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..808a833a6dd 100644
--- a/gcc/cp/name-lookup.h
+++ b/gcc/cp/name-lookup.h
@@ -461,7 +461,7 @@ 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);
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/cp/parser.cc b/gcc/cp/parser.cc
index 06cba31ada6..88d2cb8957e 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -8457,10 +8457,9 @@ cp_parser_postfix_expression (cp_parser *parser, bool
address_p, bool cast_p,
if (!args->is_empty ())
{
koenig_p = true;
- if (!any_type_dependent_arguments_p (args))
- postfix_expression
- = perform_koenig_lookup (postfix_expression, args,
- complain);
+ postfix_expression
+ = perform_koenig_lookup (postfix_expression, args,
+ complain);
}
else
postfix_expression
@@ -8493,10 +8492,9 @@ cp_parser_postfix_expression (cp_parser *parser, bool
address_p, bool cast_p,
if (do_adl_p)
{
koenig_p = true;
- if (!any_type_dependent_arguments_p (args))
- postfix_expression
- = perform_koenig_lookup (postfix_expression, args,
- complain);
+ postfix_expression
+ = perform_koenig_lookup (postfix_expression, args,
+ complain);
}
}
}
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index bbbf49363e8..7621e11ead9 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -21746,11 +21746,7 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
|| identifier_p (function)
/* C++20 P0846: Lookup found nothing. */
|| (TREE_CODE (function) == TEMPLATE_ID_EXPR
- && identifier_p (TREE_OPERAND (function, 0))))
- /* Only do this when substitution turns a dependent call
- into a non-dependent call. */
- && type_dependent_expression_p_push (t)
- && !any_type_dependent_arguments_p (call_args))
+ && identifier_p (TREE_OPERAND (function, 0)))))
function = perform_koenig_lookup (function, call_args, tf_none);
if (function != NULL_TREE
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 3e19a56f51e..ef94269067f 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -3273,16 +3273,19 @@ perform_koenig_lookup (cp_expr fn_expr, vec<tree,
va_gc> *args,
/* A call to a namespace-scope function using an unqualified name.
- Do Koenig lookup -- unless any of the arguments are
- type-dependent. */
- if (!any_type_dependent_arguments_p (args)
- && !any_dependent_template_arguments_p (tmpl_args))
- {
- fn = lookup_arg_dependent (identifier, functions, args);
+ Do Koenig lookup -- unless any of the arguments are type-dependent.
+ But with modules we need to also do lookup to see if there are any
+ candidate functions we should not discard. */
+ bool any_dep = (any_type_dependent_arguments_p (args)
+ || any_dependent_template_arguments_p (tmpl_args));
+ if (CHECKING_P || module_maybe_has_cmi_p () || !any_dep)
+ {
+ fn = lookup_arg_dependent (identifier, functions, args, any_dep);
if (!fn)
{
- /* The unqualified name could not be resolved. */
- if (complain & tf_error)
+ /* The unqualified name could not be resolved
+ This is OK if we had any dependent args. */
+ if ((complain & tf_error) && !any_dep)
fn = unqualified_fn_lookup_error (cp_expr (identifier, loc));
else
fn = identifier;
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 2ab45f3fff6..d8692d772b4 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -3724,23 +3724,118 @@ op_unqualified_lookup (tree_code code, bool is_assign)
return build_tree_list (NULL_TREE, NULL_TREE);
}
+/* Perform ADL for a dependent operator, to collect any decl-reachable
+ entities that need to not be discarded in modules streaming.
+ LOOKUPS is the existing set of namespace-scope lookups for the op,
+ which will have an entry added if any lookups are collected. */
+
+static tree
+op_adl_lookup (tree lookups, tree_code code, tree arg1, tree arg2)
+{
+ gcc_checking_assert (lookups);
+
+ if (!module_maybe_has_cmi_p ())
+ /* If not building a module CMI we don't need these results. */
+ return lookups;
+
+ if (!arg1 || !arg2)
+ /* Only two-arg operators are interesting, otherwise will always
+ be fully dependent or not be overloadable. */
+ return lookups;
+
+ if (type_dependent_expression_p (arg1)
+ && type_dependent_expression_p (arg2))
+ /* We won't get any new results anyway. */
+ return lookups;
+
+ releasing_vec args = make_tree_vector ();
+ vec_safe_push (args, arg1);
+ vec_safe_push (args, arg2);
+
+ tree fnname = ovl_op_identifier (false, code);
+ tree fns = lookup_arg_dependent (fnname, NULL_TREE, args,
+ /*tentative=*/true);
+
+ /* Handle rewrites. We don't need to care about reversed argument
+ orders as the set of entities from ADL will remain the same. */
+ if (cxx_dialect >= cxx20)
+ {
+ tree_code rewrite_code = ERROR_MARK;
+ switch (code)
+ {
+ case LT_EXPR:
+ case LE_EXPR:
+ case GT_EXPR:
+ case GE_EXPR:
+ rewrite_code = SPACESHIP_EXPR;
+ break;
+
+ case NE_EXPR:
+ rewrite_code = EQ_EXPR;
+ break;
+
+ /* We also need to ensure that lookup of a matching NE_EXPR
+ can succeed if that would inhibit a rewrite. */
+ case EQ_EXPR:
+ rewrite_code = NE_EXPR;
+ break;
+
+ default:
+ break;
+ }
+
+ if (rewrite_code)
+ {
+ tree rewrite_name = ovl_op_identifier (rewrite_code);
+ tree rewritten_fns
+ = lookup_arg_dependent (rewrite_name, NULL_TREE, args,
+ /*tentative=*/true);
+ if (rewritten_fns)
+ fns = lookup_add (rewritten_fns, fns);
+ }
+ }
+
+ if (fns)
+ {
+ if (tree found = purpose_member (NULL_TREE, lookups))
+ TREE_VALUE (found) = fns;
+ else
+ {
+ tree type = TREE_TYPE (lookups);
+ TREE_TYPE (lookups) = NULL_TREE;
+ lookups = tree_cons (NULL_TREE, fns, lookups);
+ TREE_TYPE (lookups) = type;
+ }
+ }
+
+ return lookups;
+}
+
/* Create a DEPENDENT_OPERATOR_TYPE for a dependent operator expression of
the given operator. LOOKUPS, if non-NULL, is the result of phase 1
- name lookup for the given operator. */
+ name lookup for the given operator. ARG1 and ARG2 are the operands to
+ a binary operator that could have ADL functions that we should include,
+ to ensure we don't discard any decl-reachable entities in modules. */
tree
-build_dependent_operator_type (tree lookups, tree_code code, bool is_assign)
+build_dependent_operator_type (tree lookups, tree_code code, bool is_assign,
+ tree arg1, tree arg2)
{
if (lookups)
- /* We're partially instantiating a dependent operator expression, and
- LOOKUPS is the result of phase 1 name lookup that we performed
- earlier at template definition time, so just reuse the corresponding
- DEPENDENT_OPERATOR_TYPE. */
- return TREE_TYPE (lookups);
+ {
+ /* We're partially instantiating a dependent operator expression, and
+ LOOKUPS is the result of phase 1 name lookup that we performed
+ earlier at template definition time, so just reuse the corresponding
+ DEPENDENT_OPERATOR_TYPE. But replace the ADL lookups with the new
+ set we calculated above, in case we have new information. */
+ lookups = op_adl_lookup (lookups, code, arg1, arg2);
+ return TREE_TYPE (lookups);
+ }
/* Otherwise we're processing a dependent operator expression at template
definition time, so perform phase 1 name lookup now. */
lookups = op_unqualified_lookup (code, is_assign);
+ lookups = op_adl_lookup (lookups, code, arg1, arg2);
tree type = cxx_make_type (DEPENDENT_OPERATOR_TYPE);
DEPENDENT_OPERATOR_TYPE_SAVED_LOOKUPS (type) = lookups;
@@ -4854,7 +4949,7 @@ build_x_binary_op (const op_location_t &loc, enum
tree_code code, tree arg1,
{
expr = build_min_nt_loc (loc, code, arg1, arg2);
TREE_TYPE (expr)
- = build_dependent_operator_type (lookups, code, false);
+ = build_dependent_operator_type (lookups, code, false, arg1, arg2);
return expr;
}
}
@@ -8260,7 +8355,8 @@ build_x_compound_expr (location_t loc, tree op1, tree op2,
{
result = build_min_nt_loc (loc, COMPOUND_EXPR, op1, op2);
TREE_TYPE (result)
- = build_dependent_operator_type (lookups, COMPOUND_EXPR, false);
+ = build_dependent_operator_type (lookups, COMPOUND_EXPR, false,
+ op1, op2);
return result;
}
}
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..25fe5143875
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-12_a.C
@@ -0,0 +1,76 @@
+// { 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
+}
+
+extern "C++" struct Incomplete::Z {};
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..02211f461ef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/adl-12_b.C
@@ -0,0 +1,24 @@
+// { dg-additional-options "-fmodules" }
+// Example from [temp.dep.candidate]
+
+namespace Q {
+ struct X {};
+}
+
+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
+}
diff --git a/libcc1/libcp1plugin.cc b/libcc1/libcp1plugin.cc
index 3c5e78bfe87..60a1beb91fc 100644
--- a/libcc1/libcp1plugin.cc
+++ b/libcc1/libcp1plugin.cc
@@ -3141,7 +3141,7 @@ plugin_build_call_expr (cc1_plugin::connection *self,
}
}
- if (koenig_p && !any_type_dependent_arguments_p (args))
+ if (koenig_p)
callable = perform_koenig_lookup (callable, args, tf_none);
if (TREE_CODE (callable) == COMPONENT_REF)
--
2.51.0
