Re: [C++ PATCH] Implement P1946R0 - Allow defaulting comparisons by value

[Jason Merrill <jason at redhat dot com>]

On 11/11/19 8:07 AM, Jakub Jelinek wrote:
> Hi!
>
>  From https://www.reddit.com/r/cpp/comments/dtuov8/201911_belfast_iso_c_committee_trip_report/
> I understood P1946R0 made it into C++20, so here is my attempt at
> implementing it, you had most of it implemented anyway because
> in system headers
>      friend constexpr bool
>      operator==(partial_ordering, partial_ordering) noexcept = default;
> etc. has been already accepted.

And I implemented it the rest of the way at the meeting, I just hadn't 
committed it yet.  Thanks for the additional tests, though.

Tested x86_64-pc-linux-gnu, applying to trunk.

commit 0cbd94b2731032f1e0ec0a576f7b59cbcfded735
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Nov 8 21:57:44 2019 +0000

            Implement P1946R0, Allow defaulting comparisons by value.
    
            * method.c (early_check_defaulted_comparison): Accept by-value,
            reject mixed by-value and by-reference parms.
            * decl.c (grokdeclarator): Set funcdef_flag for defaulted friend.
            * decl2.c (grokfield): Don't SET_DECL_FRIEND_CONTEXT.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index caa04afe805..86e38f4af69 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13202,6 +13202,11 @@ grokdeclarator (const cp_declarator *declarator,
 	      ;  /* We already issued a permerror.  */
 	    else if (decl && DECL_NAME (decl))
 	      {
+		if (initialized)
+		  /* Kludge: We need funcdef_flag to be true in do_friend for
+		     in-class defaulted functions, but that breaks grokfndecl.
+		     So set it here.  */
+		  funcdef_flag = true;
 		if (template_class_depth (current_class_type) == 0)
 		  {
 		    decl = check_explicit_specialization
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 4f0b2161db1..f164494afdc 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -927,10 +927,6 @@ grokfield (const cp_declarator *declarator,
 	    }
 	  else if (init == ridpointers[(int)RID_DEFAULT])
 	    {
-	      if (friendp)
-		/* ??? do_friend doesn't set this because funcdef_flag is false
-		   for in-class defaulted functions.  So set it here.  */
-		SET_DECL_FRIEND_CONTEXT (value, current_class_type);
 	      if (defaultable_fn_check (value))
 		{
 		  DECL_DEFAULTED_FN (value) = 1;
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index 47441c10c52..acba6c6da8c 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1094,38 +1094,54 @@ early_check_defaulted_comparison (tree fn)
   if (!DECL_OVERLOADED_OPERATOR_IS (fn, SPACESHIP_EXPR)
       && !same_type_p (TREE_TYPE (TREE_TYPE (fn)), boolean_type_node))
     {
-      error_at (loc, "defaulted %qD must return %<bool%>", fn);
-      ok = false;
+      diagnostic_t kind = DK_UNSPECIFIED;
+      int opt = 0;
+      if (is_auto (TREE_TYPE (fn)))
+	kind = DK_PEDWARN;
+      else
+	kind = DK_ERROR;
+      emit_diagnostic (kind, loc, opt,
+		       "defaulted %qD must return %<bool%>", fn);
+      if (kind == DK_ERROR)
+	ok = false;
     }
 
-  int i = DECL_NONSTATIC_MEMBER_FUNCTION_P (fn);
-  if (i && type_memfn_quals (TREE_TYPE (fn)) != TYPE_QUAL_CONST)
+  bool mem = DECL_NONSTATIC_MEMBER_FUNCTION_P (fn);
+  if (mem && type_memfn_quals (TREE_TYPE (fn)) != TYPE_QUAL_CONST)
     {
       error_at (loc, "defaulted %qD must be %<const%>", fn);
       ok = false;
     }
   tree parmnode = FUNCTION_FIRST_USER_PARMTYPE (fn);
+  bool saw_byval = false;
+  bool saw_byref = mem;
+  bool saw_bad = false;
   for (; parmnode != void_list_node; parmnode = TREE_CHAIN (parmnode))
     {
-      ++i;
       tree parmtype = TREE_VALUE (parmnode);
-      diagnostic_t kind = DK_UNSPECIFIED;
-      int opt = 0;
       if (same_type_p (parmtype, ctx))
-	/* The draft specifies const reference, but let's also allow by-value
-	   unless -Wpedantic, hopefully it will be added soon. */
-	kind = DK_PEDWARN,
-	  opt = OPT_Wpedantic;
+	saw_byval = true;
       else if (TREE_CODE (parmtype) != REFERENCE_TYPE
 	       || TYPE_QUALS (TREE_TYPE (parmtype)) != TYPE_QUAL_CONST
 	       || !(same_type_ignoring_top_level_qualifiers_p
 		    (TREE_TYPE (parmtype), ctx)))
-	kind = DK_ERROR;
-      if (kind)
-	emit_diagnostic (kind, loc, opt, "defaulted %qD must have "
-			 "parameter type %<const %T&%>", fn, ctx);
-      if (kind == DK_ERROR)
-	ok = false;
+	saw_bad = true;
+      else
+	saw_byref = true;
+    }
+
+  if (saw_bad || (saw_byval && saw_byref))
+    {
+      if (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn))
+	error_at (loc, "defaulted member %qD must have parameter type "
+		  "%<const %T&%>", fn, ctx);
+      else if (saw_bad)
+	error_at (loc, "defaulted %qD must have parameters of either type "
+		  "%<const %T&%> or %qT", fn, ctx, ctx);
+      else
+	error_at (loc, "defaulted %qD must have parameters of either type "
+		  "%<const %T&%> or %qT, not both", fn, ctx, ctx);
+      ok = false;
     }
 
   /* We still need to deduce deleted/constexpr/noexcept and maybe return. */
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg1.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg1.C
new file mode 100644
index 00000000000..f9b2dc449e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg1.C
@@ -0,0 +1,29 @@
+// { dg-do compile { target c++2a } }
+
+#include <compare>
+
+template <class T>
+struct D
+{
+  T i;
+  auto operator<=>(D) const = default; // { dg-error "defaulted member" }
+  bool operator==(D) const = default; // { dg-error "defaulted member" }
+  bool operator!=(D) const = default; // { dg-error "defaulted member" }
+  bool operator<(D) const = default; // { dg-error "defaulted member" }
+  bool operator<=(D) const = default; // { dg-error "defaulted member" }
+  bool operator>(D) const = default; // { dg-error "defaulted member" }
+  bool operator>=(D) const = default; // { dg-error "defaulted member" }
+};
+
+template <class T>
+struct E
+{
+  T i;
+  friend auto operator<=>(const E&, E) = default; // { dg-error "not both" }
+  friend bool operator==(const E&, E) = default; // { dg-error "not both" }
+  friend bool operator!=(const E&, E) = default; // { dg-error "not both" }
+  friend bool operator<(E, const E&) = default; // { dg-error "not both" }
+  friend bool operator<=(E, const E&) = default; // { dg-error "not both" }
+  friend bool operator>(E, const E&) = default; // { dg-error "not both" }
+  friend bool operator>=(E, const E&) = default; // { dg-error "not both" }
+};
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth4.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth4.C
new file mode 100644
index 00000000000..033578122ae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth4.C
@@ -0,0 +1,43 @@
+// Test with all operators explicitly defaulted.
+// { dg-do run { target c++2a } }
+
+#include <compare>
+
+struct D
+{
+  int i;
+  friend auto operator<=>(const D& x, const D& y) = default;
+  friend bool operator==(const D& x, const D& y) = default;
+  friend bool operator!=(const D& x, const D& y) = default;
+  friend bool operator<(const D& x, const D& y) = default;
+  friend bool operator<=(const D& x, const D& y) = default;
+  friend bool operator>(const D& x, const D& y) = default;
+  friend bool operator>=(const D& x, const D& y) = default;
+};
+
+#define assert(X) do { if (!(X)) __builtin_abort(); } while (0)
+
+int main()
+{
+  D d{42};
+  D d2{24};
+
+  assert (is_eq (d <=> d));
+  assert (is_lteq (d <=> d));
+  assert (is_gteq (d <=> d));
+  assert (is_lt (d2 <=> d));
+  assert (is_lteq (d2 <=> d));
+  assert (is_gt (d <=> d2));
+  assert (is_gteq (d <=> d2));
+
+  assert (d == d);
+  assert (!(d2 == d));
+  assert (!(d == d2));
+  assert (d != d2);
+  assert (!(d2 != d2));
+
+  assert (d2 < d);
+  assert (d2 <= d);
+  assert (d > d2);
+  assert (d >= d2);
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth5.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth5.C
new file mode 100644
index 00000000000..8e4aa864b27
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth5.C
@@ -0,0 +1,43 @@
+// Test with all operators explicitly defaulted.
+// { dg-do run { target c++2a } }
+
+#include <compare>
+
+struct D
+{
+  int i;
+  friend auto operator<=>(D x, D y) = default;
+  friend bool operator==(D x, D y) = default;
+  friend bool operator!=(D x, D y) = default;
+  friend bool operator<(D x, D y) = default;
+  friend bool operator<=(D x, D y) = default;
+  friend bool operator>(D x, D y) = default;
+  friend bool operator>=(const D x, const D y) = default;
+};
+
+#define assert(X) do { if (!(X)) __builtin_abort(); } while (0)
+
+int main()
+{
+  D d{42};
+  D d2{24};
+
+  assert (is_eq (d <=> d));
+  assert (is_lteq (d <=> d));
+  assert (is_gteq (d <=> d));
+  assert (is_lt (d2 <=> d));
+  assert (is_lteq (d2 <=> d));
+  assert (is_gt (d <=> d2));
+  assert (is_gteq (d <=> d2));
+
+  assert (d == d);
+  assert (!(d2 == d));
+  assert (!(d == d2));
+  assert (d != d2);
+  assert (!(d2 != d2));
+
+  assert (d2 < d);
+  assert (d2 <= d);
+  assert (d > d2);
+  assert (d >= d2);
+}