From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id D6B073858C39 for ; Thu, 30 Sep 2021 19:01:54 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D6B073858C39 Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-498-GgoUGTyHPbm2qcQxpLoV4w-1; Thu, 30 Sep 2021 15:01:53 -0400 X-MC-Unique: GgoUGTyHPbm2qcQxpLoV4w-1 Received: by mail-qt1-f197.google.com with SMTP id c19-20020ac81e93000000b002a71180fd3dso7883175qtm.1 for ; Thu, 30 Sep 2021 12:01:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=3JiyMEsaj1bhR6NImL67rcpIbv4OBwNr1OSy6kSVTag=; b=e6tUNNM7HQEcJY3S2HJLgOeXQgWCBqZw18dqRgVz3DOeATz3/R57515A5lpgW5qz8E u/6Vp0KEn6QBqBuAhN4Y+R0+VcTM7UNWNznYgAFUnddqpDReI7kVKQVxja8z7e42lDpc 51RzC1ZPJVCtoHXs1e4QBS5+/ZZnLqWTTQIbFB4j5dY9wtpdyDPMVuN3QrVMOtr4Bk9v ow8WEnr0g/spYkNHnm7KlqdI+bcZWPjG9UNdCZBu5zAWaRnY5NM8Xkeb3XFCBof1lMsd 7RTglTtlWNlLHOb9WJ10Zzw6rX/qjrODxauAwLnRrJcN7C6fxFkO/DIS6OEOwe56Unm1 md5Q== X-Gm-Message-State: AOAM533FnUIXUzfs06DPPL5lolMpjrugFJW4nAJvikO+56H0zKtlwuTL vEn6FAIMPm4QheIi7QWnQhHLZ1piSIMsUKM+2gzYIwIAEFTvNDKav0PHEnfSOWaFZGKPO+M3l1t i90H7uHlkAYkyMkq5Ng== X-Received: by 2002:a0c:c2c4:: with SMTP id c4mr5665234qvi.18.1633028512152; Thu, 30 Sep 2021 12:01:52 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwvnLBB0sQhrCEqWViqs1eki8HDnnh5xCRzTEzemWFmgHk3h97yGg3Rj5GMyQDKtn+lnL0/UA== X-Received: by 2002:a0c:c2c4:: with SMTP id c4mr5665178qvi.18.1633028511623; Thu, 30 Sep 2021 12:01:51 -0700 (PDT) Received: from [192.168.1.149] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id l1sm2067305qti.94.2021.09.30.12.01.50 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 30 Sep 2021 12:01:50 -0700 (PDT) Message-ID: Date: Thu, 30 Sep 2021 15:01:49 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.2 Subject: Re: [PATCH, v3] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490] To: Jakub Jelinek Cc: Patrick Palka , gcc-patches@gcc.gnu.org References: <20210928092455.GU304296@tucnak> <742e6e4c-f921-e12-ccc0-edc6ca19f99@idea> <28ed21ef-fac2-8e93-44f4-f2b5ab287621@idea> <38a8c792-f599-5802-ffe3-a43eaee81479@redhat.com> <20210928203439.GG304296@tucnak> <504786a7-2453-a347-693f-8da18902bc06@redhat.com> <20210929192140.GU304296@tucnak> <20210930172424.GD304296@tucnak> From: Jason Merrill In-Reply-To: <20210930172424.GD304296@tucnak> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Sep 2021 19:01:57 -0000 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 > > 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 >