Re: [PATCH] c++: consider built-in operator candidates first

[Jason Merrill via Gcc-patches]

On 9/20/21 12:46, Patrick Palka wrote:
During operator overload resolution, we currently consider non-member
candidates before built-in candidates.  This didn't make a difference
before r12-3346, but after this change add_candidates will avoid
computing excess argument conversions if we've already seen a strictly
viable candidate, so it's better to consider built-in candidates first.

Doesn't r12-3346 stop considering conversions after it sees a bad one, 
and later return to the bad candidate if there is no strictly viable 
candidate?  How does this patch change that?

Depending on the order of the candidates seems fragile.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

        * call.c (add_operator_candidates): Consider built-in operator
        candidates before considering non-member candidates.

gcc/testsuite/ChangeLog:

        * g++.dg/template/conv17.C: Extend test.
---
  gcc/cp/call.c                          | 13 +++++++------
  gcc/testsuite/g++.dg/template/conv17.C |  7 +++++++
  2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index c5601d96ab8..c0da083758f 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -6321,7 +6321,6 @@ add_operator_candidates (z_candidate **candidates,
                         vec<tree, va_gc> *arglist,
                         int flags, tsubst_flags_t complain)
  {
-  z_candidate *start_candidates = *candidates;
    bool ismodop = code2 != ERROR_MARK;
    tree fnname = ovl_op_identifier (ismodop, ismodop ? code2 : code);
  
@@ -6333,6 +6332,12 @@ add_operator_candidates (z_candidate **candidates,
    if (rewritten && code != EQ_EXPR && code != SPACESHIP_EXPR)
      flags &= ~LOOKUP_REWRITTEN;
  
+  /* Add built-in candidates to the candidate set.  The standard says to
+     rewrite built-in candidates, too, but there's no point.  */
+  if (!rewritten)
+    add_builtin_candidates (candidates, code, code2, fnname, arglist,
+                           flags, complain);
+
    bool memonly = false;
    switch (code)
      {
@@ -6352,6 +6357,7 @@ add_operator_candidates (z_candidate **candidates,
  
    /* Add namespace-scope operators to the list of functions to
       consider.  */
+  z_candidate *start_candidates = *candidates;
    if (!memonly)
      {
        tree fns = lookup_name (fnname, LOOK_where::BLOCK_NAMESPACE);
@@ -6423,11 +6429,6 @@ add_operator_candidates (z_candidate **candidates,
  
    if (!rewritten)
      {
-      /* The standard says to rewrite built-in candidates, too,
-        but there's no point.  */
-      add_builtin_candidates (candidates, code, code2, fnname, arglist,
-                             flags, complain);
-
        /* Maybe add C++20 rewritten comparison candidates.  */
        tree_code rewrite_code = ERROR_MARK;
        if (cxx_dialect >= cxx20
diff --git a/gcc/testsuite/g++.dg/template/conv17.C 
b/gcc/testsuite/g++.dg/template/conv17.C
index f0f10f2ef4f..87ecefb8de3 100644
--- a/gcc/testsuite/g++.dg/template/conv17.C
+++ b/gcc/testsuite/g++.dg/template/conv17.C
@@ -61,3 +61,10 @@ concept E = requires { T().h(nullptr); };
  
  static_assert(!E<C>);
  #endif
+
+// Verify that the strictly viable built-in operator+ candidate precludes
+// us from computing all argument conversions for the below non-strictly
+// viable non-member candidate.
+enum N { n };
+int operator+(N&, B);
+int f = n + 42;