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

Reply via email to