c++: Delete defaulted operator <=> if std::strong_ordering::equal doesn't convert to its rettype [PR118387]

Message ID Z4U30gN8lZ54SdHi@tucnak
State New
Headers
Series c++: Delete defaulted operator <=> if std::strong_ordering::equal doesn't convert to its rettype [PR118387] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 fail Test failed
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm fail Test failed

Commit Message

Jakub Jelinek Jan. 13, 2025, 3:57 p.m. UTC
  On Fri, Jan 10, 2025 at 12:04:53PM -0500, Jason Merrill wrote:
> > Note, the PR raises another problem.
> > If on the same testcase the B b; line is removed, we silently synthetize
> > operator<=> which will crash at runtime due to returning without a return
> > statement.  That is because the standard says that in that case
> > it should return static_cast<int>(std::strong_ordering::equal);
> > but I can't find anywhere wording which would say that if that isn't
> > valid, the function is deleted.
> > https://eel.is/c++draft/class.compare#class.spaceship-2.2
> > seems to talk just about cases where there are some members and their
> > comparison is invalid it is deleted, but here there are none and it
> > follows
> > https://eel.is/c++draft/class.compare#class.spaceship-3.sentence-2
> > So, we synthetize with tf_none, see the static_cast is invalid, don't
> > add error_mark_node statement silently, but as the function isn't deleted,
> > we just silently emit it.
> > Should the standard be amended to say that the operator should be deleted
> > even if it has no elements and the static cast from
> > https://eel.is/c++draft/class.compare#class.spaceship-3.sentence-2
> > ?
> 
> That seems pretty obviously what we want, and is what the other compilers
> implement.

So like this?

Will you handle the defect report (unless you think nothing needs to be
clarified), or should I file something?

2025-01-13  Jakub Jelinek  <jakub@redhat.com>

	PR c++/118387
	* method.cc (build_comparison_op): Set bad if
	std::strong_ordering::equal doesn't convert to rettype.

	* g++.dg/cpp2a/spaceship-err6.C: Expect another error.
	* g++.dg/cpp2a/spaceship-synth17.C: Likewise.
	* g++.dg/cpp2a/spaceship-synth-neg6.C: Likewise.
	* g++.dg/cpp2a/spaceship-synth-neg7.C: New test.



	Jakub
  

Comments

Jason Merrill Jan. 14, 2025, 1:35 a.m. UTC | #1
On 1/13/25 10:57 AM, Jakub Jelinek wrote:
> On Fri, Jan 10, 2025 at 12:04:53PM -0500, Jason Merrill wrote:
>>> Note, the PR raises another problem.
>>> If on the same testcase the B b; line is removed, we silently synthetize
>>> operator<=> which will crash at runtime due to returning without a return
>>> statement.  That is because the standard says that in that case
>>> it should return static_cast<int>(std::strong_ordering::equal);
>>> but I can't find anywhere wording which would say that if that isn't
>>> valid, the function is deleted.
>>> https://eel.is/c++draft/class.compare#class.spaceship-2.2
>>> seems to talk just about cases where there are some members and their
>>> comparison is invalid it is deleted, but here there are none and it
>>> follows
>>> https://eel.is/c++draft/class.compare#class.spaceship-3.sentence-2
>>> So, we synthetize with tf_none, see the static_cast is invalid, don't
>>> add error_mark_node statement silently, but as the function isn't deleted,
>>> we just silently emit it.
>>> Should the standard be amended to say that the operator should be deleted
>>> even if it has no elements and the static cast from
>>> https://eel.is/c++draft/class.compare#class.spaceship-3.sentence-2
>>> ?
>>
>> That seems pretty obviously what we want, and is what the other compilers
>> implement.
> 
> So like this?

I'd rather hoist the build_static_cast from later; the rules for static 
cast aren't quite the same as can_convert.  See:

>       if (defining)
>         {
>           tree val;
>           if (code == EQ_EXPR)
>             val = boolean_true_node;
>           else
>             {
>               tree seql = lookup_comparison_result (cc_strong_ordering,
>                                                     "equal", complain);
>               val = build_static_cast (input_location, rettype, seql,
>                                        complain);

Apparently this also needs to happen when !defining.

>             }
>           finish_return_stmt (val);
>         }

> Will you handle the defect report (unless you think nothing needs to be
> clarified), or should I file something?

I will.

> 2025-01-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/118387
> 	* method.cc (build_comparison_op): Set bad if
> 	std::strong_ordering::equal doesn't convert to rettype.
> 
> 	* g++.dg/cpp2a/spaceship-err6.C: Expect another error.
> 	* g++.dg/cpp2a/spaceship-synth17.C: Likewise.
> 	* g++.dg/cpp2a/spaceship-synth-neg6.C: Likewise.
> 	* g++.dg/cpp2a/spaceship-synth-neg7.C: New test.
> 
> --- gcc/cp/method.cc.jj	2025-01-11 21:58:05.387588681 +0100
> +++ gcc/cp/method.cc	2025-01-13 16:19:09.896650756 +0100
> @@ -1635,6 +1635,26 @@ build_comparison_op (tree fndecl, bool d
>   	  rettype = common_comparison_type (comps);
>   	  apply_deduced_return_type (fndecl, rettype);
>   	}
> +      else if (code == SPACESHIP_EXPR && cat_tag_for (rettype) == cc_last)
> +	{
> +	  /* The return value is ... and
> +	     static_cast<R>(std::strong_ordering::equal) otherwise.
> +	     Make sure to delete or diagnose if such a static cast is not
> +	     valid.  */
> +	  tree seql = lookup_comparison_result (cc_strong_ordering,
> +						"equal", complain);
> +	  if (seql == error_mark_node)
> +	    bad = true;
> +	  else if (!can_convert (rettype, TREE_TYPE (seql), complain))
> +	    {
> +	      if (complain & tf_error)
> +		error_at (info.loc,
> +			  "%<std::strong_ordering::equal%> does not convert "
> +			  "to %qD return type %qT",
> +			  fndecl, rettype);
> +	      bad = true;
> +	    }
> +	}
>         if (bad)
>   	{
>   	  DECL_DELETED_FN (fndecl) = true;
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-err6.C.jj	2021-04-14 19:19:14.050804249 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-err6.C	2025-01-13 16:30:13.613331069 +0100
> @@ -10,7 +10,7 @@ class MyClass
>   public:
>     MyClass(int value): mValue(value) {}
>   
> -  bool operator<=>(const MyClass&) const = default;
> +  bool operator<=>(const MyClass&) const = default;	// { dg-error "'std::strong_ordering::equal' does not convert to 'constexpr bool MyClass::operator<=>\\\(const MyClass&\\\) const' return type 'bool'" }
>   };
>   
>   int main()
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-synth17.C.jj	2025-01-11 21:58:05.460587663 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth17.C	2025-01-13 16:32:11.383677413 +0100
> @@ -8,7 +8,7 @@ struct B {};
>   struct A
>   {
>     B b;			// { dg-error "no match for 'operator<=>' in '\[^\n\r]*' \\\(operand types are 'B' and 'B'\\\)" }
> -  int operator<=> (const A &) const = default;
> +  int operator<=> (const A &) const = default;	// { dg-error "'std::strong_ordering::equal' does not convert to 'constexpr int A::operator<=>\\\(const A&\\\) const' return type 'int'" }
>   };
>   
>   int
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C.jj	2021-08-12 20:37:12.696473756 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C	2025-01-13 16:48:22.482043534 +0100
> @@ -5,7 +5,7 @@
>   
>   struct S {
>     int a;			// { dg-error "three-way comparison of 'S::a' has type 'std::strong_ordering', which does not convert to 'int\\*'" }
> -  int *operator<=>(const S&) const = default;
> +  int *operator<=>(const S&) const = default;	// { dg-error "'std::strong_ordering::equal' does not convert to 'constexpr int\\* S::operator<=>\\\(const S&\\\) const' return type 'int\\*'" }
>   };
>   
>   bool b = S{} < S{};		// { dg-error "use of deleted function 'constexpr int\\* S::operator<=>\\\(const S&\\\) const'" }
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg7.C.jj	2025-01-13 16:19:09.897650742 +0100
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg7.C	2025-01-13 16:51:00.093831428 +0100
> @@ -0,0 +1,58 @@
> +// PR c++/118387
> +// { dg-do compile { target c++20 } }
> +
> +#include <compare>
> +
> +struct A {
> +  int operator<=> (const A &) const;
> +};
> +
> +struct B {
> +  A a;
> +  int operator<=> (const B &) const = default;	// { dg-message "'constexpr int B::operator<=>\\\(const B&\\\) const' is implicitly deleted because the default definition would be ill-formed:" }
> +};			// { dg-error "std::strong_ordering::equal' does not convert to 'constexpr int B::operator<=>\\\(const B&\\\) const' return type 'int'" "" { target *-*-* } .-1 }
> +
> +struct C {
> +  int operator<=> (const C &) const = default;	// { dg-message "'constexpr int C::operator<=>\\\(const C&\\\) const' is implicitly deleted because the default definition would be ill-formed:" }
> +};			// { dg-error "std::strong_ordering::equal' does not convert to 'constexpr int C::operator<=>\\\(const C&\\\) const' return type 'int'" "" { target *-*-* } .-1 }
> +
> +struct D {
> +  auto operator<=> (const D &) const = default;
> +};
> +
> +struct E {
> +  D a;			// { dg-error "three-way comparison of 'E::a' has type 'std::strong_ordering', which does not convert to 'int'" }
> +  int operator<=> (const E &) const = default;	// { dg-message "'constexpr int E::operator<=>\\\(const E&\\\) const' is implicitly deleted because the default definition would be ill-formed:" }
> +};			// { dg-error "std::strong_ordering::equal' does not convert to 'constexpr int E::operator<=>\\\(const E&\\\) const' return type 'int'" "" { target *-*-* } .-1 }
> +
> +struct F {
> +  A a;
> +  int operator<=> (const F &) const = default;
> +};
> +
> +struct G {
> +  int operator<=> (const G &) const = default;
> +};
> +
> +struct H {
> +  D a;
> +  int operator<=> (const H &) const = default;
> +};
> +
> +auto
> +foo (B a, B b)
> +{
> +  return a <=> b;	// { dg-error "use of deleted function 'constexpr int B::operator<=>\\\(const B&\\\) const'" }
> +}
> +
> +auto
> +bar (C a, C b)
> +{
> +  return a <=> b;	// { dg-error "use of deleted function 'constexpr int C::operator<=>\\\(const C&\\\) const'" }
> +}
> +
> +auto
> +baz (E a, E b)
> +{
> +  return a <=> b;	// { dg-error "use of deleted function 'constexpr int E::operator<=>\\\(const E&\\\) const'" }
> +}
> 
> 
> 	Jakub
>
  

Patch

--- gcc/cp/method.cc.jj	2025-01-11 21:58:05.387588681 +0100
+++ gcc/cp/method.cc	2025-01-13 16:19:09.896650756 +0100
@@ -1635,6 +1635,26 @@  build_comparison_op (tree fndecl, bool d
 	  rettype = common_comparison_type (comps);
 	  apply_deduced_return_type (fndecl, rettype);
 	}
+      else if (code == SPACESHIP_EXPR && cat_tag_for (rettype) == cc_last)
+	{
+	  /* The return value is ... and
+	     static_cast<R>(std::strong_ordering::equal) otherwise.
+	     Make sure to delete or diagnose if such a static cast is not
+	     valid.  */
+	  tree seql = lookup_comparison_result (cc_strong_ordering,
+						"equal", complain);
+	  if (seql == error_mark_node)
+	    bad = true;
+	  else if (!can_convert (rettype, TREE_TYPE (seql), complain))
+	    {
+	      if (complain & tf_error)
+		error_at (info.loc,
+			  "%<std::strong_ordering::equal%> does not convert "
+			  "to %qD return type %qT",
+			  fndecl, rettype);
+	      bad = true;
+	    }
+	}
       if (bad)
 	{
 	  DECL_DELETED_FN (fndecl) = true;
--- gcc/testsuite/g++.dg/cpp2a/spaceship-err6.C.jj	2021-04-14 19:19:14.050804249 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-err6.C	2025-01-13 16:30:13.613331069 +0100
@@ -10,7 +10,7 @@  class MyClass
 public:
   MyClass(int value): mValue(value) {}
 
-  bool operator<=>(const MyClass&) const = default;
+  bool operator<=>(const MyClass&) const = default;	// { dg-error "'std::strong_ordering::equal' does not convert to 'constexpr bool MyClass::operator<=>\\\(const MyClass&\\\) const' return type 'bool'" }
 };
 
 int main()
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth17.C.jj	2025-01-11 21:58:05.460587663 +0100
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth17.C	2025-01-13 16:32:11.383677413 +0100
@@ -8,7 +8,7 @@  struct B {};
 struct A
 {
   B b;			// { dg-error "no match for 'operator<=>' in '\[^\n\r]*' \\\(operand types are 'B' and 'B'\\\)" }
-  int operator<=> (const A &) const = default;
+  int operator<=> (const A &) const = default;	// { dg-error "'std::strong_ordering::equal' does not convert to 'constexpr int A::operator<=>\\\(const A&\\\) const' return type 'int'" }
 };
 
 int
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C.jj	2021-08-12 20:37:12.696473756 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg6.C	2025-01-13 16:48:22.482043534 +0100
@@ -5,7 +5,7 @@ 
 
 struct S {
   int a;			// { dg-error "three-way comparison of 'S::a' has type 'std::strong_ordering', which does not convert to 'int\\*'" }
-  int *operator<=>(const S&) const = default;
+  int *operator<=>(const S&) const = default;	// { dg-error "'std::strong_ordering::equal' does not convert to 'constexpr int\\* S::operator<=>\\\(const S&\\\) const' return type 'int\\*'" }
 };
 
 bool b = S{} < S{};		// { dg-error "use of deleted function 'constexpr int\\* S::operator<=>\\\(const S&\\\) const'" }
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg7.C.jj	2025-01-13 16:19:09.897650742 +0100
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth-neg7.C	2025-01-13 16:51:00.093831428 +0100
@@ -0,0 +1,58 @@ 
+// PR c++/118387
+// { dg-do compile { target c++20 } }
+
+#include <compare>
+
+struct A {
+  int operator<=> (const A &) const;
+};
+
+struct B {
+  A a;
+  int operator<=> (const B &) const = default;	// { dg-message "'constexpr int B::operator<=>\\\(const B&\\\) const' is implicitly deleted because the default definition would be ill-formed:" }
+};			// { dg-error "std::strong_ordering::equal' does not convert to 'constexpr int B::operator<=>\\\(const B&\\\) const' return type 'int'" "" { target *-*-* } .-1 }
+
+struct C {
+  int operator<=> (const C &) const = default;	// { dg-message "'constexpr int C::operator<=>\\\(const C&\\\) const' is implicitly deleted because the default definition would be ill-formed:" }
+};			// { dg-error "std::strong_ordering::equal' does not convert to 'constexpr int C::operator<=>\\\(const C&\\\) const' return type 'int'" "" { target *-*-* } .-1 }
+
+struct D {
+  auto operator<=> (const D &) const = default;
+};
+
+struct E {
+  D a;			// { dg-error "three-way comparison of 'E::a' has type 'std::strong_ordering', which does not convert to 'int'" }
+  int operator<=> (const E &) const = default;	// { dg-message "'constexpr int E::operator<=>\\\(const E&\\\) const' is implicitly deleted because the default definition would be ill-formed:" }
+};			// { dg-error "std::strong_ordering::equal' does not convert to 'constexpr int E::operator<=>\\\(const E&\\\) const' return type 'int'" "" { target *-*-* } .-1 }
+
+struct F {
+  A a;
+  int operator<=> (const F &) const = default;
+};
+
+struct G {
+  int operator<=> (const G &) const = default;
+};
+
+struct H {
+  D a;
+  int operator<=> (const H &) const = default;
+};
+
+auto
+foo (B a, B b)
+{
+  return a <=> b;	// { dg-error "use of deleted function 'constexpr int B::operator<=>\\\(const B&\\\) const'" }
+}
+
+auto
+bar (C a, C b)
+{
+  return a <=> b;	// { dg-error "use of deleted function 'constexpr int C::operator<=>\\\(const C&\\\) const'" }
+}
+
+auto
+baz (E a, E b)
+{
+  return a <=> b;	// { dg-error "use of deleted function 'constexpr int E::operator<=>\\\(const E&\\\) const'" }
+}