[PATCH] c++: more name lookup for non-dep rewritten cmp ops
Patrick Palka
ppalka@redhat.com
Wed Jul 23 20:10:57 GMT 2025
On Wed, 23 Jul 2025, Patrick Palka wrote:
> As a follow-up to r16-2448-g7590c14b53a762, this patch attempts to teach
> build_min_non_dep_op_overload how to rebuild all rewritten comparison
> operators, not just != -> == ones, so that we don't incorrectly repeat
> the unqualified name lookup at instantiation time.
>
> While working on this I noticed we'll seemingly never create a rewritten
> operator expression that is in terms of a built-in operator, since we
> could have used a built-in operator directly in the first place, which
> simplifies things. I think this also means the extract_call_expr
> handling of rewritten operators is wrong since it inspects for LT_EXPR,
> SPACESHIP_EXPR etc directly, so this patch just removes it in passing.
>
> gcc/cp/ChangeLog:
>
> * call.cc (build_new_op): If the selected candidate is
> rewritten, communicate the LOOKUP_REWRITTEN/REVERSED flags to
> the caller via the *overload out-parameter, and stop clearing
> *overload in that case.
> (extract_call_expr): Remove seemingly unnecessary and incorrect
> handling of C++20 rewritten comparison.
> * tree.cc (build_min_non_dep_op_overload): Handle rebuilding any
> C++20 rewritten comparison operator expressions.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/lookup/operator-8.C: Remove XFAILs and properly
> suppress all -Wunused-result warnings.
> ---
> gcc/cp/call.cc | 38 +++-------
> gcc/cp/tree.cc | 91 ++++++++++++++++++++----
> gcc/testsuite/g++.dg/lookup/operator-8.C | 16 +++--
> 3 files changed, 98 insertions(+), 47 deletions(-)
>
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index c925dd18ab41..7efbef736bc0 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -7486,7 +7486,16 @@ build_new_op (const op_location_t &loc, enum tree_code code, int flags,
> else if (TREE_CODE (cand->fn) == FUNCTION_DECL)
> {
> if (overload)
> - *overload = cand->fn;
> + {
> + if (cand->rewritten ())
> + /* build_min_non_dep_op_overload needs to know whether the
> + candidate is rewritten/reversed. */
> + *overload = build_tree_list (build_int_cst (integer_type_node,
> + cand->flags),
> + cand->fn);
> + else
> + *overload = cand->fn;
> + }
>
> if (resolve_args (arglist, complain) == NULL)
> result = error_mark_node;
> @@ -7535,11 +7544,6 @@ build_new_op (const op_location_t &loc, enum tree_code code, int flags,
> /* If this was a C++20 rewritten comparison, adjust the result. */
> if (cand->rewritten ())
> {
> - /* FIXME build_min_non_dep_op_overload can't handle rewrites. */
> - if (code == NE_EXPR && !cand->reversed ())
> - /* It can handle != rewritten to == though. */;
> - else if (overload)
> - *overload = NULL_TREE;
> switch (code)
> {
> case EQ_EXPR:
> @@ -7891,28 +7895,6 @@ extract_call_expr (tree call)
> call = TREE_OPERAND (call, 0);
> if (TREE_CODE (call) == TARGET_EXPR)
> call = TARGET_EXPR_INITIAL (call);
> - if (cxx_dialect >= cxx20)
> - switch (TREE_CODE (call))
> - {
> - /* C++20 rewritten comparison operators. */
> - case TRUTH_NOT_EXPR:
> - call = TREE_OPERAND (call, 0);
> - break;
> - case LT_EXPR:
> - case LE_EXPR:
> - case GT_EXPR:
> - case GE_EXPR:
> - case SPACESHIP_EXPR:
> - {
> - tree op0 = TREE_OPERAND (call, 0);
> - if (integer_zerop (op0))
> - call = TREE_OPERAND (call, 1);
> - else
> - call = op0;
> - }
> - break;
> - default:;
> - }
>
> if (TREE_CODE (call) != CALL_EXPR
> && TREE_CODE (call) != AGGR_INIT_EXPR
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index c260efb7f6ba..2bed60f8c949 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -3696,7 +3696,64 @@ build_min_non_dep_op_overload (enum tree_code op,
> int nargs, expected_nargs;
> tree fn, call, obj = NULL_TREE;
>
> - bool negated = (TREE_CODE (non_dep) == TRUTH_NOT_EXPR);
> + releasing_vec args;
> + va_start (p, overload);
> +
> + bool negated = false, rewritten = false, reversed = false;
> + if (cxx_dialect >= cxx20 && TREE_CODE (overload) == TREE_LIST)
> + {
> + /* Handle rebuilding a C++20 rewritten comparison operator expression,
> + e.g. !(x == y), y <=> x, (x <=> y) @ 0, etc, that resolved to a call
> + to a user-defined operator<=>/==. */
> + gcc_checking_assert (TREE_CODE_CLASS (op) == tcc_comparison
> + || op == SPACESHIP_EXPR);
> + int flags = TREE_INT_CST_LOW (TREE_PURPOSE (overload));
> + if (TREE_CODE (non_dep) == TRUTH_NOT_EXPR)
> + {
> + negated = true;
> + non_dep = TREE_OPERAND (non_dep, 0);
> + }
> + if (flags & LOOKUP_REWRITTEN)
> + rewritten = true;
> + if (flags & LOOKUP_REVERSED)
> + reversed = true;
> + if (rewritten
> + && DECL_OVERLOADED_OPERATOR_IS (TREE_VALUE (overload),
> + SPACESHIP_EXPR))
> + {
> + /* Handle (x <=> y) @ 0 and 0 @ (y <=> x) by recursing to first
> + rebuild the <=> and then rebuilding the outer operator expression.
> + Note that OVERLOAD in this case is the selected operator<=>, and
> + the provided variadic arguments are for this <=>. */
> +
> + /* Clear LOOKUP_REWRITTEN for the recursive call. */
> + TREE_PURPOSE (overload)
> + = int_const_binop (BIT_AND_EXPR,
> + TREE_PURPOSE (overload),
> + build_int_cst (integer_type_node,
> + ~LOOKUP_REWRITTEN));
On second thought we should clear both LOOKUP_REWRITTEN and
LOOKUP_REVERSED in the recursive call since the latter is effectively a
sub-flag of the former. It means we need to std::swap spaceship_op0
and spaceship_op1 in the parent call if 'rewritten', but overall I think
I prefer this. I'm retesting the following:
-- >8 --
Subject: [PATCH] c++: more name lookup for non-dep rewritten cmp ops
gcc/cp/ChangeLog:
* call.cc (build_new_op): If the selected candidate is
rewritten, communicate the LOOKUP_REWRITTEN/REVERSED flags to
the caller via the *overload out-parameter, and stop clearing
*overload in that case.
(extract_call_expr): Remove seemingly unnecessary and incorrect
handling of C++20 rewritten comparison.
* tree.cc (build_min_non_dep_op_overload): Handle rebuilding all
C++20 rewritten comparison operator expressions.
gcc/testsuite/ChangeLog:
* g++.dg/lookup/operator-8.C: Remove XFAILs and properly
suppress all -Wunused-result warnings.
---
gcc/cp/call.cc | 38 +++--------
gcc/cp/tree.cc | 85 ++++++++++++++++++++----
gcc/testsuite/g++.dg/lookup/operator-8.C | 16 +++--
3 files changed, 92 insertions(+), 47 deletions(-)
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index c925dd18ab41..7efbef736bc0 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -7486,7 +7486,16 @@ build_new_op (const op_location_t &loc, enum tree_code code, int flags,
else if (TREE_CODE (cand->fn) == FUNCTION_DECL)
{
if (overload)
- *overload = cand->fn;
+ {
+ if (cand->rewritten ())
+ /* build_min_non_dep_op_overload needs to know whether the
+ candidate is rewritten/reversed. */
+ *overload = build_tree_list (build_int_cst (integer_type_node,
+ cand->flags),
+ cand->fn);
+ else
+ *overload = cand->fn;
+ }
if (resolve_args (arglist, complain) == NULL)
result = error_mark_node;
@@ -7535,11 +7544,6 @@ build_new_op (const op_location_t &loc, enum tree_code code, int flags,
/* If this was a C++20 rewritten comparison, adjust the result. */
if (cand->rewritten ())
{
- /* FIXME build_min_non_dep_op_overload can't handle rewrites. */
- if (code == NE_EXPR && !cand->reversed ())
- /* It can handle != rewritten to == though. */;
- else if (overload)
- *overload = NULL_TREE;
switch (code)
{
case EQ_EXPR:
@@ -7891,28 +7895,6 @@ extract_call_expr (tree call)
call = TREE_OPERAND (call, 0);
if (TREE_CODE (call) == TARGET_EXPR)
call = TARGET_EXPR_INITIAL (call);
- if (cxx_dialect >= cxx20)
- switch (TREE_CODE (call))
- {
- /* C++20 rewritten comparison operators. */
- case TRUTH_NOT_EXPR:
- call = TREE_OPERAND (call, 0);
- break;
- case LT_EXPR:
- case LE_EXPR:
- case GT_EXPR:
- case GE_EXPR:
- case SPACESHIP_EXPR:
- {
- tree op0 = TREE_OPERAND (call, 0);
- if (integer_zerop (op0))
- call = TREE_OPERAND (call, 1);
- else
- call = op0;
- }
- break;
- default:;
- }
if (TREE_CODE (call) != CALL_EXPR
&& TREE_CODE (call) != AGGR_INIT_EXPR
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index c260efb7f6ba..50659c2de8be 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -3696,7 +3696,58 @@ build_min_non_dep_op_overload (enum tree_code op,
int nargs, expected_nargs;
tree fn, call, obj = NULL_TREE;
- bool negated = (TREE_CODE (non_dep) == TRUTH_NOT_EXPR);
+ releasing_vec args;
+ va_start (p, overload);
+
+ bool negated = false, rewritten = false, reversed = false;
+ if (cxx_dialect >= cxx20 && TREE_CODE (overload) == TREE_LIST)
+ {
+ /* Handle rebuilding a C++20 rewritten comparison operator expression,
+ e.g. !(x == y), y <=> x, (x <=> y) @ 0, etc, that resolved to a call
+ to a user-defined operator<=>/==. */
+ gcc_checking_assert (TREE_CODE_CLASS (op) == tcc_comparison
+ || op == SPACESHIP_EXPR);
+ int flags = TREE_INT_CST_LOW (TREE_PURPOSE (overload));
+ if (TREE_CODE (non_dep) == TRUTH_NOT_EXPR)
+ {
+ negated = true;
+ non_dep = TREE_OPERAND (non_dep, 0);
+ }
+ if (flags & LOOKUP_REWRITTEN)
+ rewritten = true;
+ if (flags & LOOKUP_REVERSED)
+ reversed = true;
+ if (rewritten
+ && DECL_OVERLOADED_OPERATOR_IS (TREE_VALUE (overload),
+ SPACESHIP_EXPR))
+ {
+ /* Handle (x <=> y) @ 0 and 0 @ (y <=> x) by recursing to first
+ rebuild the <=>. Note that both OVERLOAD and the provided arguments
+ in this case correspond to the selected operator<=>. */
+
+ tree spaceship_non_dep = CALL_EXPR_ARG (non_dep, reversed ? 1 : 0);
+ gcc_checking_assert (TREE_CODE (spaceship_non_dep) == CALL_EXPR);
+ tree spaceship_op0 = va_arg (p, tree);
+ tree spaceship_op1 = va_arg (p, tree);
+ if (reversed)
+ std::swap (spaceship_op0, spaceship_op1);
+
+ /* Push the correct arguments for the operator OP expression, and set
+ OVERLOAD appropriately. */
+ tree op0 = build_min_non_dep_op_overload (SPACESHIP_EXPR,
+ spaceship_non_dep,
+ TREE_VALUE (overload),
+ spaceship_op0,
+ spaceship_op1);
+ tree op1 = CALL_EXPR_ARG (non_dep, reversed ? 0 : 1);
+ gcc_checking_assert (integer_zerop (op1));
+ vec_safe_push (args, op0);
+ vec_safe_push (args, op1);
+ overload = CALL_EXPR_FN (non_dep);
+ }
+ else
+ overload = TREE_VALUE (overload);
+ }
non_dep = extract_call_expr (non_dep);
nargs = call_expr_nargs (non_dep);
@@ -3717,32 +3768,40 @@ build_min_non_dep_op_overload (enum tree_code op,
expected_nargs += 1;
gcc_assert (nargs == expected_nargs);
- releasing_vec args;
- va_start (p, overload);
-
if (!DECL_OBJECT_MEMBER_FUNCTION_P (overload))
{
fn = overload;
- if (op == ARRAY_REF)
- obj = va_arg (p, tree);
- for (int i = 0; i < nargs; i++)
+ if (vec_safe_length (args) != 0)
+ /* The correct arguments were already pushed above. */
+ gcc_checking_assert (rewritten);
+ else
{
- tree arg = va_arg (p, tree);
- vec_safe_push (args, arg);
+ if (op == ARRAY_REF)
+ obj = va_arg (p, tree);
+ for (int i = 0; i < nargs; i++)
+ {
+ tree arg = va_arg (p, tree);
+ vec_safe_push (args, arg);
+ }
}
+ if (reversed)
+ std::swap ((*args)[0], (*args)[1]);
}
else
{
+ gcc_checking_assert (vec_safe_length (args) == 0);
tree object = va_arg (p, tree);
- tree binfo = TYPE_BINFO (TREE_TYPE (object));
- tree method = build_baselink (binfo, binfo, overload, NULL_TREE);
- fn = build_min (COMPONENT_REF, TREE_TYPE (overload),
- object, method, NULL_TREE);
for (int i = 0; i < nargs; i++)
{
tree arg = va_arg (p, tree);
vec_safe_push (args, arg);
}
+ if (reversed)
+ std::swap (object, (*args)[0]);
+ tree binfo = TYPE_BINFO (TREE_TYPE (object));
+ tree method = build_baselink (binfo, binfo, overload, NULL_TREE);
+ fn = build_min (COMPONENT_REF, TREE_TYPE (overload),
+ object, method, NULL_TREE);
}
va_end (p);
diff --git a/gcc/testsuite/g++.dg/lookup/operator-8.C b/gcc/testsuite/g++.dg/lookup/operator-8.C
index 7fe6a57061bd..32d432dd8432 100644
--- a/gcc/testsuite/g++.dg/lookup/operator-8.C
+++ b/gcc/testsuite/g++.dg/lookup/operator-8.C
@@ -16,12 +16,16 @@ struct A {
template<class T>
void f() {
A a;
- (void)(a != 0); // We only handle this simple case, after PR121179
- (void)(0 != a); // { dg-bogus "deleted" "" { xfail *-*-* } }
- (void)(a < 0, 0 < a); // { dg-bogus "deleted" "" { xfail *-*-* } }
- (void)(a <= 0, 0 <= a); // { dg-bogus "deleted" "" { xfail *-*-* } }
- (void)(a > 0, 0 > a); // { dg-bogus "deleted" "" { xfail *-*-* } }
- (void)(a >= 0, 0 >= a); // { dg-bogus "deleted" "" { xfail *-*-* } }
+ (void)(a != 0);
+ (void)(0 != a);
+ (void)(a < 0);
+ (void)(0 < a);
+ (void)(a <= 0);
+ (void)(0 <= a);
+ (void)(a > 0);
+ (void)(0 > a);
+ (void)(a >= 0);
+ (void)(0 >= a);
}
// These later-declared namespace-scope overloads shouldn't be considered
--
2.50.1.319.g90c0775e97
More information about the Gcc-patches
mailing list