From patchwork Fri Oct 1 15:07:17 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 45688 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 183933857813 for ; Fri, 1 Oct 2021 15:07:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 183933857813 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1633100877; bh=neCN9AtwY8j34bozbjDy5By95bJsZMKWNWoEoMeKCpE=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=wA0Nlf8ewsdwWNmcGQIC1kbeFdYXekUhMJccICr20x9WNedY9sriV0M3cNhEF38AX ThZUTFY6bTlxOhpmi89dzFPOyqEpdRMEO+hOA0sTTN07DDXH01nw3qJIRylISP3V06 i3NUhkQCPEvZdP3Av0xgAnupVwM/te9ikWP34COw= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 2FAA13857804 for ; Fri, 1 Oct 2021 15:07:24 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2FAA13857804 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-325-GxQn-7KMMHuwGY54Gvrm1Q-1; Fri, 01 Oct 2021 11:07:23 -0400 X-MC-Unique: GxQn-7KMMHuwGY54Gvrm1Q-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C9A715074B for ; Fri, 1 Oct 2021 15:07:21 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.193.109]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0036E5F4E7; Fri, 1 Oct 2021 15:07:20 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 191F7Ivt3334674 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 1 Oct 2021 17:07:18 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 191F7HWe3334673; Fri, 1 Oct 2021 17:07:17 +0200 Date: Fri, 1 Oct 2021 17:07:17 +0200 To: Jason Merrill Subject: [PATCH, v4] c++: Fix up synthetization of defaulted comparison operators on classes with bitfields [PR102490] Message-ID: <20211001150717.GQ304296@tucnak> 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> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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-Patchwork-Original-From: Jakub Jelinek via Gcc-patches From: Jakub Jelinek Reply-To: Jakub Jelinek Cc: gcc-patches@gcc.gnu.org Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" 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 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. Jakub --- 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 + +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 +#include + +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 ); +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 ); --- 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 (); +}