[PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

Hi!

The testcases in the patch are either miscompiled or ICE with checking,
because the defaulted operator== is synthetized too early (but only if
constexpr), when the corresponding class type is still incomplete type.
The problem is that at that point the bitfield FIELD_DECLs still have as
TREE_TYPE their underlying type rather than integral type with their
precision and when layout_class_type is called for the class soon after
that, it changes those types but the COMPONENT_REFs type stay the way
that they were during the operator== synthetize_method type and the
middle-end is then upset by the mismatch of types.
As what exact type will be given isn't just a one liner but quite long code
especially for over-sized bitfields, I think it is best to just not
synthetize the comparison operators so early (the defaulted_late_check
change) and call defaulted_late_check for them once again as soon as the
class is complete.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-09-28  Jakub Jelinek  <jakub@redhat.com>

	PR c++/102490
	* method.c (defaulted_late_check): Don't synthetize constexpr
	defaulted comparisons if context is still incomplete type.
	(finish_struct_1): Call defaulted_late_check again for defaulted
	comparisons.

	* g++.dg/cpp2a/spaceship-eq11.C: New test.
	* g++.dg/cpp2a/spaceship-eq12.C: New test.

--- gcc/cp/method.c.jj	2021-09-15 08:55:37.563497558 +0200
+++ gcc/cp/method.c	2021-09-27 13:48:12.139271830 +0200
@@ -3160,8 +3160,11 @@ defaulted_late_check (tree fn)
   if (kind == sfk_comparison)
     {
       /* If the function was declared constexpr, check that the definition
-	 qualifies.  Otherwise we can define the function lazily.  */
-      if (DECL_DECLARED_CONSTEXPR_P (fn) && !DECL_INITIAL (fn))
+	 qualifies.  Otherwise we can define the function lazily.
+	 Don't do this if the class type is still incomplete.  */
+      if (DECL_DECLARED_CONSTEXPR_P (fn)
+	  && !DECL_INITIAL (fn)
+	  && COMPLETE_TYPE_P (ctx))
 	{
 	  /* Prevent GC.  */
 	  function_depth++;
--- gcc/cp/class.c.jj	2021-09-03 09:46:28.801428380 +0200
+++ gcc/cp/class.c	2021-09-27 14:07:03.465562255 +0200
@@ -7467,7 +7467,14 @@ finish_struct_1 (tree t)
      for any static member objects of the type we're working on.  */
   for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     if (DECL_DECLARES_FUNCTION_P (x))
-      DECL_IN_AGGR_P (x) = false;
+      {
+	/* Synthetize constexpr defaulted comparisons.  */
+	if (!DECL_ARTIFICIAL (x)
+	    && DECL_DEFAULTED_IN_CLASS_P (x)
+	    && special_function_p (x) == sfk_comparison)
+	  defaulted_late_check (x);
+	DECL_IN_AGGR_P (x) = false;
+      }
     else if (VAR_P (x) && TREE_STATIC (x)
 	     && TREE_TYPE (x) != error_mark_node
 	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-27 14:20:04.723713371 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-27 14:20:20.387495858 +0200
@@ -0,0 +1,43 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+
+struct A
+{
+  unsigned char a : 1;
+  unsigned char b : 1;
+  constexpr bool operator== (const A &) const = default;
+};
+
+struct B
+{
+  unsigned char a : 8;
+  int : 0;
+  unsigned char b : 7;
+  constexpr bool operator== (const B &) const = default;
+};
+
+struct C
+{
+  unsigned char a : 3;
+  unsigned char b : 1;
+  constexpr bool operator== (const C &) const = default;
+};
+
+void
+foo (C &x, int y)
+{
+  x.b = y;
+}
+
+int
+main ()
+{
+  A a{}, b{};
+  B c{}, d{};
+  C e{}, f{};
+  a.b = 1;
+  d.b = 1;
+  foo (e, 0);
+  foo (f, 1);
+  return a == b || c == d || e == f;
+}
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-27 14:20:12.050611625 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-27 14:20:39.633228602 +0200
@@ -0,0 +1,5 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+// { dg-options "-O2" }
+
+#include "spaceship-eq11.C"

	Jakub

Re: [PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Patrick Palka]

On Tue, 28 Sep 2021, Jakub Jelinek via Gcc-patches wrote:

> Hi!
> 
> The testcases in the patch are either miscompiled or ICE with checking,
> because the defaulted operator== is synthetized too early (but only if
> constexpr), when the corresponding class type is still incomplete type.
> The problem is that at that point the bitfield FIELD_DECLs still have as
> TREE_TYPE their underlying type rather than integral type with their
> precision and when layout_class_type is called for the class soon after
> that, it changes those types but the COMPONENT_REFs type stay the way
> that they were during the operator== synthetize_method type and the
> middle-end is then upset by the mismatch of types.
> As what exact type will be given isn't just a one liner but quite long code
> especially for over-sized bitfields, I think it is best to just not
> synthetize the comparison operators so early (the defaulted_late_check
> change) and call defaulted_late_check for them once again as soon as the
> class is complete.

Nice, this might also fix PR98712.

> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2021-09-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/102490
> 	* method.c (defaulted_late_check): Don't synthetize constexpr
> 	defaulted comparisons if context is still incomplete type.
> 	(finish_struct_1): Call defaulted_late_check again for defaulted
> 	comparisons.
> 
> 	* g++.dg/cpp2a/spaceship-eq11.C: New test.
> 	* g++.dg/cpp2a/spaceship-eq12.C: New test.
> 
> --- gcc/cp/method.c.jj	2021-09-15 08:55:37.563497558 +0200
> +++ gcc/cp/method.c	2021-09-27 13:48:12.139271830 +0200
> @@ -3160,8 +3160,11 @@ defaulted_late_check (tree fn)
>    if (kind == sfk_comparison)
>      {
>        /* If the function was declared constexpr, check that the definition
> -	 qualifies.  Otherwise we can define the function lazily.  */
> -      if (DECL_DECLARED_CONSTEXPR_P (fn) && !DECL_INITIAL (fn))
> +	 qualifies.  Otherwise we can define the function lazily.
> +	 Don't do this if the class type is still incomplete.  */
> +      if (DECL_DECLARED_CONSTEXPR_P (fn)
> +	  && !DECL_INITIAL (fn)
> +	  && COMPLETE_TYPE_P (ctx))
>  	{

According to the function comment for defaulted_late_check, won't
COMPLETE_TYPE_P (ctx) always be false here?

>  	  /* Prevent GC.  */
>  	  function_depth++;
> --- gcc/cp/class.c.jj	2021-09-03 09:46:28.801428380 +0200
> +++ gcc/cp/class.c	2021-09-27 14:07:03.465562255 +0200
> @@ -7467,7 +7467,14 @@ finish_struct_1 (tree t)
>       for any static member objects of the type we're working on.  */
>    for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
>      if (DECL_DECLARES_FUNCTION_P (x))
> -      DECL_IN_AGGR_P (x) = false;
> +      {
> +	/* Synthetize constexpr defaulted comparisons.  */
> +	if (!DECL_ARTIFICIAL (x)
> +	    && DECL_DEFAULTED_IN_CLASS_P (x)
> +	    && special_function_p (x) == sfk_comparison)
> +	  defaulted_late_check (x);
> +	DECL_IN_AGGR_P (x) = false;
> +      }
>      else if (VAR_P (x) && TREE_STATIC (x)
>  	     && TREE_TYPE (x) != error_mark_node
>  	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-27 14:20:04.723713371 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-27 14:20:20.387495858 +0200
> @@ -0,0 +1,43 @@
> +// PR c++/102490
> +// { dg-do run { target c++20 } }
> +
> +struct A
> +{
> +  unsigned char a : 1;
> +  unsigned char b : 1;
> +  constexpr bool operator== (const A &) const = default;
> +};
> +
> +struct B
> +{
> +  unsigned char a : 8;
> +  int : 0;
> +  unsigned char b : 7;
> +  constexpr bool operator== (const B &) const = default;
> +};
> +
> +struct C
> +{
> +  unsigned char a : 3;
> +  unsigned char b : 1;
> +  constexpr bool operator== (const C &) const = default;
> +};
> +
> +void
> +foo (C &x, int y)
> +{
> +  x.b = y;
> +}
> +
> +int
> +main ()
> +{
> +  A a{}, b{};
> +  B c{}, d{};
> +  C e{}, f{};
> +  a.b = 1;
> +  d.b = 1;
> +  foo (e, 0);
> +  foo (f, 1);
> +  return a == b || c == d || e == f;
> +}
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-27 14:20:12.050611625 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-27 14:20:39.633228602 +0200
> @@ -0,0 +1,5 @@
> +// PR c++/102490
> +// { dg-do run { target c++20 } }
> +// { dg-options "-O2" }
> +
> +#include "spaceship-eq11.C"
> 
> 	Jakub
> 
> 

Re: [PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Patrick Palka]

On Tue, 28 Sep 2021, Patrick Palka wrote:

> On Tue, 28 Sep 2021, Jakub Jelinek via Gcc-patches wrote:
> 
> > Hi!
> > 
> > The testcases in the patch are either miscompiled or ICE with checking,
> > because the defaulted operator== is synthetized too early (but only if
> > constexpr), when the corresponding class type is still incomplete type.
> > The problem is that at that point the bitfield FIELD_DECLs still have as
> > TREE_TYPE their underlying type rather than integral type with their
> > precision and when layout_class_type is called for the class soon after
> > that, it changes those types but the COMPONENT_REFs type stay the way
> > that they were during the operator== synthetize_method type and the
> > middle-end is then upset by the mismatch of types.
> > As what exact type will be given isn't just a one liner but quite long code
> > especially for over-sized bitfields, I think it is best to just not
> > synthetize the comparison operators so early (the defaulted_late_check
> > change) and call defaulted_late_check for them once again as soon as the
> > class is complete.
> 
> Nice, this might also fix PR98712.
> 
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > 2021-09-28  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR c++/102490
> > 	* method.c (defaulted_late_check): Don't synthetize constexpr
> > 	defaulted comparisons if context is still incomplete type.
> > 	(finish_struct_1): Call defaulted_late_check again for defaulted
> > 	comparisons.
> > 
> > 	* g++.dg/cpp2a/spaceship-eq11.C: New test.
> > 	* g++.dg/cpp2a/spaceship-eq12.C: New test.
> > 
> > --- gcc/cp/method.c.jj	2021-09-15 08:55:37.563497558 +0200
> > +++ gcc/cp/method.c	2021-09-27 13:48:12.139271830 +0200
> > @@ -3160,8 +3160,11 @@ defaulted_late_check (tree fn)
> >    if (kind == sfk_comparison)
> >      {
> >        /* If the function was declared constexpr, check that the definition
> > -	 qualifies.  Otherwise we can define the function lazily.  */
> > -      if (DECL_DECLARED_CONSTEXPR_P (fn) && !DECL_INITIAL (fn))
> > +	 qualifies.  Otherwise we can define the function lazily.
> > +	 Don't do this if the class type is still incomplete.  */
> > +      if (DECL_DECLARED_CONSTEXPR_P (fn)
> > +	  && !DECL_INITIAL (fn)
> > +	  && COMPLETE_TYPE_P (ctx))
> >  	{
> 
> According to the function comment for defaulted_late_check, won't
> COMPLETE_TYPE_P (ctx) always be false here?

If so, I wonder if we could get away with moving this entire fragment
from defaulted_late_check to finish_struct_1 instead of calling
defaulted_late_check from finish_struct_1.

> 
> >  	  /* Prevent GC.  */
> >  	  function_depth++;
> > --- gcc/cp/class.c.jj	2021-09-03 09:46:28.801428380 +0200
> > +++ gcc/cp/class.c	2021-09-27 14:07:03.465562255 +0200
> > @@ -7467,7 +7467,14 @@ finish_struct_1 (tree t)
> >       for any static member objects of the type we're working on.  */
> >    for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
> >      if (DECL_DECLARES_FUNCTION_P (x))
> > -      DECL_IN_AGGR_P (x) = false;
> > +      {
> > +	/* Synthetize constexpr defaulted comparisons.  */
> > +	if (!DECL_ARTIFICIAL (x)
> > +	    && DECL_DEFAULTED_IN_CLASS_P (x)
> > +	    && special_function_p (x) == sfk_comparison)
> > +	  defaulted_late_check (x);
> > +	DECL_IN_AGGR_P (x) = false;
> > +      }
> >      else if (VAR_P (x) && TREE_STATIC (x)
> >  	     && TREE_TYPE (x) != error_mark_node
> >  	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
> > --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-27 14:20:04.723713371 +0200
> > +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-27 14:20:20.387495858 +0200
> > @@ -0,0 +1,43 @@
> > +// PR c++/102490
> > +// { dg-do run { target c++20 } }
> > +
> > +struct A
> > +{
> > +  unsigned char a : 1;
> > +  unsigned char b : 1;
> > +  constexpr bool operator== (const A &) const = default;
> > +};
> > +
> > +struct B
> > +{
> > +  unsigned char a : 8;
> > +  int : 0;
> > +  unsigned char b : 7;
> > +  constexpr bool operator== (const B &) const = default;
> > +};
> > +
> > +struct C
> > +{
> > +  unsigned char a : 3;
> > +  unsigned char b : 1;
> > +  constexpr bool operator== (const C &) const = default;
> > +};
> > +
> > +void
> > +foo (C &x, int y)
> > +{
> > +  x.b = y;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +  A a{}, b{};
> > +  B c{}, d{};
> > +  C e{}, f{};
> > +  a.b = 1;
> > +  d.b = 1;
> > +  foo (e, 0);
> > +  foo (f, 1);
> > +  return a == b || c == d || e == f;
> > +}
> > --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-27 14:20:12.050611625 +0200
> > +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-27 14:20:39.633228602 +0200
> > @@ -0,0 +1,5 @@
> > +// PR c++/102490
> > +// { dg-do run { target c++20 } }
> > +// { dg-options "-O2" }
> > +
> > +#include "spaceship-eq11.C"
> > 
> > 	Jakub
> > 
> > 
> 

Re: [PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jason Merrill]

On 9/28/21 09:53, Patrick Palka wrote:
> On Tue, 28 Sep 2021, Patrick Palka wrote:
> 
>> On Tue, 28 Sep 2021, Jakub Jelinek via Gcc-patches wrote:
>>
>>> Hi!
>>>
>>> The testcases in the patch are either miscompiled or ICE with checking,
>>> because the defaulted operator== is synthetized too early (but only if
>>> constexpr), when the corresponding class type is still incomplete type.
>>> The problem is that at that point the bitfield FIELD_DECLs still have as
>>> TREE_TYPE their underlying type rather than integral type with their
>>> precision and when layout_class_type is called for the class soon after
>>> that, it changes those types but the COMPONENT_REFs type stay the way
>>> that they were during the operator== synthetize_method type and the
>>> middle-end is then upset by the mismatch of types.
>>> As what exact type will be given isn't just a one liner but quite long code
>>> especially for over-sized bitfields, I think it is best to just not
>>> synthetize the comparison operators so early (the defaulted_late_check
>>> change) and call defaulted_late_check for them once again as soon as the
>>> class is complete.
>>
>> Nice, this might also fix PR98712.
>>
>>>
>>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>>>
>>> 2021-09-28  Jakub Jelinek  <jakub@redhat.com>
>>>
>>> 	PR c++/102490
>>> 	* method.c (defaulted_late_check): Don't synthetize constexpr
>>> 	defaulted comparisons if context is still incomplete type.
>>> 	(finish_struct_1): Call defaulted_late_check again for defaulted
>>> 	comparisons.
>>>
>>> 	* g++.dg/cpp2a/spaceship-eq11.C: New test.
>>> 	* g++.dg/cpp2a/spaceship-eq12.C: New test.
>>>
>>> --- gcc/cp/method.c.jj	2021-09-15 08:55:37.563497558 +0200
>>> +++ gcc/cp/method.c	2021-09-27 13:48:12.139271830 +0200
>>> @@ -3160,8 +3160,11 @@ defaulted_late_check (tree fn)
>>>     if (kind == sfk_comparison)
>>>       {
>>>         /* If the function was declared constexpr, check that the definition
>>> -	 qualifies.  Otherwise we can define the function lazily.  */
>>> -      if (DECL_DECLARED_CONSTEXPR_P (fn) && !DECL_INITIAL (fn))
>>> +	 qualifies.  Otherwise we can define the function lazily.
>>> +	 Don't do this if the class type is still incomplete.  */
>>> +      if (DECL_DECLARED_CONSTEXPR_P (fn)
>>> +	  && !DECL_INITIAL (fn)
>>> +	  && COMPLETE_TYPE_P (ctx))
>>>   	{
>>
>> According to the function comment for defaulted_late_check, won't
>> COMPLETE_TYPE_P (ctx) always be false here?

Not for a function defaulted outside the class.

> If so, I wonder if we could get away with moving this entire fragment
> from defaulted_late_check to finish_struct_1 instead of calling
> defaulted_late_check from finish_struct_1.

The comment in check_bases_and_members says that we call it there so 
that it's before we clone [cd]tors.  Probably better to leave the call 
there for other functions, just skip it for comparisons.

>>>   	  /* Prevent GC.  */
>>>   	  function_depth++;
>>> --- gcc/cp/class.c.jj	2021-09-03 09:46:28.801428380 +0200
>>> +++ gcc/cp/class.c	2021-09-27 14:07:03.465562255 +0200
>>> @@ -7467,7 +7467,14 @@ finish_struct_1 (tree t)
>>>        for any static member objects of the type we're working on.  */
>>>     for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
>>>       if (DECL_DECLARES_FUNCTION_P (x))
>>> -      DECL_IN_AGGR_P (x) = false;
>>> +      {
>>> +	/* Synthetize constexpr defaulted comparisons.  */
>>> +	if (!DECL_ARTIFICIAL (x)
>>> +	    && DECL_DEFAULTED_IN_CLASS_P (x)
>>> +	    && special_function_p (x) == sfk_comparison)
>>> +	  defaulted_late_check (x);
>>> +	DECL_IN_AGGR_P (x) = false;
>>> +      }
>>>       else if (VAR_P (x) && TREE_STATIC (x)
>>>   	     && TREE_TYPE (x) != error_mark_node
>>>   	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
>>> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-27 14:20:04.723713371 +0200
>>> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-27 14:20:20.387495858 +0200
>>> @@ -0,0 +1,43 @@
>>> +// PR c++/102490
>>> +// { dg-do run { target c++20 } }
>>> +
>>> +struct A
>>> +{
>>> +  unsigned char a : 1;
>>> +  unsigned char b : 1;
>>> +  constexpr bool operator== (const A &) const = default;
>>> +};
>>> +
>>> +struct B
>>> +{
>>> +  unsigned char a : 8;
>>> +  int : 0;
>>> +  unsigned char b : 7;
>>> +  constexpr bool operator== (const B &) const = default;
>>> +};
>>> +
>>> +struct C
>>> +{
>>> +  unsigned char a : 3;
>>> +  unsigned char b : 1;
>>> +  constexpr bool operator== (const C &) const = default;
>>> +};
>>> +
>>> +void
>>> +foo (C &x, int y)
>>> +{
>>> +  x.b = y;
>>> +}
>>> +
>>> +int
>>> +main ()
>>> +{
>>> +  A a{}, b{};
>>> +  B c{}, d{};
>>> +  C e{}, f{};
>>> +  a.b = 1;
>>> +  d.b = 1;
>>> +  foo (e, 0);
>>> +  foo (f, 1);
>>> +  return a == b || c == d || e == f;
>>> +}
>>> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-27 14:20:12.050611625 +0200
>>> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-27 14:20:39.633228602 +0200
>>> @@ -0,0 +1,5 @@
>>> +// PR c++/102490
>>> +// { dg-do run { target c++20 } }
>>> +// { dg-options "-O2" }
>>> +
>>> +#include "spaceship-eq11.C"
>>>
>>> 	Jakub
>>>
>>>
>>
> 

[PATCH, v2] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Tue, Sep 28, 2021 at 03:33:35PM -0400, Jason Merrill wrote:
> > > According to the function comment for defaulted_late_check, won't
> > > COMPLETE_TYPE_P (ctx) always be false here?
> 
> Not for a function defaulted outside the class.
> 
> > If so, I wonder if we could get away with moving this entire fragment
> > from defaulted_late_check to finish_struct_1 instead of calling
> > defaulted_late_check from finish_struct_1.
> 
> The comment in check_bases_and_members says that we call it there so that
> it's before we clone [cd]tors.  Probably better to leave the call there for
> other functions, just skip it for comparisons.

So like this instead then?  Just tested with dg.exp=*spaceship* so far.

2021-09-28  Jakub Jelinek  <jakub@redhat.com>

	PR c++/102490
	* method.c (defaulted_late_check): Don't synthetize constexpr
	defaulted comparisons.
	(finish_struct_1): Synthetize constexpr defaulted comparisons here
	after layout_class_type.

	* g++.dg/cpp2a/spaceship-eq11.C: New test.
	* g++.dg/cpp2a/spaceship-eq12.C: New test.

--- gcc/cp/method.c.jj	2021-09-28 11:34:10.165412477 +0200
+++ gcc/cp/method.c	2021-09-28 22:28:23.637981709 +0200
@@ -3158,18 +3158,7 @@ defaulted_late_check (tree fn)
   special_function_kind kind = special_function_p (fn);
 
   if (kind == sfk_comparison)
-    {
-      /* If the function was declared constexpr, check that the definition
-	 qualifies.  Otherwise we can define the function lazily.  */
-      if (DECL_DECLARED_CONSTEXPR_P (fn) && !DECL_INITIAL (fn))
-	{
-	  /* Prevent GC.  */
-	  function_depth++;
-	  synthesize_method (fn);
-	  function_depth--;
-	}
-      return;
-    }
+    return;
 
   bool fn_const_p = (copy_fn_p (fn) == 2);
   tree implicit_fn = implicitly_declare_fn (kind, ctx, fn_const_p,
--- gcc/cp/class.c.jj	2021-09-28 11:34:10.096413431 +0200
+++ gcc/cp/class.c	2021-09-28 22:29:59.072669058 +0200
@@ -7467,7 +7467,21 @@ finish_struct_1 (tree t)
      for any static member objects of the type we're working on.  */
   for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     if (DECL_DECLARES_FUNCTION_P (x))
-      DECL_IN_AGGR_P (x) = false;
+      {
+	/* Synthetize constexpr defaulted comparisons.  */
+	if (!DECL_ARTIFICIAL (x)
+	    && DECL_DEFAULTED_IN_CLASS_P (x)
+	    && special_function_p (x) == sfk_comparison
+	    && DECL_DECLARED_CONSTEXPR_P (x)
+	    && !DECL_INITIAL (x))
+	  {
+	    /* Prevent GC.  */
+	    function_depth++;
+	    synthesize_method (x);
+	    function_depth--;
+	  }
+	DECL_IN_AGGR_P (x) = false;
+      }
     else if (VAR_P (x) && TREE_STATIC (x)
 	     && TREE_TYPE (x) != error_mark_node
 	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-28 22:27:40.524574708 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-28 22:27:40.524574708 +0200
@@ -0,0 +1,43 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+
+struct A
+{
+  unsigned char a : 1;
+  unsigned char b : 1;
+  constexpr bool operator== (const A &) const = default;
+};
+
+struct B
+{
+  unsigned char a : 8;
+  int : 0;
+  unsigned char b : 7;
+  constexpr bool operator== (const B &) const = default;
+};
+
+struct C
+{
+  unsigned char a : 3;
+  unsigned char b : 1;
+  constexpr bool operator== (const C &) const = default;
+};
+
+void
+foo (C &x, int y)
+{
+  x.b = y;
+}
+
+int
+main ()
+{
+  A a{}, b{};
+  B c{}, d{};
+  C e{}, f{};
+  a.b = 1;
+  d.b = 1;
+  foo (e, 0);
+  foo (f, 1);
+  return a == b || c == d || e == f;
+}
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-28 22:27:40.524574708 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-28 22:27:40.524574708 +0200
@@ -0,0 +1,5 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+// { dg-options "-O2" }
+
+#include "spaceship-eq11.C"


	Jakub

Re: [PATCH, v2] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Tue, Sep 28, 2021 at 10:34:39PM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Tue, Sep 28, 2021 at 03:33:35PM -0400, Jason Merrill wrote:
> > > > According to the function comment for defaulted_late_check, won't
> > > > COMPLETE_TYPE_P (ctx) always be false here?
> > 
> > Not for a function defaulted outside the class.
> > 
> > > If so, I wonder if we could get away with moving this entire fragment
> > > from defaulted_late_check to finish_struct_1 instead of calling
> > > defaulted_late_check from finish_struct_1.
> > 
> > The comment in check_bases_and_members says that we call it there so that
> > it's before we clone [cd]tors.  Probably better to leave the call there for
> > other functions, just skip it for comparisons.
> 
> So like this instead then?  Just tested with dg.exp=*spaceship* so far.
> 
> 2021-09-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/102490
> 	* method.c (defaulted_late_check): Don't synthetize constexpr
> 	defaulted comparisons.
> 	(finish_struct_1): Synthetize constexpr defaulted comparisons here
> 	after layout_class_type.
> 
> 	* g++.dg/cpp2a/spaceship-eq11.C: New test.
> 	* g++.dg/cpp2a/spaceship-eq12.C: New test.

Also successfully bootstrapped/regtested on x86_64-linux and i686-linux.

	Jakub

Re: [PATCH, v2] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jason Merrill]

On 9/28/21 16:34, Jakub Jelinek wrote:
> On Tue, Sep 28, 2021 at 03:33:35PM -0400, Jason Merrill wrote:
>>>> According to the function comment for defaulted_late_check, won't
>>>> COMPLETE_TYPE_P (ctx) always be false here?
>>
>> Not for a function defaulted outside the class.
>>
>>> If so, I wonder if we could get away with moving this entire fragment
>>> from defaulted_late_check to finish_struct_1 instead of calling
>>> defaulted_late_check from finish_struct_1.
>>
>> The comment in check_bases_and_members says that we call it there so that
>> it's before we clone [cd]tors.  Probably better to leave the call there for
>> other functions, just skip it for comparisons.
> 
> So like this instead then?  Just tested with dg.exp=*spaceship* so far.

I was thinking to keep the checking/synthesis in defaulted_late_check, 
just change when it's called.

That is, not to change defaulted_late_check at all, to keep the 
finish_struct_1 change you had in your first patch, and to change 
check_bases_and_members to not call defaulted_late_check for comparisons.

We probably also want to assert COMPLETE_TYPE_P in build_comparison_op, 
at least when info.defining.

BTW, the word is "synthesize".

> 2021-09-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/102490
> 	* method.c (defaulted_late_check): Don't synthetize constexpr
> 	defaulted comparisons.
> 	(finish_struct_1): Synthetize constexpr defaulted comparisons here
> 	after layout_class_type.
> 
> 	* g++.dg/cpp2a/spaceship-eq11.C: New test.
> 	* g++.dg/cpp2a/spaceship-eq12.C: New test.
> 
> --- gcc/cp/method.c.jj	2021-09-28 11:34:10.165412477 +0200
> +++ gcc/cp/method.c	2021-09-28 22:28:23.637981709 +0200
> @@ -3158,18 +3158,7 @@ defaulted_late_check (tree fn)
>     special_function_kind kind = special_function_p (fn);
>   
>     if (kind == sfk_comparison)
> -    {
> -      /* If the function was declared constexpr, check that the definition
> -	 qualifies.  Otherwise we can define the function lazily.  */
> -      if (DECL_DECLARED_CONSTEXPR_P (fn) && !DECL_INITIAL (fn))
> -	{
> -	  /* Prevent GC.  */
> -	  function_depth++;
> -	  synthesize_method (fn);
> -	  function_depth--;
> -	}
> -      return;
> -    }
> +    return;
>   
>     bool fn_const_p = (copy_fn_p (fn) == 2);
>     tree implicit_fn = implicitly_declare_fn (kind, ctx, fn_const_p,
> --- gcc/cp/class.c.jj	2021-09-28 11:34:10.096413431 +0200
> +++ gcc/cp/class.c	2021-09-28 22:29:59.072669058 +0200
> @@ -7467,7 +7467,21 @@ finish_struct_1 (tree t)
>        for any static member objects of the type we're working on.  */
>     for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
>       if (DECL_DECLARES_FUNCTION_P (x))
> -      DECL_IN_AGGR_P (x) = false;
> +      {
> +	/* Synthetize constexpr defaulted comparisons.  */
> +	if (!DECL_ARTIFICIAL (x)
> +	    && DECL_DEFAULTED_IN_CLASS_P (x)
> +	    && special_function_p (x) == sfk_comparison
> +	    && DECL_DECLARED_CONSTEXPR_P (x)
> +	    && !DECL_INITIAL (x))
> +	  {
> +	    /* Prevent GC.  */
> +	    function_depth++;
> +	    synthesize_method (x);
> +	    function_depth--;
> +	  }
> +	DECL_IN_AGGR_P (x) = false;
> +      }
>       else if (VAR_P (x) && TREE_STATIC (x)
>   	     && TREE_TYPE (x) != error_mark_node
>   	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-28 22:27:40.524574708 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-28 22:27:40.524574708 +0200
> @@ -0,0 +1,43 @@
> +// PR c++/102490
> +// { dg-do run { target c++20 } }
> +
> +struct A
> +{
> +  unsigned char a : 1;
> +  unsigned char b : 1;
> +  constexpr bool operator== (const A &) const = default;
> +};
> +
> +struct B
> +{
> +  unsigned char a : 8;
> +  int : 0;
> +  unsigned char b : 7;
> +  constexpr bool operator== (const B &) const = default;
> +};
> +
> +struct C
> +{
> +  unsigned char a : 3;
> +  unsigned char b : 1;
> +  constexpr bool operator== (const C &) const = default;
> +};
> +
> +void
> +foo (C &x, int y)
> +{
> +  x.b = y;
> +}
> +
> +int
> +main ()
> +{
> +  A a{}, b{};
> +  B c{}, d{};
> +  C e{}, f{};
> +  a.b = 1;
> +  d.b = 1;
> +  foo (e, 0);
> +  foo (f, 1);
> +  return a == b || c == d || e == f;
> +}
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-28 22:27:40.524574708 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-28 22:27:40.524574708 +0200
> @@ -0,0 +1,5 @@
> +// PR c++/102490
> +// { dg-do run { target c++20 } }
> +// { dg-options "-O2" }
> +
> +#include "spaceship-eq11.C"
> 
> 
> 	Jakub
> 

Re: [PATCH, v2] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Wed, Sep 29, 2021 at 02:14:36PM -0400, Jason Merrill wrote:
> On 9/28/21 16:34, Jakub Jelinek wrote:
> > On Tue, Sep 28, 2021 at 03:33:35PM -0400, Jason Merrill wrote:
> > > > > According to the function comment for defaulted_late_check, won't
> > > > > COMPLETE_TYPE_P (ctx) always be false here?
> > > 
> > > Not for a function defaulted outside the class.
> > > 
> > > > If so, I wonder if we could get away with moving this entire fragment
> > > > from defaulted_late_check to finish_struct_1 instead of calling
> > > > defaulted_late_check from finish_struct_1.
> > > 
> > > The comment in check_bases_and_members says that we call it there so that
> > > it's before we clone [cd]tors.  Probably better to leave the call there for
> > > other functions, just skip it for comparisons.
> > 
> > So like this instead then?  Just tested with dg.exp=*spaceship* so far.
> 
> I was thinking to keep the checking/synthesis in defaulted_late_check, just
> change when it's called.
> 
> That is, not to change defaulted_late_check at all, to keep the
> finish_struct_1 change you had in your first patch, and to change
> check_bases_and_members to not call defaulted_late_check for comparisons.
>
> We probably also want to assert COMPLETE_TYPE_P in build_comparison_op, at
> least when info.defining.

I've tried that, but the assertion in build_comparison_op ICEs on
spaceship-synth8.C testcase:
#0  fancy_abort (file=0x2d75b3c "../../gcc/cp/method.c", line=1387, function=0x2d761c3 "build_comparison_op") at ../../gcc/diagnostic.c:1982
#1  0x0000000000c8dad0 in build_comparison_op (fndecl=<function_decl 0x7fffea195600 operator<=>>, complain=0) at ../../gcc/cp/method.c:1387
#2  0x0000000000c8fa7c in synthesize_method (fndecl=<function_decl 0x7fffea195600 operator<=>>) at ../../gcc/cp/method.c:1780
#3  0x0000000000e0ac55 in maybe_instantiate_noexcept (fn=<function_decl 0x7fffea195600 operator<=>>, complain=3) at ../../gcc/cp/pt.c:25769
#4  0x0000000000e33672 in maybe_check_overriding_exception_spec (overrider=<function_decl 0x7fffea195600 operator<=>>, basefn=<function_decl 0x7fffea195500 operator<=>>)
    at ../../gcc/cp/search.c:1897
#5  0x0000000000e33f5e in check_final_overrider (overrider=<function_decl 0x7fffea195600 operator<=>>, basefn=<function_decl 0x7fffea195500 operator<=>>)
    at ../../gcc/cp/search.c:2015
#6  0x0000000000e35365 in look_for_overrides_r (type=<record_type 0x7fffea191dc8 D>, fndecl=<function_decl 0x7fffea195600 operator<=>>) at ../../gcc/cp/search.c:2183
#7  0x0000000000e34e60 in look_for_overrides (type=<record_type 0x7fffea191d20 E>, fndecl=<function_decl 0x7fffea195600 operator<=>>) at ../../gcc/cp/search.c:2127
#8  0x0000000000afc3ba in check_for_override (decl=<function_decl 0x7fffea195600 operator<=>>, ctype=<record_type 0x7fffea191d20 E>) at ../../gcc/cp/class.c:2934
#9  0x0000000000b047c2 in check_methods (t=<record_type 0x7fffea191d20 E>) at ../../gcc/cp/class.c:4720
#10 0x0000000000b0d25c in check_bases_and_members (t=<record_type 0x7fffea191d20 E>) at ../../gcc/cp/class.c:5990
#11 0x0000000000b141de in finish_struct_1 (t=<record_type 0x7fffea191d20 E>) at ../../gcc/cp/class.c:7373
#12 0x0000000000b15f16 in finish_struct (t=<record_type 0x7fffea191d20 E>, attributes=<tree 0x0>) at ../../gcc/cp/class.c:7676

Do you think there is any hope to defer even that, or is that something
that has to be done before layout_class_type?

--- gcc/cp/method.c.jj	2021-09-29 10:07:28.791062239 +0200
+++ gcc/cp/method.c	2021-09-29 20:24:14.229131851 +0200
@@ -1384,6 +1384,7 @@ build_comparison_op (tree fndecl, tsubst
     lhs = convert_from_reference (lhs);
   rhs = convert_from_reference (rhs);
   tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
+  gcc_assert (COMPLETE_TYPE_P (ctype));
 
   iloc_sentinel ils (info.loc);
 
--- gcc/cp/class.c.jj	2021-09-29 10:07:28.745062879 +0200
+++ gcc/cp/class.c	2021-09-29 20:23:13.117984089 +0200
@@ -6133,7 +6133,8 @@ check_bases_and_members (tree t)
 		 no longer ill-formed, it is defined as deleted instead.  */
 	      DECL_DELETED_FN (fn) = true;
 	  }
-	defaulted_late_check (fn);
+	if (special_function_p (fn) != sfk_comparison)
+	  defaulted_late_check (fn);
       }
 
   if (LAMBDA_TYPE_P (t))
@@ -7467,7 +7468,14 @@ finish_struct_1 (tree t)
      for any static member objects of the type we're working on.  */
   for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     if (DECL_DECLARES_FUNCTION_P (x))
-      DECL_IN_AGGR_P (x) = false;
+      {
+	/* Synthetize constexpr defaulted comparisons.  */
+	if (!DECL_ARTIFICIAL (x)
+	    && DECL_DEFAULTED_IN_CLASS_P (x)
+	    && special_function_p (x) == sfk_comparison)
+	  defaulted_late_check (x);
+	DECL_IN_AGGR_P (x) = false;
+      }
     else if (VAR_P (x) && TREE_STATIC (x)
 	     && TREE_TYPE (x) != error_mark_node
 	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-29 20:21:49.861145165 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-29 20:21:49.861145165 +0200
@@ -0,0 +1,43 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+
+struct A
+{
+  unsigned char a : 1;
+  unsigned char b : 1;
+  constexpr bool operator== (const A &) const = default;
+};
+
+struct B
+{
+  unsigned char a : 8;
+  int : 0;
+  unsigned char b : 7;
+  constexpr bool operator== (const B &) const = default;
+};
+
+struct C
+{
+  unsigned char a : 3;
+  unsigned char b : 1;
+  constexpr bool operator== (const C &) const = default;
+};
+
+void
+foo (C &x, int y)
+{
+  x.b = y;
+}
+
+int
+main ()
+{
+  A a{}, b{};
+  B c{}, d{};
+  C e{}, f{};
+  a.b = 1;
+  d.b = 1;
+  foo (e, 0);
+  foo (f, 1);
+  return a == b || c == d || e == f;
+}
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-29 20:21:49.861145165 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-29 20:21:49.861145165 +0200
@@ -0,0 +1,5 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+// { dg-options "-O2" }
+
+#include "spaceship-eq11.C"


	Jakub

Re: [PATCH, v2] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jason Merrill]

On 9/29/21 15:21, Jakub Jelinek wrote:
> On Wed, Sep 29, 2021 at 02:14:36PM -0400, Jason Merrill wrote:
>> On 9/28/21 16:34, Jakub Jelinek wrote:
>>> On Tue, Sep 28, 2021 at 03:33:35PM -0400, Jason Merrill wrote:
>>>>>> According to the function comment for defaulted_late_check, won't
>>>>>> COMPLETE_TYPE_P (ctx) always be false here?
>>>>
>>>> Not for a function defaulted outside the class.
>>>>
>>>>> If so, I wonder if we could get away with moving this entire fragment
>>>>> from defaulted_late_check to finish_struct_1 instead of calling
>>>>> defaulted_late_check from finish_struct_1.
>>>>
>>>> The comment in check_bases_and_members says that we call it there so that
>>>> it's before we clone [cd]tors.  Probably better to leave the call there for
>>>> other functions, just skip it for comparisons.
>>>
>>> So like this instead then?  Just tested with dg.exp=*spaceship* so far.
>>
>> I was thinking to keep the checking/synthesis in defaulted_late_check, just
>> change when it's called.
>>
>> That is, not to change defaulted_late_check at all, to keep the
>> finish_struct_1 change you had in your first patch, and to change
>> check_bases_and_members to not call defaulted_late_check for comparisons.
>>
>> We probably also want to assert COMPLETE_TYPE_P in build_comparison_op, at
>> least when info.defining.
> 
> I've tried that, but the assertion in build_comparison_op ICEs on
> spaceship-synth8.C testcase:
> #0  fancy_abort (file=0x2d75b3c "../../gcc/cp/method.c", line=1387, function=0x2d761c3 "build_comparison_op") at ../../gcc/diagnostic.c:1982
> #1  0x0000000000c8dad0 in build_comparison_op (fndecl=<function_decl 0x7fffea195600 operator<=>>, complain=0) at ../../gcc/cp/method.c:1387
> #2  0x0000000000c8fa7c in synthesize_method (fndecl=<function_decl 0x7fffea195600 operator<=>>) at ../../gcc/cp/method.c:1780
> #3  0x0000000000e0ac55 in maybe_instantiate_noexcept (fn=<function_decl 0x7fffea195600 operator<=>>, complain=3) at ../../gcc/cp/pt.c:25769
> #4  0x0000000000e33672 in maybe_check_overriding_exception_spec (overrider=<function_decl 0x7fffea195600 operator<=>>, basefn=<function_decl 0x7fffea195500 operator<=>>)
>      at ../../gcc/cp/search.c:1897
> #5  0x0000000000e33f5e in check_final_overrider (overrider=<function_decl 0x7fffea195600 operator<=>>, basefn=<function_decl 0x7fffea195500 operator<=>>)
>      at ../../gcc/cp/search.c:2015
> #6  0x0000000000e35365 in look_for_overrides_r (type=<record_type 0x7fffea191dc8 D>, fndecl=<function_decl 0x7fffea195600 operator<=>>) at ../../gcc/cp/search.c:2183
> #7  0x0000000000e34e60 in look_for_overrides (type=<record_type 0x7fffea191d20 E>, fndecl=<function_decl 0x7fffea195600 operator<=>>) at ../../gcc/cp/search.c:2127
> #8  0x0000000000afc3ba in check_for_override (decl=<function_decl 0x7fffea195600 operator<=>>, ctype=<record_type 0x7fffea191d20 E>) at ../../gcc/cp/class.c:2934
> #9  0x0000000000b047c2 in check_methods (t=<record_type 0x7fffea191d20 E>) at ../../gcc/cp/class.c:4720
> #10 0x0000000000b0d25c in check_bases_and_members (t=<record_type 0x7fffea191d20 E>) at ../../gcc/cp/class.c:5990
> #11 0x0000000000b141de in finish_struct_1 (t=<record_type 0x7fffea191d20 E>) at ../../gcc/cp/class.c:7373
> #12 0x0000000000b15f16 in finish_struct (t=<record_type 0x7fffea191d20 E>, attributes=<tree 0x0>) at ../../gcc/cp/class.c:7676
> 
> Do you think there is any hope to defer even that, or is that something
> that has to be done before layout_class_type?

It ought to be possible to defer check_final_overrider, but it sounds 
awkward.

Or maybe_instantiate_noexcept could use the non-defining path in 
build_comparison_op.

Maybe we want a maybe_synthesize_method that actually builds the 
function if the type is complete, or takes the non-defining path if not.

> --- gcc/cp/method.c.jj	2021-09-29 10:07:28.791062239 +0200
> +++ gcc/cp/method.c	2021-09-29 20:24:14.229131851 +0200
> @@ -1384,6 +1384,7 @@ build_comparison_op (tree fndecl, tsubst
>       lhs = convert_from_reference (lhs);
>     rhs = convert_from_reference (rhs);
>     tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
> +  gcc_assert (COMPLETE_TYPE_P (ctype));
>     iloc_sentinel ils (info.loc);
>   
> --- gcc/cp/class.c.jj	2021-09-29 10:07:28.745062879 +0200
> +++ gcc/cp/class.c	2021-09-29 20:23:13.117984089 +0200
> @@ -6133,7 +6133,8 @@ check_bases_and_members (tree t)
>   		 no longer ill-formed, it is defined as deleted instead.  */
>   	      DECL_DELETED_FN (fn) = true;
>   	  }
> -	defaulted_late_check (fn);
> +	if (special_function_p (fn) != sfk_comparison)
> +	  defaulted_late_check (fn);
>         }
>   
>     if (LAMBDA_TYPE_P (t))
> @@ -7467,7 +7468,14 @@ finish_struct_1 (tree t)
>        for any static member objects of the type we're working on.  */
>     for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
>       if (DECL_DECLARES_FUNCTION_P (x))
> -      DECL_IN_AGGR_P (x) = false;
> +      {
> +	/* Synthetize constexpr defaulted comparisons.  */

"Synthesize"

> +	if (!DECL_ARTIFICIAL (x)
> +	    && DECL_DEFAULTED_IN_CLASS_P (x)
> +	    && special_function_p (x) == sfk_comparison)
> +	  defaulted_late_check (x);
> +	DECL_IN_AGGR_P (x) = false;
> +      }
>       else if (VAR_P (x) && TREE_STATIC (x)
>   	     && TREE_TYPE (x) != error_mark_node
>   	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-29 20:21:49.861145165 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-29 20:21:49.861145165 +0200
> @@ -0,0 +1,43 @@
> +// PR c++/102490
> +// { dg-do run { target c++20 } }
> +
> +struct A
> +{
> +  unsigned char a : 1;
> +  unsigned char b : 1;
> +  constexpr bool operator== (const A &) const = default;
> +};
> +
> +struct B
> +{
> +  unsigned char a : 8;
> +  int : 0;
> +  unsigned char b : 7;
> +  constexpr bool operator== (const B &) const = default;
> +};
> +
> +struct C
> +{
> +  unsigned char a : 3;
> +  unsigned char b : 1;
> +  constexpr bool operator== (const C &) const = default;
> +};
> +
> +void
> +foo (C &x, int y)
> +{
> +  x.b = y;
> +}
> +
> +int
> +main ()
> +{
> +  A a{}, b{};
> +  B c{}, d{};
> +  C e{}, f{};
> +  a.b = 1;
> +  d.b = 1;
> +  foo (e, 0);
> +  foo (f, 1);
> +  return a == b || c == d || e == f;
> +}
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-29 20:21:49.861145165 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-29 20:21:49.861145165 +0200
> @@ -0,0 +1,5 @@
> +// PR c++/102490
> +// { dg-do run { target c++20 } }
> +// { dg-options "-O2" }
> +
> +#include "spaceship-eq11.C"
> 
> 
> 	Jakub
> 

[PATCH, v3] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Wed, Sep 29, 2021 at 03:38:45PM -0400, Jason Merrill wrote:
> It ought to be possible to defer check_final_overrider, but it sounds
> awkward.
> 
> Or maybe_instantiate_noexcept could use the non-defining path in
> build_comparison_op.
> 
> Maybe we want a maybe_synthesize_method that actually builds the function if
> the type is complete, or takes the non-defining path if not.

So something like this?
spaceship-synth8.C (apparently added by me, so how valid/invalid it is is
unclear) now doesn't ICE anymore, but without the change I've put there
is now rejected because std::strong_ordering::less etc. aren't found.
Previously when we were synthesizing it we did that before the FIELD_DECLs
for bases have been added, so those weren't compared, but now they actually
are compared.
After fixing the incomplete std::strong_ordering spaceship-synth8.C is now
accepted, but I'm afraid I'm getting lost in this - clang++ rejects that
testcase instead complaining that D has <=> operator, but has it pure virtual.
And, when maybe_instantiate_noexcept tries to find out if the defaulted
method would be implicitly deleted or not, when it does so before the class
is complete, seems it can check whether there are errors when comparing the
direct members of the class, but not errors about bases...

2021-09-30  Jakub Jelinek  <jakub@redhat.com>

	PR c++/102490
	* cp-tree.h (build_comparison_op): Declare.
	* method.c (comp_info): Remove defining member.
	(comp_info::comp_info): Remove complain argument, don't initialize
	defining.
	(build_comparison_op): No longer static.  Add defining argument.
	Adjust comp_info construction.  Use defining instead of info.defining.
	Assert that if defining, ctype is a complete type.
	(synthesize_method, maybe_explain_implicit_delete,
	explain_implicit_non_constexpr): Adjust build_comparison_op callers.
	* class.c (check_bases_and_members): Don't call defaulted_late_check
	for sfk_comparison.
	(finish_struct_1): Call it here instead after class has been completed.
	* pt.c (maybe_instantiate_noexcept): For sfk_comparison of still
	incomplete classes, call build_comparison_op in non-defining mode
	instead of calling synthesize_method.

	* g++.dg/cpp2a/spaceship-synth8.C (std::strong_ordering): Provide more
	complete definition.
	(std::strong_ordering::less, std::strong_ordering::equal,
	std::strong_ordering::greater): Define.
	* g++.dg/cpp2a/spaceship-eq11.C: New test.
	* g++.dg/cpp2a/spaceship-eq12.C: New test.

--- gcc/cp/cp-tree.h.jj	2021-09-18 09:44:31.728743713 +0200
+++ gcc/cp/cp-tree.h	2021-09-30 18:39:10.416847290 +0200
@@ -7012,6 +7012,7 @@ extern bool maybe_explain_implicit_delet
 extern void explain_implicit_non_constexpr	(tree);
 extern bool deduce_inheriting_ctor		(tree);
 extern bool decl_remember_implicit_trigger_p	(tree);
+extern void build_comparison_op			(tree, bool, tsubst_flags_t);
 extern void synthesize_method			(tree);
 extern tree lazily_declare_fn			(special_function_kind,
 						 tree);
--- gcc/cp/method.c.jj	2021-09-30 09:22:46.323918164 +0200
+++ gcc/cp/method.c	2021-09-30 18:51:14.510744549 +0200
@@ -1288,21 +1288,16 @@ struct comp_info
 {
   tree fndecl;
   location_t loc;
-  bool defining;
   bool first_time;
   bool constexp;
   bool was_constexp;
   bool noex;
 
-  comp_info (tree fndecl, tsubst_flags_t &complain)
+  comp_info (tree fndecl)
     : fndecl (fndecl)
   {
     loc = DECL_SOURCE_LOCATION (fndecl);
 
-    /* We only have tf_error set when we're called from
-       explain_invalid_constexpr_fn or maybe_explain_implicit_delete.  */
-    defining = !(complain & tf_error);
-
     first_time = DECL_MAYBE_DELETED (fndecl);
     DECL_MAYBE_DELETED (fndecl) = false;
 
@@ -1364,12 +1359,12 @@ struct comp_info
    to use synthesize_method at the earliest opportunity and bail out if the
    function ends up being deleted.  */
 
-static void
-build_comparison_op (tree fndecl, tsubst_flags_t complain)
+void
+build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
 {
-  comp_info info (fndecl, complain);
+  comp_info info (fndecl);
 
-  if (!info.defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
+  if (!defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
     return;
 
   int flags = LOOKUP_NORMAL;
@@ -1384,6 +1379,7 @@ build_comparison_op (tree fndecl, tsubst
     lhs = convert_from_reference (lhs);
   rhs = convert_from_reference (rhs);
   tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
+  gcc_assert (!defining || COMPLETE_TYPE_P (ctype));
 
   iloc_sentinel ils (info.loc);
 
@@ -1399,7 +1395,7 @@ build_comparison_op (tree fndecl, tsubst
     }
 
   tree compound_stmt = NULL_TREE;
-  if (info.defining)
+  if (defining)
     compound_stmt = begin_compound_stmt (0);
   else
     ++cp_unevaluated_operand;
@@ -1474,8 +1470,8 @@ build_comparison_op (tree fndecl, tsubst
 		break;
 	      tree idx;
 	      /* [1] array, no loop needed, just add [0] ARRAY_REF.
-		 Similarly if !info.defining.  */
-	      if (integer_zerop (maxval) || !info.defining)
+		 Similarly if !defining.  */
+	      if (integer_zerop (maxval) || !defining)
 		idx = size_zero_node;
 	      /* Some other array, will need runtime loop.  */
 	      else
@@ -1584,7 +1580,7 @@ build_comparison_op (tree fndecl, tsubst
 	  tree comp = comps[i];
 	  tree eq, retval = NULL_TREE, if_ = NULL_TREE;
 	  tree loop_indexes = NULL_TREE;
-	  if (info.defining)
+	  if (defining)
 	    {
 	      if (TREE_CODE (comp) == TREE_LIST)
 		{
@@ -1632,7 +1628,7 @@ build_comparison_op (tree fndecl, tsubst
 		comp = build_static_cast (input_location, rettype, comp,
 					  complain);
 	      info.check (comp);
-	      if (info.defining)
+	      if (defining)
 		{
 		  tree var = create_temporary_var (rettype);
 		  pushdecl (var);
@@ -1645,7 +1641,7 @@ build_comparison_op (tree fndecl, tsubst
 	    }
 	  tree ceq = contextual_conv_bool (eq, complain);
 	  info.check (ceq);
-	  if (info.defining)
+	  if (defining)
 	    {
 	      finish_if_stmt_cond (ceq, if_);
 	      finish_then_clause (if_);
@@ -1658,7 +1654,7 @@ build_comparison_op (tree fndecl, tsubst
 		finish_for_stmt (TREE_VALUE (loop_index));
 	    }
 	}
-      if (info.defining)
+      if (defining)
 	{
 	  tree val;
 	  if (code == EQ_EXPR)
@@ -1679,7 +1675,7 @@ build_comparison_op (tree fndecl, tsubst
 				NULL_TREE, NULL, complain);
       comp = contextual_conv_bool (comp, complain);
       info.check (comp);
-      if (info.defining)
+      if (defining)
 	{
 	  tree neg = build1 (TRUTH_NOT_EXPR, boolean_type_node, comp);
 	  finish_return_stmt (neg);
@@ -1692,12 +1688,12 @@ build_comparison_op (tree fndecl, tsubst
       tree comp2 = build_new_op (info.loc, code, flags, comp, integer_zero_node,
 				 NULL_TREE, NULL, complain);
       info.check (comp2);
-      if (info.defining)
+      if (defining)
 	finish_return_stmt (comp2);
     }
 
  out:
-  if (info.defining)
+  if (defining)
     finish_compound_stmt (compound_stmt);
   else
     --cp_unevaluated_operand;
@@ -1776,7 +1772,7 @@ synthesize_method (tree fndecl)
   else if (sfk == sfk_comparison)
     {
       /* Pass tf_none so the function is just deleted if there's a problem.  */
-      build_comparison_op (fndecl, tf_none);
+      build_comparison_op (fndecl, true, tf_none);
       need_body = false;
     }
 
@@ -2747,7 +2743,7 @@ maybe_explain_implicit_delete (tree decl
 	  inform (DECL_SOURCE_LOCATION (decl),
 		  "%q#D is implicitly deleted because the default "
 		  "definition would be ill-formed:", decl);
-	  build_comparison_op (decl, tf_warning_or_error);
+	  build_comparison_op (decl, false, tf_warning_or_error);
 	}
       else if (!informed)
 	{
@@ -2808,7 +2804,7 @@ explain_implicit_non_constexpr (tree dec
   if (sfk == sfk_comparison)
     {
       DECL_DECLARED_CONSTEXPR_P (decl) = true;
-      build_comparison_op (decl, tf_warning_or_error);
+      build_comparison_op (decl, false, tf_warning_or_error);
       DECL_DECLARED_CONSTEXPR_P (decl) = false;
     }
   else
--- gcc/cp/class.c.jj	2021-09-30 09:22:46.299918499 +0200
+++ gcc/cp/class.c	2021-09-30 18:40:52.330425343 +0200
@@ -6133,7 +6133,8 @@ check_bases_and_members (tree t)
 		 no longer ill-formed, it is defined as deleted instead.  */
 	      DECL_DELETED_FN (fn) = true;
 	  }
-	defaulted_late_check (fn);
+	if (special_function_p (fn) != sfk_comparison)
+	  defaulted_late_check (fn);
       }
 
   if (LAMBDA_TYPE_P (t))
@@ -7467,7 +7468,14 @@ finish_struct_1 (tree t)
      for any static member objects of the type we're working on.  */
   for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     if (DECL_DECLARES_FUNCTION_P (x))
-      DECL_IN_AGGR_P (x) = false;
+      {
+	/* Synthesize constexpr defaulted comparisons.  */
+	if (!DECL_ARTIFICIAL (x)
+	    && DECL_DEFAULTED_IN_CLASS_P (x)
+	    && special_function_p (x) == sfk_comparison)
+	  defaulted_late_check (x);
+	DECL_IN_AGGR_P (x) = false;
+      }
     else if (VAR_P (x) && TREE_STATIC (x)
 	     && TREE_TYPE (x) != error_mark_node
 	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
--- gcc/cp/pt.c.jj	2021-09-29 10:07:41.815881043 +0200
+++ gcc/cp/pt.c	2021-09-30 18:48:09.091331517 +0200
@@ -25765,6 +25765,25 @@ maybe_instantiate_noexcept (tree fn, tsu
 	/* We're in start_preparsed_function, keep going.  */
 	return true;
 
+      if (special_function_p (fn) == sfk_comparison)
+	{
+	  tree lhs = DECL_ARGUMENTS (fn);
+	  if (is_this_parameter (lhs))
+	    lhs = cp_build_fold_indirect_ref (lhs);
+	  else
+	    lhs = convert_from_reference (lhs);
+	  tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
+	  /* If the comparison type is still incomplete, don't synthesize the
+	     method, just see if it is not implicitly deleted.  */
+	  if (!COMPLETE_TYPE_P (ctype))
+	    {
+	      push_deferring_access_checks (dk_no_deferred);
+	      build_comparison_op (fn, false, tf_none);
+	      pop_deferring_access_checks ();
+	      return !DECL_MAYBE_DELETED (fn);
+	    }
+	}
+
       ++function_depth;
       synthesize_method (fn);
       --function_depth;
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C.jj	2020-07-28 15:39:10.012756173 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C	2021-09-30 19:02:22.107430284 +0200
@@ -1,7 +1,18 @@
 // PR c++/94907
 // { dg-do compile { target c++20 } }
 
-namespace std { struct strong_ordering { }; }
+namespace std { struct strong_ordering {
+  int _v;
+  constexpr strong_ordering (int v) :_v(v) {}
+  constexpr operator int (void) const { return _v; }
+  static const strong_ordering less;
+  static const strong_ordering equal;
+  static const strong_ordering greater;
+};
+constexpr strong_ordering strong_ordering::less = -1;
+constexpr strong_ordering strong_ordering::equal = 0;
+constexpr strong_ordering strong_ordering::greater = 1;
+}
 
 struct E;
 struct D {
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-30 18:34:22.897858916 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-30 18:34:22.897858916 +0200
@@ -0,0 +1,43 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+
+struct A
+{
+  unsigned char a : 1;
+  unsigned char b : 1;
+  constexpr bool operator== (const A &) const = default;
+};
+
+struct B
+{
+  unsigned char a : 8;
+  int : 0;
+  unsigned char b : 7;
+  constexpr bool operator== (const B &) const = default;
+};
+
+struct C
+{
+  unsigned char a : 3;
+  unsigned char b : 1;
+  constexpr bool operator== (const C &) const = default;
+};
+
+void
+foo (C &x, int y)
+{
+  x.b = y;
+}
+
+int
+main ()
+{
+  A a{}, b{};
+  B c{}, d{};
+  C e{}, f{};
+  a.b = 1;
+  d.b = 1;
+  foo (e, 0);
+  foo (f, 1);
+  return a == b || c == d || e == f;
+}
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-30 18:34:22.897858916 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-30 18:34:22.897858916 +0200
@@ -0,0 +1,5 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+// { dg-options "-O2" }
+
+#include "spaceship-eq11.C"


	Jakub

Re: [PATCH, v3] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jason Merrill]

On 9/30/21 13:24, Jakub Jelinek wrote:
> On Wed, Sep 29, 2021 at 03:38:45PM -0400, Jason Merrill wrote:
>> It ought to be possible to defer check_final_overrider, but it sounds
>> awkward.
>>
>> Or maybe_instantiate_noexcept could use the non-defining path in
>> build_comparison_op.
>>
>> Maybe we want a maybe_synthesize_method that actually builds the function if
>> the type is complete, or takes the non-defining path if not.
> 
> So something like this?

Much like, thanks.

> spaceship-synth8.C (apparently added by me, so how valid/invalid it is is
> unclear) now doesn't ICE anymore, but without the change I've put there
> is now rejected because std::strong_ordering::less etc. aren't found.
> Previously when we were synthesizing it we did that before the FIELD_DECLs
> for bases have been added, so those weren't compared, but now they actually
> are compared.

Ah, good to have that fixed.

> After fixing the incomplete std::strong_ordering spaceship-synth8.C is now
> accepted, but I'm afraid I'm getting lost in this - clang++ rejects that
> testcase instead complaining that D has <=> operator, but has it pure virtual.

Ah, I think we need to add LOOKUP_NO_VIRTUAL to the flags variable, as 
we do in do_build_copy_assign.  I suppose it wouldn't hurt to add 
LOOKUP_DEFAULTED as well.

One more comment in the patch below.

> And, when maybe_instantiate_noexcept tries to find out if the defaulted
> method would be implicitly deleted or not, when it does so before the class
> is complete, seems it can check whether there are errors when comparing the
> direct members of the class, but not errors about bases...
> 
> 2021-09-30  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/102490
> 	* cp-tree.h (build_comparison_op): Declare.
> 	* method.c (comp_info): Remove defining member.
> 	(comp_info::comp_info): Remove complain argument, don't initialize
> 	defining.
> 	(build_comparison_op): No longer static.  Add defining argument.
> 	Adjust comp_info construction.  Use defining instead of info.defining.
> 	Assert that if defining, ctype is a complete type.
> 	(synthesize_method, maybe_explain_implicit_delete,
> 	explain_implicit_non_constexpr): Adjust build_comparison_op callers.
> 	* class.c (check_bases_and_members): Don't call defaulted_late_check
> 	for sfk_comparison.
> 	(finish_struct_1): Call it here instead after class has been completed.
> 	* pt.c (maybe_instantiate_noexcept): For sfk_comparison of still
> 	incomplete classes, call build_comparison_op in non-defining mode
> 	instead of calling synthesize_method.
> 
> 	* g++.dg/cpp2a/spaceship-synth8.C (std::strong_ordering): Provide more
> 	complete definition.
> 	(std::strong_ordering::less, std::strong_ordering::equal,
> 	std::strong_ordering::greater): Define.
> 	* g++.dg/cpp2a/spaceship-eq11.C: New test.
> 	* g++.dg/cpp2a/spaceship-eq12.C: New test.
> 
> --- gcc/cp/cp-tree.h.jj	2021-09-18 09:44:31.728743713 +0200
> +++ gcc/cp/cp-tree.h	2021-09-30 18:39:10.416847290 +0200
> @@ -7012,6 +7012,7 @@ extern bool maybe_explain_implicit_delet
>   extern void explain_implicit_non_constexpr	(tree);
>   extern bool deduce_inheriting_ctor		(tree);
>   extern bool decl_remember_implicit_trigger_p	(tree);
> +extern void build_comparison_op			(tree, bool, tsubst_flags_t);
>   extern void synthesize_method			(tree);
>   extern tree lazily_declare_fn			(special_function_kind,
>   						 tree);
> --- gcc/cp/method.c.jj	2021-09-30 09:22:46.323918164 +0200
> +++ gcc/cp/method.c	2021-09-30 18:51:14.510744549 +0200
> @@ -1288,21 +1288,16 @@ struct comp_info
>   {
>     tree fndecl;
>     location_t loc;
> -  bool defining;
>     bool first_time;
>     bool constexp;
>     bool was_constexp;
>     bool noex;
>   
> -  comp_info (tree fndecl, tsubst_flags_t &complain)
> +  comp_info (tree fndecl)
>       : fndecl (fndecl)
>     {
>       loc = DECL_SOURCE_LOCATION (fndecl);
>   
> -    /* We only have tf_error set when we're called from
> -       explain_invalid_constexpr_fn or maybe_explain_implicit_delete.  */
> -    defining = !(complain & tf_error);
> -
>       first_time = DECL_MAYBE_DELETED (fndecl);
>       DECL_MAYBE_DELETED (fndecl) = false;
>   
> @@ -1364,12 +1359,12 @@ struct comp_info
>      to use synthesize_method at the earliest opportunity and bail out if the
>      function ends up being deleted.  */
>   
> -static void
> -build_comparison_op (tree fndecl, tsubst_flags_t complain)
> +void
> +build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
>   {
> -  comp_info info (fndecl, complain);
> +  comp_info info (fndecl);
>   
> -  if (!info.defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
> +  if (!defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
>       return;
>   
>     int flags = LOOKUP_NORMAL;
> @@ -1384,6 +1379,7 @@ build_comparison_op (tree fndecl, tsubst
>       lhs = convert_from_reference (lhs);
>     rhs = convert_from_reference (rhs);
>     tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
> +  gcc_assert (!defining || COMPLETE_TYPE_P (ctype));
>   
>     iloc_sentinel ils (info.loc);
>   
> @@ -1399,7 +1395,7 @@ build_comparison_op (tree fndecl, tsubst
>       }
>   
>     tree compound_stmt = NULL_TREE;
> -  if (info.defining)
> +  if (defining)
>       compound_stmt = begin_compound_stmt (0);
>     else
>       ++cp_unevaluated_operand;
> @@ -1474,8 +1470,8 @@ build_comparison_op (tree fndecl, tsubst
>   		break;
>   	      tree idx;
>   	      /* [1] array, no loop needed, just add [0] ARRAY_REF.
> -		 Similarly if !info.defining.  */
> -	      if (integer_zerop (maxval) || !info.defining)
> +		 Similarly if !defining.  */
> +	      if (integer_zerop (maxval) || !defining)
>   		idx = size_zero_node;
>   	      /* Some other array, will need runtime loop.  */
>   	      else
> @@ -1584,7 +1580,7 @@ build_comparison_op (tree fndecl, tsubst
>   	  tree comp = comps[i];
>   	  tree eq, retval = NULL_TREE, if_ = NULL_TREE;
>   	  tree loop_indexes = NULL_TREE;
> -	  if (info.defining)
> +	  if (defining)
>   	    {
>   	      if (TREE_CODE (comp) == TREE_LIST)
>   		{
> @@ -1632,7 +1628,7 @@ build_comparison_op (tree fndecl, tsubst
>   		comp = build_static_cast (input_location, rettype, comp,
>   					  complain);
>   	      info.check (comp);
> -	      if (info.defining)
> +	      if (defining)
>   		{
>   		  tree var = create_temporary_var (rettype);
>   		  pushdecl (var);
> @@ -1645,7 +1641,7 @@ build_comparison_op (tree fndecl, tsubst
>   	    }
>   	  tree ceq = contextual_conv_bool (eq, complain);
>   	  info.check (ceq);
> -	  if (info.defining)
> +	  if (defining)
>   	    {
>   	      finish_if_stmt_cond (ceq, if_);
>   	      finish_then_clause (if_);
> @@ -1658,7 +1654,7 @@ build_comparison_op (tree fndecl, tsubst
>   		finish_for_stmt (TREE_VALUE (loop_index));
>   	    }
>   	}
> -      if (info.defining)
> +      if (defining)
>   	{
>   	  tree val;
>   	  if (code == EQ_EXPR)
> @@ -1679,7 +1675,7 @@ build_comparison_op (tree fndecl, tsubst
>   				NULL_TREE, NULL, complain);
>         comp = contextual_conv_bool (comp, complain);
>         info.check (comp);
> -      if (info.defining)
> +      if (defining)
>   	{
>   	  tree neg = build1 (TRUTH_NOT_EXPR, boolean_type_node, comp);
>   	  finish_return_stmt (neg);
> @@ -1692,12 +1688,12 @@ build_comparison_op (tree fndecl, tsubst
>         tree comp2 = build_new_op (info.loc, code, flags, comp, integer_zero_node,
>   				 NULL_TREE, NULL, complain);
>         info.check (comp2);
> -      if (info.defining)
> +      if (defining)
>   	finish_return_stmt (comp2);
>       }
>   
>    out:
> -  if (info.defining)
> +  if (defining)
>       finish_compound_stmt (compound_stmt);
>     else
>       --cp_unevaluated_operand;
> @@ -1776,7 +1772,7 @@ synthesize_method (tree fndecl)
>     else if (sfk == sfk_comparison)
>       {
>         /* Pass tf_none so the function is just deleted if there's a problem.  */
> -      build_comparison_op (fndecl, tf_none);
> +      build_comparison_op (fndecl, true, tf_none);
>         need_body = false;
>       }
>   
> @@ -2747,7 +2743,7 @@ maybe_explain_implicit_delete (tree decl
>   	  inform (DECL_SOURCE_LOCATION (decl),
>   		  "%q#D is implicitly deleted because the default "
>   		  "definition would be ill-formed:", decl);
> -	  build_comparison_op (decl, tf_warning_or_error);
> +	  build_comparison_op (decl, false, tf_warning_or_error);
>   	}
>         else if (!informed)
>   	{
> @@ -2808,7 +2804,7 @@ explain_implicit_non_constexpr (tree dec
>     if (sfk == sfk_comparison)
>       {
>         DECL_DECLARED_CONSTEXPR_P (decl) = true;
> -      build_comparison_op (decl, tf_warning_or_error);
> +      build_comparison_op (decl, false, tf_warning_or_error);
>         DECL_DECLARED_CONSTEXPR_P (decl) = false;
>       }
>     else
> --- gcc/cp/class.c.jj	2021-09-30 09:22:46.299918499 +0200
> +++ gcc/cp/class.c	2021-09-30 18:40:52.330425343 +0200
> @@ -6133,7 +6133,8 @@ check_bases_and_members (tree t)
>   		 no longer ill-formed, it is defined as deleted instead.  */
>   	      DECL_DELETED_FN (fn) = true;
>   	  }
> -	defaulted_late_check (fn);
> +	if (special_function_p (fn) != sfk_comparison)
> +	  defaulted_late_check (fn);
>         }
>   
>     if (LAMBDA_TYPE_P (t))
> @@ -7467,7 +7468,14 @@ finish_struct_1 (tree t)
>        for any static member objects of the type we're working on.  */
>     for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
>       if (DECL_DECLARES_FUNCTION_P (x))
> -      DECL_IN_AGGR_P (x) = false;
> +      {
> +	/* Synthesize constexpr defaulted comparisons.  */
> +	if (!DECL_ARTIFICIAL (x)
> +	    && DECL_DEFAULTED_IN_CLASS_P (x)
> +	    && special_function_p (x) == sfk_comparison)
> +	  defaulted_late_check (x);
> +	DECL_IN_AGGR_P (x) = false;
> +      }
>       else if (VAR_P (x) && TREE_STATIC (x)
>   	     && TREE_TYPE (x) != error_mark_node
>   	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
> --- gcc/cp/pt.c.jj	2021-09-29 10:07:41.815881043 +0200
> +++ gcc/cp/pt.c	2021-09-30 18:48:09.091331517 +0200
> @@ -25765,6 +25765,25 @@ maybe_instantiate_noexcept (tree fn, tsu
>   	/* We're in start_preparsed_function, keep going.  */
>   	return true;
>   
> +      if (special_function_p (fn) == sfk_comparison)
> +	{
> +	  tree lhs = DECL_ARGUMENTS (fn);
> +	  if (is_this_parameter (lhs))
> +	    lhs = cp_build_fold_indirect_ref (lhs);
> +	  else
> +	    lhs = convert_from_reference (lhs);
> +	  tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
> +	  /* If the comparison type is still incomplete, don't synthesize the
> +	     method, just see if it is not implicitly deleted.  */
> +	  if (!COMPLETE_TYPE_P (ctype))
> +	    {
> +	      push_deferring_access_checks (dk_no_deferred);
> +	      build_comparison_op (fn, false, tf_none);
> +	      pop_deferring_access_checks ();
> +	      return !DECL_MAYBE_DELETED (fn);
> +	    }
> +	}
> +
>         ++function_depth;
>         synthesize_method (fn);
>         --function_depth;

Let's factor this (from the added code to here) into a 
maybe_synthesize_method in method.c.  That way build_comparison_op can 
stay static.

> --- gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C.jj	2020-07-28 15:39:10.012756173 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C	2021-09-30 19:02:22.107430284 +0200
> @@ -1,7 +1,18 @@
>   // PR c++/94907
>   // { dg-do compile { target c++20 } }
>   
> -namespace std { struct strong_ordering { }; }
> +namespace std { struct strong_ordering {
> +  int _v;
> +  constexpr strong_ordering (int v) :_v(v) {}
> +  constexpr operator int (void) const { return _v; }
> +  static const strong_ordering less;
> +  static const strong_ordering equal;
> +  static const strong_ordering greater;
> +};
> +constexpr strong_ordering strong_ordering::less = -1;
> +constexpr strong_ordering strong_ordering::equal = 0;
> +constexpr strong_ordering strong_ordering::greater = 1;
> +}
>   
>   struct E;
>   struct D {
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-09-30 18:34:22.897858916 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-09-30 18:34:22.897858916 +0200
> @@ -0,0 +1,43 @@
> +// PR c++/102490
> +// { dg-do run { target c++20 } }
> +
> +struct A
> +{
> +  unsigned char a : 1;
> +  unsigned char b : 1;
> +  constexpr bool operator== (const A &) const = default;
> +};
> +
> +struct B
> +{
> +  unsigned char a : 8;
> +  int : 0;
> +  unsigned char b : 7;
> +  constexpr bool operator== (const B &) const = default;
> +};
> +
> +struct C
> +{
> +  unsigned char a : 3;
> +  unsigned char b : 1;
> +  constexpr bool operator== (const C &) const = default;
> +};
> +
> +void
> +foo (C &x, int y)
> +{
> +  x.b = y;
> +}
> +
> +int
> +main ()
> +{
> +  A a{}, b{};
> +  B c{}, d{};
> +  C e{}, f{};
> +  a.b = 1;
> +  d.b = 1;
> +  foo (e, 0);
> +  foo (f, 1);
> +  return a == b || c == d || e == f;
> +}
> --- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-09-30 18:34:22.897858916 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-09-30 18:34:22.897858916 +0200
> @@ -0,0 +1,5 @@
> +// PR c++/102490
> +// { dg-do run { target c++20 } }
> +// { dg-options "-O2" }
> +
> +#include "spaceship-eq11.C"
> 
> 
> 	Jakub
> 

[PATCH, v4] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Thu, Sep 30, 2021 at 03:01:49PM -0400, Jason Merrill wrote:
> > After fixing the incomplete std::strong_ordering spaceship-synth8.C is now
> > accepted, but I'm afraid I'm getting lost in this - clang++ rejects that
> > testcase instead complaining that D has <=> operator, but has it pure virtual.
> 
> Ah, I think we need to add LOOKUP_NO_VIRTUAL to the flags variable, as we do
> in do_build_copy_assign.  I suppose it wouldn't hurt to add LOOKUP_DEFAULTED
> as well.

I've tried that (see patch below), but neither in build_comparison_op, nor
in genericize_spaceship those changes made any difference for
spaceship-synth8.C, it is still accepted instead of rejected.

> > +      if (special_function_p (fn) == sfk_comparison)
> > +	{
> > +	  tree lhs = DECL_ARGUMENTS (fn);
> > +	  if (is_this_parameter (lhs))
> > +	    lhs = cp_build_fold_indirect_ref (lhs);
> > +	  else
> > +	    lhs = convert_from_reference (lhs);
> > +	  tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
> > +	  /* If the comparison type is still incomplete, don't synthesize the
> > +	     method, just see if it is not implicitly deleted.  */
> > +	  if (!COMPLETE_TYPE_P (ctype))
> > +	    {
> > +	      push_deferring_access_checks (dk_no_deferred);
> > +	      build_comparison_op (fn, false, tf_none);
> > +	      pop_deferring_access_checks ();
> > +	      return !DECL_MAYBE_DELETED (fn);
> > +	    }
> > +	}
> > +
> >         ++function_depth;
> >         synthesize_method (fn);
> >         --function_depth;
> 
> Let's factor this (from the added code to here) into a
> maybe_synthesize_method in method.c.  That way build_comparison_op can stay
> static.

Ok, done.
In addition, I've added the testcases from PR98712 to the patch.
I'm still worried about maybe_synthesize_method, if done e.g. from
maybe_instantiate_noexcept before the class is COMPLETE_TYPE_P, could
end up deducing a wrong return type, one that e.g. doesn't take into account
base classes.  Tried that with spaceship-synth13.C testcase, but there
maybe_instantiate_noexcept isn't called early, and I'm out of ideas how to do
that.  Also, do maybe_instantiate_noexcept callers care just about the
return type of the method or also about whether something in the body could
throw?

2021-10-01  Jakub Jelinek  <jakub@redhat.com>

	PR c++/98712
	PR c++/102490
	* cp-tree.h (maybe_synthesize_method): Declare.
	* method.c (genericize_spaceship): Use
	LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED instead of
	LOOKUP_NORMAL for flags.
	(comp_info): Remove defining member.
	(comp_info::comp_info): Remove complain argument, don't initialize
	defining.
	(build_comparison_op): Add defining argument. Adjust comp_info
	construction.  Use defining instead of info.defining.  Assert that if
	defining, ctype is a complete type.  Use
	LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED instead of
	LOOKUP_NORMAL for flags.
	(synthesize_method, maybe_explain_implicit_delete,
	explain_implicit_non_constexpr): Adjust build_comparison_op callers.
	(maybe_synthesize_method): New function.
	* class.c (check_bases_and_members): Don't call defaulted_late_check
	for sfk_comparison.
	(finish_struct_1): Call it here instead after class has been completed.
	* pt.c (maybe_instantiate_noexcept): Call maybe_synthesize_method
	instead of synthesize_method.

	* g++.dg/cpp2a/spaceship-synth8.C (std::strong_ordering): Provide more
	complete definition.
	(std::strong_ordering::less, std::strong_ordering::equal,
	std::strong_ordering::greater): Define.
	* g++.dg/cpp2a/spaceship-synth12.C: New test.
	* g++.dg/cpp2a/spaceship-synth13.C: New test.
	* g++.dg/cpp2a/spaceship-eq11.C: New test.
	* g++.dg/cpp2a/spaceship-eq12.C: New test.
	* g++.dg/cpp2a/spaceship-eq13.C: New test.

--- gcc/cp/cp-tree.h.jj	2021-10-01 10:24:37.500266902 +0200
+++ gcc/cp/cp-tree.h	2021-10-01 16:40:08.880795343 +0200
@@ -7013,6 +7013,7 @@ extern void explain_implicit_non_constex
 extern bool deduce_inheriting_ctor		(tree);
 extern bool decl_remember_implicit_trigger_p	(tree);
 extern void synthesize_method			(tree);
+extern void maybe_synthesize_method		(tree);
 extern tree lazily_declare_fn			(special_function_kind,
 						 tree);
 extern tree skip_artificial_parms_for		(const_tree, tree);
--- gcc/cp/method.c.jj	2021-10-01 10:24:58.312971997 +0200
+++ gcc/cp/method.c	2021-10-01 16:46:18.018588835 +0200
@@ -1098,7 +1098,7 @@ genericize_spaceship (location_t loc, tr
 
   tree gt = lookup_comparison_result (tag, type, 1);
 
-  int flags = LOOKUP_NORMAL;
+  int flags = LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED;
   tsubst_flags_t complain = tf_none;
   tree comp;
 
@@ -1288,21 +1288,16 @@ struct comp_info
 {
   tree fndecl;
   location_t loc;
-  bool defining;
   bool first_time;
   bool constexp;
   bool was_constexp;
   bool noex;
 
-  comp_info (tree fndecl, tsubst_flags_t &complain)
+  comp_info (tree fndecl)
     : fndecl (fndecl)
   {
     loc = DECL_SOURCE_LOCATION (fndecl);
 
-    /* We only have tf_error set when we're called from
-       explain_invalid_constexpr_fn or maybe_explain_implicit_delete.  */
-    defining = !(complain & tf_error);
-
     first_time = DECL_MAYBE_DELETED (fndecl);
     DECL_MAYBE_DELETED (fndecl) = false;
 
@@ -1364,15 +1359,15 @@ struct comp_info
    to use synthesize_method at the earliest opportunity and bail out if the
    function ends up being deleted.  */
 
-static void
-build_comparison_op (tree fndecl, tsubst_flags_t complain)
+void
+build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
 {
-  comp_info info (fndecl, complain);
+  comp_info info (fndecl);
 
-  if (!info.defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
+  if (!defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
     return;
 
-  int flags = LOOKUP_NORMAL;
+  int flags = LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED;
   const ovl_op_info_t *op = IDENTIFIER_OVL_OP_INFO (DECL_NAME (fndecl));
   tree_code code = op->tree_code;
 
@@ -1384,6 +1379,7 @@ build_comparison_op (tree fndecl, tsubst
     lhs = convert_from_reference (lhs);
   rhs = convert_from_reference (rhs);
   tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
+  gcc_assert (!defining || COMPLETE_TYPE_P (ctype));
 
   iloc_sentinel ils (info.loc);
 
@@ -1399,7 +1395,7 @@ build_comparison_op (tree fndecl, tsubst
     }
 
   tree compound_stmt = NULL_TREE;
-  if (info.defining)
+  if (defining)
     compound_stmt = begin_compound_stmt (0);
   else
     ++cp_unevaluated_operand;
@@ -1478,8 +1474,8 @@ build_comparison_op (tree fndecl, tsubst
 		break;
 	      tree idx;
 	      /* [1] array, no loop needed, just add [0] ARRAY_REF.
-		 Similarly if !info.defining.  */
-	      if (integer_zerop (maxval) || !info.defining)
+		 Similarly if !defining.  */
+	      if (integer_zerop (maxval) || !defining)
 		idx = size_zero_node;
 	      /* Some other array, will need runtime loop.  */
 	      else
@@ -1588,7 +1584,7 @@ build_comparison_op (tree fndecl, tsubst
 	  tree comp = comps[i];
 	  tree eq, retval = NULL_TREE, if_ = NULL_TREE;
 	  tree loop_indexes = NULL_TREE;
-	  if (info.defining)
+	  if (defining)
 	    {
 	      if (TREE_CODE (comp) == TREE_LIST)
 		{
@@ -1636,7 +1632,7 @@ build_comparison_op (tree fndecl, tsubst
 		comp = build_static_cast (input_location, rettype, comp,
 					  complain);
 	      info.check (comp);
-	      if (info.defining)
+	      if (defining)
 		{
 		  tree var = create_temporary_var (rettype);
 		  pushdecl (var);
@@ -1649,7 +1645,7 @@ build_comparison_op (tree fndecl, tsubst
 	    }
 	  tree ceq = contextual_conv_bool (eq, complain);
 	  info.check (ceq);
-	  if (info.defining)
+	  if (defining)
 	    {
 	      finish_if_stmt_cond (ceq, if_);
 	      finish_then_clause (if_);
@@ -1662,7 +1658,7 @@ build_comparison_op (tree fndecl, tsubst
 		finish_for_stmt (TREE_VALUE (loop_index));
 	    }
 	}
-      if (info.defining)
+      if (defining)
 	{
 	  tree val;
 	  if (code == EQ_EXPR)
@@ -1683,7 +1679,7 @@ build_comparison_op (tree fndecl, tsubst
 				NULL_TREE, NULL, complain);
       comp = contextual_conv_bool (comp, complain);
       info.check (comp);
-      if (info.defining)
+      if (defining)
 	{
 	  tree neg = build1 (TRUTH_NOT_EXPR, boolean_type_node, comp);
 	  finish_return_stmt (neg);
@@ -1696,12 +1692,12 @@ build_comparison_op (tree fndecl, tsubst
       tree comp2 = build_new_op (info.loc, code, flags, comp, integer_zero_node,
 				 NULL_TREE, NULL, complain);
       info.check (comp2);
-      if (info.defining)
+      if (defining)
 	finish_return_stmt (comp2);
     }
 
  out:
-  if (info.defining)
+  if (defining)
     finish_compound_stmt (compound_stmt);
   else
     --cp_unevaluated_operand;
@@ -1780,7 +1776,7 @@ synthesize_method (tree fndecl)
   else if (sfk == sfk_comparison)
     {
       /* Pass tf_none so the function is just deleted if there's a problem.  */
-      build_comparison_op (fndecl, tf_none);
+      build_comparison_op (fndecl, true, tf_none);
       need_body = false;
     }
 
@@ -1814,6 +1810,32 @@ synthesize_method (tree fndecl)
 	      fndecl);
 }
 
+/* Like synthesize_method, but don't actually synthesize defaulted comparison
+   methods if their class is still incomplete.  Just deduce the return
+   type in that case.  */
+
+void
+maybe_synthesize_method (tree fndecl)
+{
+  if (special_function_p (fndecl) == sfk_comparison)
+    {
+      tree lhs = DECL_ARGUMENTS (fndecl);
+      if (is_this_parameter (lhs))
+	lhs = cp_build_fold_indirect_ref (lhs);
+      else
+	lhs = convert_from_reference (lhs);
+      tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
+      if (!COMPLETE_TYPE_P (ctype))
+	{
+	  push_deferring_access_checks (dk_no_deferred);
+	  build_comparison_op (fndecl, false, tf_none);
+	  pop_deferring_access_checks ();
+	  return;
+	}
+    }
+  return synthesize_method (fndecl);
+}
+
 /* Build a reference to type TYPE with cv-quals QUALS, which is an
    rvalue if RVALUE is true.  */
 
@@ -2753,7 +2775,7 @@ maybe_explain_implicit_delete (tree decl
 	  inform (DECL_SOURCE_LOCATION (decl),
 		  "%q#D is implicitly deleted because the default "
 		  "definition would be ill-formed:", decl);
-	  build_comparison_op (decl, tf_warning_or_error);
+	  build_comparison_op (decl, false, tf_warning_or_error);
 	}
       else if (!informed)
 	{
@@ -2814,7 +2836,7 @@ explain_implicit_non_constexpr (tree dec
   if (sfk == sfk_comparison)
     {
       DECL_DECLARED_CONSTEXPR_P (decl) = true;
-      build_comparison_op (decl, tf_warning_or_error);
+      build_comparison_op (decl, false, tf_warning_or_error);
       DECL_DECLARED_CONSTEXPR_P (decl) = false;
     }
   else
--- gcc/cp/class.c.jj	2021-10-01 10:24:37.440267752 +0200
+++ gcc/cp/class.c	2021-10-01 16:30:39.900821821 +0200
@@ -6133,7 +6133,8 @@ check_bases_and_members (tree t)
 		 no longer ill-formed, it is defined as deleted instead.  */
 	      DECL_DELETED_FN (fn) = true;
 	  }
-	defaulted_late_check (fn);
+	if (special_function_p (fn) != sfk_comparison)
+	  defaulted_late_check (fn);
       }
 
   if (LAMBDA_TYPE_P (t))
@@ -7467,7 +7468,14 @@ finish_struct_1 (tree t)
      for any static member objects of the type we're working on.  */
   for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     if (DECL_DECLARES_FUNCTION_P (x))
-      DECL_IN_AGGR_P (x) = false;
+      {
+	/* Synthesize constexpr defaulted comparisons.  */
+	if (!DECL_ARTIFICIAL (x)
+	    && DECL_DEFAULTED_IN_CLASS_P (x)
+	    && special_function_p (x) == sfk_comparison)
+	  defaulted_late_check (x);
+	DECL_IN_AGGR_P (x) = false;
+      }
     else if (VAR_P (x) && TREE_STATIC (x)
 	     && TREE_TYPE (x) != error_mark_node
 	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
--- gcc/cp/pt.c.jj	2021-10-01 10:24:37.639264932 +0200
+++ gcc/cp/pt.c	2021-10-01 16:41:01.157058011 +0200
@@ -25766,7 +25766,7 @@ maybe_instantiate_noexcept (tree fn, tsu
 	return true;
 
       ++function_depth;
-      synthesize_method (fn);
+      maybe_synthesize_method (fn);
       --function_depth;
       return !DECL_MAYBE_DELETED (fn);
     }
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C.jj	2021-10-01 10:24:37.879261532 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C	2021-10-01 16:30:39.905821751 +0200
@@ -1,7 +1,18 @@
 // PR c++/94907
 // { dg-do compile { target c++20 } }
 
-namespace std { struct strong_ordering { }; }
+namespace std { struct strong_ordering {
+  int _v;
+  constexpr strong_ordering (int v) :_v(v) {}
+  constexpr operator int (void) const { return _v; }
+  static const strong_ordering less;
+  static const strong_ordering equal;
+  static const strong_ordering greater;
+};
+constexpr strong_ordering strong_ordering::less = -1;
+constexpr strong_ordering strong_ordering::equal = 0;
+constexpr strong_ordering strong_ordering::greater = 1;
+}
 
 struct E;
 struct D {
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth12.C.jj	2021-10-01 16:35:19.989870012 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth12.C	2021-10-01 16:36:54.691534289 +0200
@@ -0,0 +1,24 @@
+// PR c++/98712
+// { dg-do run { target c++20 } }
+
+#include <compare>
+
+struct S
+{
+  int s = 0;
+  S(int s) : s(s) {}
+  auto operator<=>(const S&) const = default;
+};
+
+struct T : S
+{
+  T(int s) : S(s) {}
+  constexpr auto operator<=>(const T&) const = default;
+};
+
+int
+main()
+{
+  if (T(0) >= T(1))
+    __builtin_abort ();
+}
--- gcc/testsuite/g++.dg/cpp2a/spaceship-synth13.C.jj	2021-10-01 16:36:01.211288600 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-synth13.C	2021-10-01 16:37:14.471255306 +0200
@@ -0,0 +1,29 @@
+// { dg-do compile { target c++20 } }
+
+#include <compare>
+#include <type_traits>
+
+struct E;
+struct D {
+  auto operator<=>(const D&) const = default;
+  float f;
+};
+struct E : D {
+  auto operator<=>(const E&) const = default;
+  int i;
+};
+extern E e1, e2;
+auto a = e1 <=> e2;
+static_assert (std::is_same_v <decltype (a), std::partial_ordering>);
+struct G;
+struct H {
+  constexpr auto operator<=>(const H&) const = default;
+  float f;
+};
+struct G : H {
+  constexpr auto operator<=>(const G&) const = default;
+  int i;
+};
+extern G g1, g2;
+auto c = g1 <=> g2;
+static_assert (std::is_same_v <decltype (c), std::partial_ordering>);
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C.jj	2021-10-01 16:30:39.905821751 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C	2021-10-01 16:30:39.905821751 +0200
@@ -0,0 +1,43 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+
+struct A
+{
+  unsigned char a : 1;
+  unsigned char b : 1;
+  constexpr bool operator== (const A &) const = default;
+};
+
+struct B
+{
+  unsigned char a : 8;
+  int : 0;
+  unsigned char b : 7;
+  constexpr bool operator== (const B &) const = default;
+};
+
+struct C
+{
+  unsigned char a : 3;
+  unsigned char b : 1;
+  constexpr bool operator== (const C &) const = default;
+};
+
+void
+foo (C &x, int y)
+{
+  x.b = y;
+}
+
+int
+main ()
+{
+  A a{}, b{};
+  B c{}, d{};
+  C e{}, f{};
+  a.b = 1;
+  d.b = 1;
+  foo (e, 0);
+  foo (f, 1);
+  return a == b || c == d || e == f;
+}
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C.jj	2021-10-01 16:30:39.905821751 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C	2021-10-01 16:30:39.905821751 +0200
@@ -0,0 +1,5 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+// { dg-options "-O2" }
+
+#include "spaceship-eq11.C"
--- gcc/testsuite/g++.dg/cpp2a/spaceship-eq13.C.jj	2021-10-01 16:33:54.176080628 +0200
+++ gcc/testsuite/g++.dg/cpp2a/spaceship-eq13.C	2021-10-01 16:38:40.436042812 +0200
@@ -0,0 +1,22 @@
+// PR c++/98712
+// { dg-do run { target c++20 } }
+
+struct S
+{
+  int s = 0;
+  S(int s) : s(s) {}
+  bool operator==(const S&) const = default;
+};
+
+struct T : S
+{
+  T(int s) : S(s) {}
+  constexpr bool operator==(const T&) const = default;
+};
+
+int
+main()
+{
+  if (T(0) == T(1))
+    __builtin_abort ();
+}


	Jakub

[PATCH, v5] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

On 10/1/21 11:07, Jakub Jelinek wrote:
> On Thu, Sep 30, 2021 at 03:01:49PM -0400, Jason Merrill wrote:
>>> After fixing the incomplete std::strong_ordering spaceship-synth8.C is now
>>> accepted, but I'm afraid I'm getting lost in this - clang++ rejects that
>>> testcase instead complaining that D has <=> operator, but has it pure virtual.
>>
>> Ah, I think we need to add LOOKUP_NO_VIRTUAL to the flags variable, as we do
>> in do_build_copy_assign.  I suppose it wouldn't hurt to add LOOKUP_DEFAULTED
>> as well.
> 
> I've tried that (see patch below), but neither in build_comparison_op, nor
> in genericize_spaceship those changes made any difference for
> spaceship-synth8.C, it is still accepted instead of rejected.

I've switched to handling bases via binfo as discussed on IRC and added 
spaceship-synth14.C to test proper base handling with virtual <=>. 
Here's what I'm committing:

[-- Attachment #2: 0001-c-defaulted-with-bitfields-PR102490.patch --]
[-- Type: text/x-patch, Size: 24696 bytes --]

From dd8421dd37fad0a3f0a5572bcbf142ff22a79297 Mon Sep 17 00:00:00 2001
From: Jakub Jelinek <jakub@redhat.com>
Date: Fri, 1 Oct 2021 17:07:17 +0200
Subject: [PATCH] c++: defaulted <=> with bitfields [PR102490]
To: gcc-patches@gcc.gnu.org

The testcases in the patch are either miscompiled or ICE with checking,
because the defaulted operator== is synthesized too early (but only if
constexpr), when the corresponding class type is still incomplete type.  The
problem is that at that point the bitfield FIELD_DECLs still have as
TREE_TYPE their underlying type rather than integral type with their
precision and when layout_class_type is called for the class soon after
that, it changes those types but the COMPONENT_REFs type stay the way that
they were during the operator== synthesize_method type and the middle-end is
then upset by the mismatch of types.  As what exact type will be given isn't
just a one liner but quite long code especially for over-sized bitfields, I
think it is best to just not synthesize the comparison operators so early
and call defaulted_late_check for them once again as soon as the class is
complete.

This is also a problem for virtual operator<=>, where we need to compare the
noexcept-specifier to validate the override before the class is complete.
Rather than try to defer that comparison, maybe_instantiate_noexcept now
calls maybe_synthesize_method, which calls build_comparison_op in
non-defining mode if the class isn't complete yet.  In that situation we
also might not have base fields yet, so we look in the binfo for the bases.

Co-authored-by: Jason Merrill <jason@redhat.com>

2021-10-01  Jakub Jelinek  <jakub@redhat.com>

	PR c++/98712
	PR c++/102490
	* cp-tree.h (maybe_synthesize_method): Declare.
	* method.c (genericize_spaceship): Use
	LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED instead of
	LOOKUP_NORMAL for flags.
	(comp_info): Remove defining member.  Add complain, code, retcat.
	(comp_info::comp_info): Adjust.
	(do_one_comp): Split out from build_comparison_op.   Use
	LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED instead of
	LOOKUP_NORMAL for flags.
	(build_comparison_op): Add defining argument. Adjust comp_info
	construction.  Use defining instead of info.defining.  Assert that
	if defining, ctype is a complete type.  Walk base binfos.
	(synthesize_method, maybe_explain_implicit_delete,
	explain_implicit_non_constexpr): Adjust build_comparison_op callers.
	(maybe_synthesize_method): New function.
	* class.c (check_bases_and_members): Don't call defaulted_late_check
	for sfk_comparison.
	(finish_struct_1): Call it here instead after class has been
	completed.
	* pt.c (maybe_instantiate_noexcept): Call maybe_synthesize_method
	instead of synthesize_method.

	* g++.dg/cpp2a/spaceship-synth8.C (std::strong_ordering): Provide
	more complete definition.
	(std::strong_ordering::less, std::strong_ordering::equal,
	std::strong_ordering::greater): Define.
	* g++.dg/cpp2a/spaceship-synth12.C: New test.
	* g++.dg/cpp2a/spaceship-synth13.C: New test.
	* g++.dg/cpp2a/spaceship-synth14.C: New test.
	* g++.dg/cpp2a/spaceship-eq11.C: New test.
	* g++.dg/cpp2a/spaceship-eq12.C: New test.
	* g++.dg/cpp2a/spaceship-eq13.C: New test.
---
 gcc/cp/cp-tree.h                              |   1 +
 gcc/cp/class.c                                |  13 +-
 gcc/cp/method.c                               | 240 +++++++++++-------
 gcc/cp/pt.c                                   |   2 +-
 gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C   |  43 ++++
 gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C   |   5 +
 gcc/testsuite/g++.dg/cpp2a/spaceship-eq13.C   |  22 ++
 .../g++.dg/cpp2a/spaceship-synth12.C          |  24 ++
 .../g++.dg/cpp2a/spaceship-synth13.C          |  29 +++
 .../g++.dg/cpp2a/spaceship-synth14.C          |  26 ++
 gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C |  13 +-
 11 files changed, 328 insertions(+), 90 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-eq13.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-synth12.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-synth13.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-synth14.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1fcd50c64fd..5248ecd8fe7 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7013,6 +7013,7 @@ extern void explain_implicit_non_constexpr	(tree);
 extern bool deduce_inheriting_ctor		(tree);
 extern bool decl_remember_implicit_trigger_p	(tree);
 extern void synthesize_method			(tree);
+extern void maybe_synthesize_method		(tree);
 extern tree lazily_declare_fn			(special_function_kind,
 						 tree);
 extern tree skip_artificial_parms_for		(const_tree, tree);
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index fe225c61a62..59611627d18 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -6119,6 +6119,10 @@ check_bases_and_members (tree t)
 	&& !DECL_ARTIFICIAL (fn)
 	&& DECL_DEFAULTED_IN_CLASS_P (fn))
       {
+	/* ...except handle comparisons later, in finish_struct_1.  */
+	if (special_function_p (fn) == sfk_comparison)
+	  continue;
+
 	int copy = copy_fn_p (fn);
 	if (copy > 0)
 	  {
@@ -7467,7 +7471,14 @@ finish_struct_1 (tree t)
      for any static member objects of the type we're working on.  */
   for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     if (DECL_DECLARES_FUNCTION_P (x))
-      DECL_IN_AGGR_P (x) = false;
+      {
+	/* Synthesize constexpr defaulted comparisons.  */
+	if (!DECL_ARTIFICIAL (x)
+	    && DECL_DEFAULTED_IN_CLASS_P (x)
+	    && special_function_p (x) == sfk_comparison)
+	  defaulted_late_check (x);
+	DECL_IN_AGGR_P (x) = false;
+      }
     else if (VAR_P (x) && TREE_STATIC (x)
 	     && TREE_TYPE (x) != error_mark_node
 	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
diff --git a/gcc/cp/method.c b/gcc/cp/method.c
index c38912a7ce9..1023aefc575 100644
--- a/gcc/cp/method.c
+++ b/gcc/cp/method.c
@@ -1288,21 +1288,19 @@ struct comp_info
 {
   tree fndecl;
   location_t loc;
-  bool defining;
+  tsubst_flags_t complain;
+  tree_code code;
+  comp_cat_tag retcat;
   bool first_time;
   bool constexp;
   bool was_constexp;
   bool noex;
 
-  comp_info (tree fndecl, tsubst_flags_t &complain)
-    : fndecl (fndecl)
+  comp_info (tree fndecl, tsubst_flags_t complain)
+    : fndecl (fndecl), complain (complain)
   {
     loc = DECL_SOURCE_LOCATION (fndecl);
 
-    /* We only have tf_error set when we're called from
-       explain_invalid_constexpr_fn or maybe_explain_implicit_delete.  */
-    defining = !(complain & tf_error);
-
     first_time = DECL_MAYBE_DELETED (fndecl);
     DECL_MAYBE_DELETED (fndecl) = false;
 
@@ -1358,23 +1356,99 @@ struct comp_info
   }
 };
 
+/* Subroutine of build_comparison_op, to compare a single subobject.  */
+
+static tree
+do_one_comp (location_t loc, const comp_info &info, tree sub, tree lhs, tree rhs)
+{
+  const tree_code code = info.code;
+  const tree fndecl = info.fndecl;
+  const comp_cat_tag retcat = info.retcat;
+  const tsubst_flags_t complain = info.complain;
+
+  tree overload = NULL_TREE;
+  int flags = LOOKUP_NORMAL | LOOKUP_NONVIRTUAL | LOOKUP_DEFAULTED;
+  /* If we have an explicit comparison category return type we can fall back
+     to </=, so don't give an error yet if <=> lookup fails.  */
+  bool tentative = retcat != cc_last;
+  tree comp = build_new_op (loc, code, flags, lhs, rhs,
+			    NULL_TREE, &overload,
+			    tentative ? tf_none : complain);
+
+  if (code != SPACESHIP_EXPR)
+    return comp;
+
+  tree rettype = TREE_TYPE (TREE_TYPE (fndecl));
+
+  if (comp == error_mark_node)
+    {
+      if (overload == NULL_TREE && (tentative || complain))
+	{
+	  /* No viable <=>, try using op< and op==.  */
+	  tree lteq = genericize_spaceship (loc, rettype, lhs, rhs);
+	  if (lteq != error_mark_node)
+	    {
+	      /* We found usable < and ==.  */
+	      if (retcat != cc_last)
+		/* Return type is a comparison category, use them.  */
+		comp = lteq;
+	      else if (complain & tf_error)
+		/* Return type is auto, suggest changing it.  */
+		inform (info.loc, "changing the return type from %qs "
+			"to a comparison category type will allow the "
+			"comparison to use %qs and %qs", "auto",
+			"operator<", "operator==");
+	    }
+	  else if (tentative && complain)
+	    /* No usable < and ==, give an error for op<=>.  */
+	    build_new_op (loc, code, flags, lhs, rhs, complain);
+	}
+      if (comp == error_mark_node)
+	return error_mark_node;
+    }
+
+  if (FNDECL_USED_AUTO (fndecl)
+      && cat_tag_for (TREE_TYPE (comp)) == cc_last)
+    {
+      /* The operator function is defined as deleted if ... Ri is not a
+	 comparison category type.  */
+      if (complain & tf_error)
+	inform (loc,
+		"three-way comparison of %qD has type %qT, not a "
+		"comparison category type", sub, TREE_TYPE (comp));
+      return error_mark_node;
+    }
+  else if (!FNDECL_USED_AUTO (fndecl)
+	   && !can_convert (rettype, TREE_TYPE (comp), complain))
+    {
+      if (complain & tf_error)
+	error_at (loc,
+		  "three-way comparison of %qD has type %qT, which "
+		  "does not convert to %qT",
+		  sub, TREE_TYPE (comp), rettype);
+      return error_mark_node;
+    }
+
+  return comp;
+}
+
 /* Build up the definition of a defaulted comparison operator.  Unlike other
    defaulted functions that use synthesized_method_walk to determine whether
    the function is e.g. deleted, for comparisons we use the same code.  We try
    to use synthesize_method at the earliest opportunity and bail out if the
    function ends up being deleted.  */
 
-static void
-build_comparison_op (tree fndecl, tsubst_flags_t complain)
+void
+build_comparison_op (tree fndecl, bool defining, tsubst_flags_t complain)
 {
   comp_info info (fndecl, complain);
 
-  if (!info.defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
+  if (!defining && !(complain & tf_error) && !DECL_MAYBE_DELETED (fndecl))
     return;
 
   int flags = LOOKUP_NORMAL;
   const ovl_op_info_t *op = IDENTIFIER_OVL_OP_INFO (DECL_NAME (fndecl));
-  tree_code code = op->tree_code;
+  tree_code code = info.code = op->tree_code;
 
   tree lhs = DECL_ARGUMENTS (fndecl);
   tree rhs = DECL_CHAIN (lhs);
@@ -1384,6 +1458,7 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
     lhs = convert_from_reference (lhs);
   rhs = convert_from_reference (rhs);
   tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
+  gcc_assert (!defining || COMPLETE_TYPE_P (ctype));
 
   iloc_sentinel ils (info.loc);
 
@@ -1399,7 +1474,7 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
     }
 
   tree compound_stmt = NULL_TREE;
-  if (info.defining)
+  if (defining)
     compound_stmt = begin_compound_stmt (0);
   else
     ++cp_unevaluated_operand;
@@ -1413,21 +1488,42 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
 
   if (code == EQ_EXPR || code == SPACESHIP_EXPR)
     {
-      comp_cat_tag retcat = cc_last;
+      comp_cat_tag &retcat = (info.retcat = cc_last);
       if (code == SPACESHIP_EXPR && !FNDECL_USED_AUTO (fndecl))
 	retcat = cat_tag_for (rettype);
 
       bool bad = false;
       auto_vec<tree> comps;
 
-      /* Compare each of the subobjects.  Note that we get bases from
-	 next_initializable_field because we're past C++17.  */
+      /* Compare the base subobjects.  We handle them this way, rather than in
+	 the field loop below, because maybe_instantiate_noexcept might bring
+	 us here before we've built the base fields.  */
+      for (tree base_binfo : BINFO_BASE_BINFOS (TYPE_BINFO (ctype)))
+	{
+	  tree lhs_base
+	    = build_base_path (PLUS_EXPR, lhs, base_binfo, 0, complain);
+	  tree rhs_base
+	    = build_base_path (PLUS_EXPR, rhs, base_binfo, 0, complain);
+
+	  location_t loc = DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (ctype));
+	  tree comp = do_one_comp (loc, info, BINFO_TYPE (base_binfo),
+				   lhs_base, rhs_base);
+	  if (comp == error_mark_node)
+	    {
+	      bad = true;
+	      continue;
+	    }
+
+	  comps.safe_push (comp);
+	}
+
+      /* Now compare the field subobjects.  */
       for (tree field = next_initializable_field (TYPE_FIELDS (ctype));
 	   field;
 	   field = next_initializable_field (DECL_CHAIN (field)))
 	{
-	  if (DECL_VIRTUAL_P (field))
-	    /* Don't compare vptr fields.  */
+	  if (DECL_VIRTUAL_P (field) || DECL_FIELD_IS_BASE (field))
+	    /* We ignore the vptr, and we already handled bases.  */
 	    continue;
 
 	  tree expr_type = TREE_TYPE (field);
@@ -1478,8 +1574,8 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
 		break;
 	      tree idx;
 	      /* [1] array, no loop needed, just add [0] ARRAY_REF.
-		 Similarly if !info.defining.  */
-	      if (integer_zerop (maxval) || !info.defining)
+		 Similarly if !defining.  */
+	      if (integer_zerop (maxval) || !defining)
 		idx = size_zero_node;
 	      /* Some other array, will need runtime loop.  */
 	      else
@@ -1496,69 +1592,13 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
 	  if (TREE_CODE (expr_type) == ARRAY_TYPE)
 	    continue;
 
-	  tree overload = NULL_TREE;
-	  tree comp = build_new_op (field_loc, code, flags, lhs_mem, rhs_mem,
-				    NULL_TREE, &overload,
-				    retcat != cc_last ? tf_none : complain);
+	  tree comp = do_one_comp (field_loc, info, field, lhs_mem, rhs_mem);
 	  if (comp == error_mark_node)
 	    {
-	      if (overload == NULL_TREE && code == SPACESHIP_EXPR
-		  && (retcat != cc_last || complain))
-		{
-		  tree comptype = (retcat != cc_last ? rettype
-				   : DECL_SAVED_AUTO_RETURN_TYPE (fndecl));
-		  /* No viable <=>, try using op< and op==.  */
-		  tree lteq = genericize_spaceship (field_loc, comptype,
-						    lhs_mem, rhs_mem);
-		  if (lteq != error_mark_node)
-		    {
-		      /* We found usable < and ==.  */
-		      if (retcat != cc_last)
-			/* Return type is a comparison category, use them.  */
-			comp = lteq;
-		      else if (complain & tf_error)
-			/* Return type is auto, suggest changing it.  */
-			inform (info.loc, "changing the return type from %qs "
-				"to a comparison category type will allow the "
-				"comparison to use %qs and %qs", "auto",
-				"operator<", "operator==");
-		    }
-		  else if (retcat != cc_last && complain != tf_none)
-		    /* No usable < and ==, give an error for op<=>.  */
-		    build_new_op (field_loc, code, flags, lhs_mem, rhs_mem,
-				  complain);
-		}
-	      if (comp == error_mark_node)
-		{
-		  bad = true;
-		  continue;
-		}
-	    }
-	  if (code != SPACESHIP_EXPR)
-	    ;
-	  else if (FNDECL_USED_AUTO (fndecl)
-		   && cat_tag_for (TREE_TYPE (comp)) == cc_last)
-	    {
-	      /* The operator function is defined as deleted if ... Ri is not a
-		 comparison category type.  */
-	      if (complain & tf_error)
-		inform (field_loc,
-			"three-way comparison of %qD has type %qT, not a "
-			"comparison category type", field, TREE_TYPE (comp));
-	      bad = true;
-	      continue;
-	    }
-	  else if (!FNDECL_USED_AUTO (fndecl)
-		   && !can_convert (rettype, TREE_TYPE (comp), complain))
-	    {
-	      if (complain & tf_error)
-		error_at (field_loc,
-			  "three-way comparison of %qD has type %qT, which "
-			  "does not convert to %qT",
-			  field, TREE_TYPE (comp), rettype);
 	      bad = true;
 	      continue;
 	    }
+
 	  /* Most of the time, comp is the expression that should be evaluated
 	     to compare the two members.  If the expression needs to be
 	     evaluated more than once in a loop, it will be a TREE_LIST
@@ -1588,7 +1628,7 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
 	  tree comp = comps[i];
 	  tree eq, retval = NULL_TREE, if_ = NULL_TREE;
 	  tree loop_indexes = NULL_TREE;
-	  if (info.defining)
+	  if (defining)
 	    {
 	      if (TREE_CODE (comp) == TREE_LIST)
 		{
@@ -1636,7 +1676,7 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
 		comp = build_static_cast (input_location, rettype, comp,
 					  complain);
 	      info.check (comp);
-	      if (info.defining)
+	      if (defining)
 		{
 		  tree var = create_temporary_var (rettype);
 		  pushdecl (var);
@@ -1649,7 +1689,7 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
 	    }
 	  tree ceq = contextual_conv_bool (eq, complain);
 	  info.check (ceq);
-	  if (info.defining)
+	  if (defining)
 	    {
 	      finish_if_stmt_cond (ceq, if_);
 	      finish_then_clause (if_);
@@ -1662,7 +1702,7 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
 		finish_for_stmt (TREE_VALUE (loop_index));
 	    }
 	}
-      if (info.defining)
+      if (defining)
 	{
 	  tree val;
 	  if (code == EQ_EXPR)
@@ -1683,7 +1723,7 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
 				NULL_TREE, NULL, complain);
       comp = contextual_conv_bool (comp, complain);
       info.check (comp);
-      if (info.defining)
+      if (defining)
 	{
 	  tree neg = build1 (TRUTH_NOT_EXPR, boolean_type_node, comp);
 	  finish_return_stmt (neg);
@@ -1696,12 +1736,12 @@ build_comparison_op (tree fndecl, tsubst_flags_t complain)
       tree comp2 = build_new_op (info.loc, code, flags, comp, integer_zero_node,
 				 NULL_TREE, NULL, complain);
       info.check (comp2);
-      if (info.defining)
+      if (defining)
 	finish_return_stmt (comp2);
     }
 
  out:
-  if (info.defining)
+  if (defining)
     finish_compound_stmt (compound_stmt);
   else
     --cp_unevaluated_operand;
@@ -1780,7 +1820,7 @@ synthesize_method (tree fndecl)
   else if (sfk == sfk_comparison)
     {
       /* Pass tf_none so the function is just deleted if there's a problem.  */
-      build_comparison_op (fndecl, tf_none);
+      build_comparison_op (fndecl, true, tf_none);
       need_body = false;
     }
 
@@ -1814,6 +1854,32 @@ synthesize_method (tree fndecl)
 	      fndecl);
 }
 
+/* Like synthesize_method, but don't actually synthesize defaulted comparison
+   methods if their class is still incomplete.  Just deduce the return
+   type in that case.  */
+
+void
+maybe_synthesize_method (tree fndecl)
+{
+  if (special_function_p (fndecl) == sfk_comparison)
+    {
+      tree lhs = DECL_ARGUMENTS (fndecl);
+      if (is_this_parameter (lhs))
+	lhs = cp_build_fold_indirect_ref (lhs);
+      else
+	lhs = convert_from_reference (lhs);
+      tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
+      if (!COMPLETE_TYPE_P (ctype))
+	{
+	  push_deferring_access_checks (dk_no_deferred);
+	  build_comparison_op (fndecl, false, tf_none);
+	  pop_deferring_access_checks ();
+	  return;
+	}
+    }
+  return synthesize_method (fndecl);
+}
+
 /* Build a reference to type TYPE with cv-quals QUALS, which is an
    rvalue if RVALUE is true.  */
 
@@ -2753,7 +2819,7 @@ maybe_explain_implicit_delete (tree decl)
 	  inform (DECL_SOURCE_LOCATION (decl),
 		  "%q#D is implicitly deleted because the default "
 		  "definition would be ill-formed:", decl);
-	  build_comparison_op (decl, tf_warning_or_error);
+	  build_comparison_op (decl, false, tf_warning_or_error);
 	}
       else if (!informed)
 	{
@@ -2814,7 +2880,7 @@ explain_implicit_non_constexpr (tree decl)
   if (sfk == sfk_comparison)
     {
       DECL_DECLARED_CONSTEXPR_P (decl) = true;
-      build_comparison_op (decl, tf_warning_or_error);
+      build_comparison_op (decl, false, tf_warning_or_error);
       DECL_DECLARED_CONSTEXPR_P (decl) = false;
     }
   else
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 1dcdffe322a..c059bbe58d3 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -25766,7 +25766,7 @@ maybe_instantiate_noexcept (tree fn, tsubst_flags_t complain)
 	return true;
 
       ++function_depth;
-      synthesize_method (fn);
+      maybe_synthesize_method (fn);
       --function_depth;
       return !DECL_MAYBE_DELETED (fn);
     }
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C
new file mode 100644
index 00000000000..b71ed4f8317
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq11.C
@@ -0,0 +1,43 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+
+struct A
+{
+  unsigned char a : 1;
+  unsigned char b : 1;
+  constexpr bool operator== (const A &) const = default;
+};
+
+struct B
+{
+  unsigned char a : 8;
+  int : 0;
+  unsigned char b : 7;
+  constexpr bool operator== (const B &) const = default;
+};
+
+struct C
+{
+  unsigned char a : 3;
+  unsigned char b : 1;
+  constexpr bool operator== (const C &) const = default;
+};
+
+void
+foo (C &x, int y)
+{
+  x.b = y;
+}
+
+int
+main ()
+{
+  A a{}, b{};
+  B c{}, d{};
+  C e{}, f{};
+  a.b = 1;
+  d.b = 1;
+  foo (e, 0);
+  foo (f, 1);
+  return a == b || c == d || e == f;
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C
new file mode 100644
index 00000000000..8346e7e3896
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq12.C
@@ -0,0 +1,5 @@
+// PR c++/102490
+// { dg-do run { target c++20 } }
+// { dg-options "-O2" }
+
+#include "spaceship-eq11.C"
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-eq13.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq13.C
new file mode 100644
index 00000000000..cb521edf7f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-eq13.C
@@ -0,0 +1,22 @@
+// PR c++/98712
+// { dg-do run { target c++20 } }
+
+struct S
+{
+  int s = 0;
+  S(int s) : s(s) {}
+  bool operator==(const S&) const = default;
+};
+
+struct T : S
+{
+  T(int s) : S(s) {}
+  constexpr bool operator==(const T&) const = default;
+};
+
+int
+main()
+{
+  if (T(0) == T(1))
+    __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth12.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth12.C
new file mode 100644
index 00000000000..85b478437b8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth12.C
@@ -0,0 +1,24 @@
+// PR c++/98712
+// { dg-do run { target c++20 } }
+
+#include <compare>
+
+struct S
+{
+  int s = 0;
+  S(int s) : s(s) {}
+  auto operator<=>(const S&) const = default;
+};
+
+struct T : S
+{
+  T(int s) : S(s) {}
+  constexpr auto operator<=>(const T&) const = default;
+};
+
+int
+main()
+{
+  if (T(0) >= T(1))
+    __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth13.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth13.C
new file mode 100644
index 00000000000..ab479d7353e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth13.C
@@ -0,0 +1,29 @@
+// { dg-do compile { target c++20 } }
+
+#include <compare>
+#include <type_traits>
+
+struct E;
+struct D {
+  auto operator<=>(const D&) const = default;
+  float f;
+};
+struct E : D {
+  auto operator<=>(const E&) const = default;
+  int i;
+};
+extern E e1, e2;
+auto a = e1 <=> e2;
+static_assert (std::is_same_v <decltype (a), std::partial_ordering>);
+struct G;
+struct H {
+  constexpr auto operator<=>(const H&) const = default;
+  float f;
+};
+struct G : H {
+  constexpr auto operator<=>(const G&) const = default;
+  int i;
+};
+extern G g1, g2;
+auto c = g1 <=> g2;
+static_assert (std::is_same_v <decltype (c), std::partial_ordering>);
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth14.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth14.C
new file mode 100644
index 00000000000..369d3a3e9dd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth14.C
@@ -0,0 +1,26 @@
+// Test virtual <=>.
+// { dg-do run { target c++20 } }
+
+#include <compare>
+
+struct E;
+struct D {
+  std::partial_ordering operator<=>(const D&) const = default;
+  virtual std::partial_ordering operator<=>(const E&) const = 0;
+  float f;
+  D(float f): f(f) {}
+};
+struct E : D {
+  std::partial_ordering operator<=>(const E&) const override = default;
+  int i;
+  E(float f, int i): D(f), i(i) {}
+};
+
+int main()
+{
+  E e1{0.0,42};
+  E e2{1.0,24};
+  auto a = e1 <=> e2;
+  if (!is_lt (e1 <=> e2))
+    __builtin_abort();
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C
index bd1c4d2af2f..886176522f5 100644
--- a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth8.C
@@ -1,7 +1,18 @@
 // PR c++/94907
 // { dg-do compile { target c++20 } }
 
-namespace std { struct strong_ordering { }; }
+namespace std { struct strong_ordering {
+  int _v;
+  constexpr strong_ordering (int v) :_v(v) {}
+  constexpr operator int (void) const { return _v; }
+  static const strong_ordering less;
+  static const strong_ordering equal;
+  static const strong_ordering greater;
+};
+constexpr strong_ordering strong_ordering::less = -1;
+constexpr strong_ordering strong_ordering::equal = 0;
+constexpr strong_ordering strong_ordering::greater = 1;
+}
 
 struct E;
 struct D {
-- 
2.27.0

Re: [PATCH, v5] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Tue, Oct 05, 2021 at 10:40:45PM -0400, Jason Merrill wrote:
> I've switched to handling bases via binfo as discussed on IRC and added
> spaceship-synth14.C to test proper base handling with virtual <=>. Here's
> what I'm committing:

Thanks a lot.

I see spaceship-synth8.C is accepted without errors (| LOOKUP_NONVIRTUAL |
LOOKUP_DEFAULTED didn't help it for me back when playing with it), but if I add

E e1, e2;
auto x = e1 <=> e2;
at the end of it, it is rejected:
spaceship-synth8.C:26:17: error: use of deleted function ‘virtual constexpr std::strong_ordering E::operator<=>(const E&) const’
   26 | auto x = e1 <=> e2;
      |                 ^~
spaceship-synth8.C:22:24: note: ‘virtual constexpr std::strong_ordering E::operator<=>(const E&) const’ is implicitly deleted because the default definition would be ill-formed:
   22 |   std::strong_ordering operator<=>(const E&) const override = default;
      |                        ^~~~~~~~
spaceship-synth8.C:21:8: error: no match for ‘operator<=>’ (operand types are ‘const D’ and ‘const D’)
   21 | struct E : D {
      |        ^
spaceship-synth8.C:19:32: note: candidate: ‘virtual std::strong_ordering D::operator<=>(const E&) const’ (reversed)
   19 |   virtual std::strong_ordering operator<=>(const struct E&) const = 0;
      |                                ^~~~~~~~
spaceship-synth8.C:19:44: note:   no known conversion for argument 1 from ‘const D’ to ‘const E&’
   19 |   virtual std::strong_ordering operator<=>(const struct E&) const = 0;
      |                                            ^~~~~~~~~~~~~~~

Is that ok (i.e. whether it is accepted or rejected when the operator<=>
is actually not called falls into "no diagnostics required" category)?

Note, before this fix we were accepting it even with those
E e1, e2;
auto x = e1 <=> e2;
lines in there.  Perhaps we want to copy spaceship-synth8.C to another
test that will add those two lines and check for the errors...

	Jakub

Re: [PATCH, v5] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]

On 10/6/21 05:06, Jakub Jelinek wrote:
> On Tue, Oct 05, 2021 at 10:40:45PM -0400, Jason Merrill wrote:
>> I've switched to handling bases via binfo as discussed on IRC and added
>> spaceship-synth14.C to test proper base handling with virtual <=>. Here's
>> what I'm committing:
> 
> Thanks a lot.
> 
> I see spaceship-synth8.C is accepted without errors (| LOOKUP_NONVIRTUAL |
> LOOKUP_DEFAULTED didn't help it for me back when playing with it), but if I add
> 
> E e1, e2;
> auto x = e1 <=> e2;
> at the end of it, it is rejected:
> spaceship-synth8.C:26:17: error: use of deleted function ‘virtual constexpr std::strong_ordering E::operator<=>(const E&) const’
>     26 | auto x = e1 <=> e2;
>        |                 ^~
> spaceship-synth8.C:22:24: note: ‘virtual constexpr std::strong_ordering E::operator<=>(const E&) const’ is implicitly deleted because the default definition would be ill-formed:
>     22 |   std::strong_ordering operator<=>(const E&) const override = default;
>        |                        ^~~~~~~~
> spaceship-synth8.C:21:8: error: no match for ‘operator<=>’ (operand types are ‘const D’ and ‘const D’)
>     21 | struct E : D {
>        |        ^
> spaceship-synth8.C:19:32: note: candidate: ‘virtual std::strong_ordering D::operator<=>(const E&) const’ (reversed)
>     19 |   virtual std::strong_ordering operator<=>(const struct E&) const = 0;
>        |                                ^~~~~~~~
> spaceship-synth8.C:19:44: note:   no known conversion for argument 1 from ‘const D’ to ‘const E&’
>     19 |   virtual std::strong_ordering operator<=>(const struct E&) const = 0;
>        |                                            ^~~~~~~~~~~~~~~
> 
> Is that ok (i.e. whether it is accepted or rejected when the operator<=>
> is actually not called falls into "no diagnostics required" category)?

It's not even NDR, it's implicitly deleted, so it's well-formed until 
called.

> Note, before this fix we were accepting it even with those
> E e1, e2;
> auto x = e1 <=> e2;
> lines in there.  Perhaps we want to copy spaceship-synth8.C to another
> test that will add those two lines and check for the errors...

Done:


[-- Attachment #2: 0001-c-One-more-spaceship-test.patch --]
[-- Type: text/x-patch, Size: 1645 bytes --]

From 15b57c0ffe8ac9d568e76e70244cf723b72b1a82 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Wed, 6 Oct 2021 17:12:02 -0400
Subject: [PATCH] c++: One more spaceship test.
To: gcc-patches@gcc.gnu.org

Jakub suggested adding a variant where we actually try to call the
implicitly deleted operator.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/spaceship-synth8a.C: New test.
---
 .../g++.dg/cpp2a/spaceship-synth8a.C          | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/spaceship-synth8a.C

diff --git a/gcc/testsuite/g++.dg/cpp2a/spaceship-synth8a.C b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth8a.C
new file mode 100644
index 00000000000..42a32da77f9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/spaceship-synth8a.C
@@ -0,0 +1,25 @@
+// PR c++/94907
+// { dg-do compile { target c++20 } }
+
+namespace std { struct strong_ordering {
+  int _v;
+  constexpr strong_ordering (int v) :_v(v) {}
+  constexpr operator int (void) const { return _v; }
+  static const strong_ordering less;
+  static const strong_ordering equal;
+  static const strong_ordering greater;
+};
+constexpr strong_ordering strong_ordering::less = -1;
+constexpr strong_ordering strong_ordering::equal = 0;
+constexpr strong_ordering strong_ordering::greater = 1;
+}
+
+struct E;
+struct D {
+  virtual std::strong_ordering operator<=>(const struct E&) const = 0;
+};
+struct E : D {							       // { dg-error "no match" }
+  std::strong_ordering operator<=>(const E&) const override = default; // { dg-message "default" }
+};
+
+auto x = E() <=> E();		// { dg-error "deleted" }
-- 
2.27.0

Re: [PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Tue, Sep 28, 2021 at 09:49:11AM -0400, Patrick Palka via Gcc-patches wrote:
> > --- gcc/cp/method.c.jj	2021-09-15 08:55:37.563497558 +0200
> > +++ gcc/cp/method.c	2021-09-27 13:48:12.139271830 +0200
> > @@ -3160,8 +3160,11 @@ defaulted_late_check (tree fn)
> >    if (kind == sfk_comparison)
> >      {
> >        /* If the function was declared constexpr, check that the definition
> > -	 qualifies.  Otherwise we can define the function lazily.  */
> > -      if (DECL_DECLARED_CONSTEXPR_P (fn) && !DECL_INITIAL (fn))
> > +	 qualifies.  Otherwise we can define the function lazily.
> > +	 Don't do this if the class type is still incomplete.  */
> > +      if (DECL_DECLARED_CONSTEXPR_P (fn)
> > +	  && !DECL_INITIAL (fn)
> > +	  && COMPLETE_TYPE_P (ctx))
> >  	{
> 
> According to the function comment for defaulted_late_check, won't
> COMPLETE_TYPE_P (ctx) always be false here?

It is true in the call from the following hunk.
The function comment at least to me doesn't imply it is always called on
incomplete types, and defaultable_fn_check also calls it.
> 
> >  	  /* Prevent GC.  */
> >  	  function_depth++;
> > --- gcc/cp/class.c.jj	2021-09-03 09:46:28.801428380 +0200
> > +++ gcc/cp/class.c	2021-09-27 14:07:03.465562255 +0200
> > @@ -7467,7 +7467,14 @@ finish_struct_1 (tree t)
> >       for any static member objects of the type we're working on.  */
> >    for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
> >      if (DECL_DECLARES_FUNCTION_P (x))
> > -      DECL_IN_AGGR_P (x) = false;
> > +      {
> > +	/* Synthetize constexpr defaulted comparisons.  */
> > +	if (!DECL_ARTIFICIAL (x)
> > +	    && DECL_DEFAULTED_IN_CLASS_P (x)
> > +	    && special_function_p (x) == sfk_comparison)
> > +	  defaulted_late_check (x);
> > +	DECL_IN_AGGR_P (x) = false;
> > +      }
> >      else if (VAR_P (x) && TREE_STATIC (x)
> >  	     && TREE_TYPE (x) != error_mark_node
> >  	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))

	Jakub

Re: [PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Patrick Palka]

On Tue, 28 Sep 2021, Jakub Jelinek wrote:

> On Tue, Sep 28, 2021 at 09:49:11AM -0400, Patrick Palka via Gcc-patches wrote:
> > > --- gcc/cp/method.c.jj	2021-09-15 08:55:37.563497558 +0200
> > > +++ gcc/cp/method.c	2021-09-27 13:48:12.139271830 +0200
> > > @@ -3160,8 +3160,11 @@ defaulted_late_check (tree fn)
> > >    if (kind == sfk_comparison)
> > >      {
> > >        /* If the function was declared constexpr, check that the definition
> > > -	 qualifies.  Otherwise we can define the function lazily.  */
> > > -      if (DECL_DECLARED_CONSTEXPR_P (fn) && !DECL_INITIAL (fn))
> > > +	 qualifies.  Otherwise we can define the function lazily.
> > > +	 Don't do this if the class type is still incomplete.  */
> > > +      if (DECL_DECLARED_CONSTEXPR_P (fn)
> > > +	  && !DECL_INITIAL (fn)
> > > +	  && COMPLETE_TYPE_P (ctx))
> > >  	{
> > 
> > According to the function comment for defaulted_late_check, won't
> > COMPLETE_TYPE_P (ctx) always be false here?
> 
> It is true in the call from the following hunk.
> The function comment at least to me doesn't imply it is always called on
> incomplete types, and defaultable_fn_check also calls it.

Ah yeah, sorry for the noise, I misunderstood the function comment.

On a related note I think 'ctx' can also be a NAMESPACE_DECL here in
the case of a defaulted non-member operator<=> (as in the below), for
which I'd expect the added COMPLETE_TYPE_P check to crash, but it looks
like in this case DECL_INITIAL is error_mark_node instead of NULL_TREE
so a crash is averted.  If anyone else was wondering...

  struct A {
    friend constexpr bool operator==(const A&, const A&);
  };

  constexpr bool operator==(const A&, const A&) = default;

> > 
> > >  	  /* Prevent GC.  */
> > >  	  function_depth++;
> > > --- gcc/cp/class.c.jj	2021-09-03 09:46:28.801428380 +0200
> > > +++ gcc/cp/class.c	2021-09-27 14:07:03.465562255 +0200
> > > @@ -7467,7 +7467,14 @@ finish_struct_1 (tree t)
> > >       for any static member objects of the type we're working on.  */
> > >    for (x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
> > >      if (DECL_DECLARES_FUNCTION_P (x))
> > > -      DECL_IN_AGGR_P (x) = false;
> > > +      {
> > > +	/* Synthetize constexpr defaulted comparisons.  */
> > > +	if (!DECL_ARTIFICIAL (x)
> > > +	    && DECL_DEFAULTED_IN_CLASS_P (x)
> > > +	    && special_function_p (x) == sfk_comparison)
> > > +	  defaulted_late_check (x);
> > > +	DECL_IN_AGGR_P (x) = false;
> > > +      }
> > >      else if (VAR_P (x) && TREE_STATIC (x)
> > >  	     && TREE_TYPE (x) != error_mark_node
> > >  	     && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (x)), t))
> 
> 	Jakub
> 
> 

Re: [PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Tue, Sep 28, 2021 at 12:44:58PM -0400, Patrick Palka wrote:
> Ah yeah, sorry for the noise, I misunderstood the function comment.
> 
> On a related note I think 'ctx' can also be a NAMESPACE_DECL here in
> the case of a defaulted non-member operator<=> (as in the below), for
> which I'd expect the added COMPLETE_TYPE_P check to crash, but it looks
> like in this case DECL_INITIAL is error_mark_node instead of NULL_TREE
> so a crash is averted.  If anyone else was wondering...
> 
>   struct A {
>     friend constexpr bool operator==(const A&, const A&);
>   };
> 
>   constexpr bool operator==(const A&, const A&) = default;

That means maybe ctx isn't the right way to get at the type and we
should look it up from the first argument's type?
I guess I'll look at where the build_comparison_op takes it from...

	Jakub

Re: [PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Tue, Sep 28, 2021 at 06:49:38PM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Tue, Sep 28, 2021 at 12:44:58PM -0400, Patrick Palka wrote:
> > Ah yeah, sorry for the noise, I misunderstood the function comment.
> > 
> > On a related note I think 'ctx' can also be a NAMESPACE_DECL here in
> > the case of a defaulted non-member operator<=> (as in the below), for
> > which I'd expect the added COMPLETE_TYPE_P check to crash, but it looks
> > like in this case DECL_INITIAL is error_mark_node instead of NULL_TREE
> > so a crash is averted.  If anyone else was wondering...
> > 
> >   struct A {
> >     friend constexpr bool operator==(const A&, const A&);
> >   };
> > 
> >   constexpr bool operator==(const A&, const A&) = default;
> 
> That means maybe ctx isn't the right way to get at the type and we
> should look it up from the first argument's type?
> I guess I'll look at where the build_comparison_op takes it from...

  tree lhs = DECL_ARGUMENTS (fndecl);
  if (is_this_parameter (lhs))
    lhs = cp_build_fold_indirect_ref (lhs);
  else
    lhs = convert_from_reference (lhs);
  tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
apparently.

	Jakub

Re: [PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Patrick Palka]

On Tue, 28 Sep 2021, Jakub Jelinek wrote:

> On Tue, Sep 28, 2021 at 06:49:38PM +0200, Jakub Jelinek via Gcc-patches wrote:
> > On Tue, Sep 28, 2021 at 12:44:58PM -0400, Patrick Palka wrote:
> > > Ah yeah, sorry for the noise, I misunderstood the function comment.
> > > 
> > > On a related note I think 'ctx' can also be a NAMESPACE_DECL here in
> > > the case of a defaulted non-member operator<=> (as in the below), for
> > > which I'd expect the added COMPLETE_TYPE_P check to crash, but it looks
> > > like in this case DECL_INITIAL is error_mark_node instead of NULL_TREE
> > > so a crash is averted.  If anyone else was wondering...
> > > 
> > >   struct A {
> > >     friend constexpr bool operator==(const A&, const A&);
> > >   };
> > > 
> > >   constexpr bool operator==(const A&, const A&) = default;
> > 
> > That means maybe ctx isn't the right way to get at the type and we
> > should look it up from the first argument's type?
> > I guess I'll look at where the build_comparison_op takes it from...

I suspect this synthesize_method call from defaulted_late_check is
really only needed when operator<=> has been defaulted inside the class
definition, because out-of-class defaulted definitions generally already
get eagerly synthesized IIUC.  So it might be fine to keep using ctx if
we also check DECL_DEFAULTED_IN_CLASS_P in defaulted_late_check.  But
Jason knows for sure..

> 
>   tree lhs = DECL_ARGUMENTS (fndecl);
>   if (is_this_parameter (lhs))
>     lhs = cp_build_fold_indirect_ref (lhs);
>   else
>     lhs = convert_from_reference (lhs);
>   tree ctype = TYPE_MAIN_VARIANT (TREE_TYPE (lhs));
> apparently.
> 
> 	Jakub
> 
> 

Re: [PATCH] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490]

[Jakub Jelinek]

On Tue, Sep 28, 2021 at 01:25:13PM -0400, Patrick Palka via Gcc-patches wrote:
> On Tue, 28 Sep 2021, Jakub Jelinek wrote:
> 
> > On Tue, Sep 28, 2021 at 06:49:38PM +0200, Jakub Jelinek via Gcc-patches wrote:
> > > On Tue, Sep 28, 2021 at 12:44:58PM -0400, Patrick Palka wrote:
> > > > Ah yeah, sorry for the noise, I misunderstood the function comment.
> > > > 
> > > > On a related note I think 'ctx' can also be a NAMESPACE_DECL here in
> > > > the case of a defaulted non-member operator<=> (as in the below), for
> > > > which I'd expect the added COMPLETE_TYPE_P check to crash, but it looks
> > > > like in this case DECL_INITIAL is error_mark_node instead of NULL_TREE
> > > > so a crash is averted.  If anyone else was wondering...
> > > > 
> > > >   struct A {
> > > >     friend constexpr bool operator==(const A&, const A&);
> > > >   };
> > > > 
> > > >   constexpr bool operator==(const A&, const A&) = default;
> > > 
> > > That means maybe ctx isn't the right way to get at the type and we
> > > should look it up from the first argument's type?
> > > I guess I'll look at where the build_comparison_op takes it from...
> 
> I suspect this synthesize_method call from defaulted_late_check is
> really only needed when operator<=> has been defaulted inside the class
> definition, because out-of-class defaulted definitions generally already
> get eagerly synthesized IIUC.  So it might be fine to keep using ctx if
> we also check DECL_DEFAULTED_IN_CLASS_P in defaulted_late_check.  But
> Jason knows for sure..

Indeed, cp_finish_decl has:
8333			  /* An out-of-class default definition is defined at
8334			     the point where it is explicitly defaulted.  */
8335			  if (DECL_DELETED_FN (decl))
8336			    maybe_explain_implicit_delete (decl);
8337			  else if (DECL_INITIAL (decl) == error_mark_node)
8338			    synthesize_method (decl);

	Jakub