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

[Jason Merrill <jason@redhat.com>]

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
>