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