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 <ja...@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
> >
> >
>
