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

  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.


	Jakub
  

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
> 
>
  

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
> > 
> > 
>
  

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
  

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
> 
>
  

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
  

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
  

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
> 
>
  

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
  

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
>>>
>>>
>>
>