Consider parm types equivalence for operator rewrite tiebreaker.

The C++ committee continues to discuss how best to avoid breaking existing
code with the new rules for reversed operators.  A recent suggestion was to
base the tie-breaker on the parameter types of the candidates, which made a
lot of sense to me, so this patch implements that.

This patch also mentions that a candidate was reversed or rewritten when
printing the list of candidates, and warns about a comparison that becomes
recursive under the new rules.  There is no flag for this warning; people
can silence it by swapping the operands.

	* call.c (same_fn_or_template): Change to cand_parms_match.
	(joust): Adjust.
	(print_z_candidate): Mark rewritten/reversed candidates.
	(build_new_op_1): Warn about recursive call with reversed arguments.

From-SVN: r278465

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index cc7a58ec5fd98f676a1a73c92c49c1ef2491789a..2313a824b488e98d316f2cf7f7de823d7061c2c5 100644 (file)
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,10 @@
+2019-11-12  Jason Merrill  <jason@redhat.com>
+
+       * call.c (same_fn_or_template): Change to cand_parms_match.
+       (joust): Adjust.
+       (print_z_candidate): Mark rewritten/reversed candidates.
+       (build_new_op_1): Warn about recursive call with reversed arguments.
+
 2019-11-15  Andrew Sutton  <asutton@lock3software.com>
 
        PR c++/89913

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 13639a1c9010b542eb0345393fafdff14bdae20c..f4dfa7b3f56bcb2a62a76df28f1e30f0d5414ce2 100644 (file)
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -3694,6 +3694,10 @@ print_z_candidate (location_t loc, const char *msgstr,
     inform (cloc, "%s%#qD (near match)", msg, fn);
   else if (DECL_DELETED_FN (fn))
     inform (cloc, "%s%#qD (deleted)", msg, fn);
+  else if (candidate->reversed ())
+    inform (cloc, "%s%#qD (reversed)", msg, fn);
+  else if (candidate->rewritten ())
+    inform (cloc, "%s%#qD (rewritten)", msg, fn);
   else
     inform (cloc, "%s%#qD", msg, fn);
   if (fn != candidate->fn)
@@ -6219,8 +6223,14 @@ build_new_op_1 (const op_location_t &loc, enum tree_code code, int flags,
          else
            {
              if (cand->reversed ())
-               /* We swapped these in add_candidate, swap them back now.  */
-               std::swap (cand->convs[0], cand->convs[1]);
+               {
+                 /* We swapped these in add_candidate, swap them back now.  */
+                 std::swap (cand->convs[0], cand->convs[1]);
+                 if (cand->fn == current_function_decl)
+                   warning_at (loc, 0, "in C++20 this comparison calls the "
+                               "current function recursively with reversed "
+                               "arguments");
+               }
              result = build_over_call (cand, LOOKUP_NORMAL, complain);
            }
 
@@ -10995,18 +11005,32 @@ joust_maybe_elide_copy (z_candidate *&cand)
   return false;
 }
 
-/* True if cand1 and cand2 represent the same function or function
-   template.  */
+/* True if the defining declarations of the two candidates have equivalent
+   parameters.  */
 
-static bool
-same_fn_or_template (z_candidate *cand1, z_candidate *cand2)
+bool
+cand_parms_match (z_candidate *c1, z_candidate *c2)
 {
-  if (cand1->fn == cand2->fn)
+  tree fn1 = c1->template_decl;
+  tree fn2 = c2->template_decl;
+  if (fn1 && fn2)
+    {
+      fn1 = most_general_template (TI_TEMPLATE (fn1));
+      fn1 = DECL_TEMPLATE_RESULT (fn1);
+      fn2 = most_general_template (TI_TEMPLATE (fn2));
+      fn2 = DECL_TEMPLATE_RESULT (fn2);
+    }
+  else
+    {
+      fn1 = c1->fn;
+      fn2 = c2->fn;
+    }
+  if (fn1 == fn2)
     return true;
-  if (!cand1->template_decl || !cand2->template_decl)
+  if (identifier_p (fn1) || identifier_p (fn2))
     return false;
-  return (most_general_template (TI_TEMPLATE (cand1->template_decl))
-         == most_general_template (TI_TEMPLATE (cand2->template_decl)));
+  return compparms (TYPE_ARG_TYPES (TREE_TYPE (fn1)),
+                   TYPE_ARG_TYPES (TREE_TYPE (fn2)));
 }
 
 /* Compare two candidates for overloading as described in
@@ -11155,20 +11179,11 @@ joust (struct z_candidate *cand1, struct z_candidate *cand2, bool warn,
 
          if (winner && comp != winner)
            {
-             if (same_fn_or_template (cand1, cand2))
-               {
-                 /* Ambiguity between normal and reversed versions of the
-                    same comparison operator; prefer the normal one.
-                    https://lists.isocpp.org/core/2019/10/7438.php  */
-                 if (cand1->reversed ())
-                   winner = -1;
-                 else
-                   {
-                     gcc_checking_assert (cand2->reversed ());
-                     winner = 1;
-                   }
-                 break;
-               }
+             /* Ambiguity between normal and reversed comparison operators
+                with the same parameter types; prefer the normal one.  */
+             if ((cand1->reversed () != cand2->reversed ())
+                 && cand_parms_match (cand1, cand2))
+               return cand1->reversed () ? -1 : 1;
 
              winner = 0;
              goto tweak;

diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite2.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite2.C
new file mode 100644 (file)
index 0000000..4f3d127
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite2.C
@@ -0,0 +1,12 @@
+// Test that C++20 overload changes don't break sloppy code.
+
+struct C {
+    bool operator==(const C&);
+    bool operator!=(const C&);
+};
+
+int main() {
+    C c1, c2;
+    (void)(c1 == c2);
+    (void)(c1 != c2);
+}

diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite3.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite3.C
new file mode 100644 (file)
index 0000000..ef29cff
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite3.C
@@ -0,0 +1,10 @@
+// Test that very different operators still cause ambiguity with reversed.
+
+struct X { operator int(); };
+bool operator==(X, int);    // #1 { dg-message "reversed" "" { target c++2a } }
+struct Y { operator int(); };
+bool operator==(Y, int);    // #2 { dg-message "reversed" "" { target c++2a } }
+
+X x; Y y;
+bool b1 = x == y;              // { dg-error "ambiguous" "" { target c++2a } }
+bool b2 = y == x;              // { dg-error "ambiguous" "" { target c++2a } }

diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite4.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite4.C
new file mode 100644 (file)
index 0000000..c068b5a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-rewrite4.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++11 } }
+
+struct iterator;
+struct const_iterator {
+  const_iterator(const iterator&);
+  bool operator==(const const_iterator &ci) const = delete;
+};
+struct iterator {
+  bool operator==(const const_iterator &ci) const {
+    return ci == *this;                // { dg-error "deleted" "" { target c++17_down } }
+  }                            // { dg-warning "reversed" "" { target c++2a } .-1 }
+};