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 ESMTPS id 4E9EC3858D1E for ; Wed, 22 May 2024 01:55:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4E9EC3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 4E9EC3858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716342940; cv=none; b=qJ94KHBsSQIGBm7IoNGpa/0nFfSQU3v7WkwRerSKrEWsmuk/EIB3Xwq33JzCMPQKkQSfzxcEovmUkfp5XWNF3u9brlbowGSL/k3ceUatkrervgeHSkZAMK86DkSl/P3pvTlV3VmOPJjmW3KU8poIQsT1bzbKr5ywszw4Yj6ekG0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1716342940; c=relaxed/simple; bh=UTSW7TJo8uv/kvROmbsnfF7BdMQCcC0kpA9N/7N5IN4=; h=DKIM-Signature:From:Date:To:Subject:Message-ID:MIME-Version; b=nium+Ldk65rpaJwtiViFiMXZEd+lyX7LD5m/wfBoQcd39PqNWfCorCoVV2TGTQxfr8DlDtaHcuFTSblG/ghbjjkk2VrtBFvyEm5hsUMEuk+a0eiv/3ikPzY7LL0Z9jZzkon+PoKMDBLtF92sNCgHUFan/N7A9crFRPKAiV5YKgc= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1716342938; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=gQWsmTRiJNY4jTV8G44POFJDW1nrAodASOXYgGN5UQM=; b=HaMaaPyyiwuIV6QDykLPy4SMg1vkDMovwVFi3a0zhvNM9LYz9bUn4Sb9MQOvzKXin6kW1F FrvW7J/DQ/YO7HOtouV4pyMzCjI7ii1KBPgb2AdW39wqEHhuahVUDAWDQ9Ukp2aKQ05SWR v6ypIAKG7aOzOo7rVi0hp07+0P9XM+U= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-46-DpHPFDyROMe_MQi7twWFaQ-1; Tue, 21 May 2024 21:55:36 -0400 X-MC-Unique: DpHPFDyROMe_MQi7twWFaQ-1 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-43df9a3fc0fso156537361cf.0 for ; Tue, 21 May 2024 18:55:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716342936; x=1716947736; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=gQWsmTRiJNY4jTV8G44POFJDW1nrAodASOXYgGN5UQM=; b=L5nGaU28J11XXCcS+WX28SwbQHH66krFyDVR8GahN9mogItNo60JZJrAiYT6g9ghxy 45l2Y3BaLu0dQVYkM/Au/P89P6j0+zLquDj4piwh4LaW6mvU0sbzXwqgzpkua1FHwMMy 9MjDm+MQ5BbLGL/+4JfJxJS8r1bfTZI44SB24ZBQ6RJ7Hd6Ly9zu/KNtlmQ3R2t1plpi Jw71Qi0FeYtvoJSc0FbapKCvHpZ/ocLyiBEGfquh40v0xmxlKMfn4gvnTB7w2qh3xFyW lMINEaun5y1mEDaf331rhoe972U/zlyKM/xV97/37C3XzZSqreGXBvVgHq3MiHqu55Zj k2Rg== X-Forwarded-Encrypted: i=1; AJvYcCWe06+5dm/s/6U89HiigSuCq5YwWHJh3r4FYQdMaCw836Mklinf6Clf6KHPrqt8SqpkllQhhkBHozfXEr91+TEbQLorLLGXDQ== X-Gm-Message-State: AOJu0YxwEm0j6XSnXkK6mAZqG9egZW7UNGN6reYr2radkgq2U9Is95Bo xGqJM9XRPawnXOR3LqOeNKK0RAWI1hI3noWTVi2/6lL3FNcUfU/GhZRqbY3JdyaAbL1yMCdwW81 LvbptyalZYfYtXMPAN4SvbaPPu+1BZaE4y4czhQRn6fJ/cYNduZr2lQ8= X-Received: by 2002:a05:622a:1a1e:b0:43a:ed8b:75b1 with SMTP id d75a77b69052e-43f9e0f3220mr8450171cf.43.1716342935527; Tue, 21 May 2024 18:55:35 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFfwWbjkgK9nR0B3OGnBrVSMF0dOInb4YTMtdjSv7qI75ekuklab5pIjWSEHVvPzsxFo3Wyug== X-Received: by 2002:a05:622a:1a1e:b0:43a:ed8b:75b1 with SMTP id d75a77b69052e-43f9e0f3220mr8449961cf.43.1716342934790; Tue, 21 May 2024 18:55:34 -0700 (PDT) Received: from [192.168.1.130] (ool-18bb2a2e.dyn.optonline.net. [24.187.42.46]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-43e16c5485fsm126901271cf.2.2024.05.21.18.55.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 May 2024 18:55:34 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Tue, 21 May 2024 21:55:33 -0400 (EDT) To: Jason Merrill cc: Patrick Palka , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159] In-Reply-To: Message-ID: <9c0b3ede-9457-aa5c-0934-b82f7a44291f@idea> References: <20240521193629.4129787-1-ppalka@redhat.com> <42726cf3-3bf8-499a-b455-d0181f0b8d3b@redhat.com> <3356d9ff-9c60-a215-427c-9deaf6fb5024@idea> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-13.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Tue, 21 May 2024, Jason Merrill wrote: > On 5/21/24 17:27, Patrick Palka wrote: > > On Tue, 21 May 2024, Jason Merrill wrote: > > > > > On 5/21/24 15:36, Patrick Palka wrote: > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look > > > > OK for trunk? > > > > > > > > Alternatively, I considered fixing this by incrementing > > > > comparing_specializations around the call to comp_except_specs in > > > > cp_check_qualified_type, but generally for types whose identity > > > > depends on whether comparing_specializations is set we need to > > > > use structural equality anyway IIUC. > > > > > > Why not both? > > > > I figured the latter change isn't necessary/observable since > > comparing_specializations would only make a difference for complex > > exception specifications, and with this patch we won't even call > > cp_check_qualified_type on a complex eh spec. > > My concern is that if we're building a function type multiple times with the > same noexcept-spec, this patch would mean creating multiple equivalent > function types instead of reusing one already created for the same function. > > > > > + bool complex_p = (cr && cr != noexcept_true_spec > > > > + && !UNPARSED_NOEXCEPT_SPEC_P (cr)); > > > > > > Why treat unparsed specs differently from parsed ones? > > > > Unparsed specs are unique according to cp_tree_equal, so in turn > > function types with unparsed specs are unique, so it should be safe to > > treat such types as canonical. I'm not sure if this optimization > > matters though; I'm happy to remove this case. > > The idea that this optimization could make a difference raised the concern > above. Aha, makes sense. To that end it seems we could strengthen the ce_exact in comp_except_specs to require == instead of cp_tree_equal equality when comparing two noexcept-specs; the only ce_exact callers are cp_check_qualified_type and cxx_type_hash_eq, which should be fine with that strengthening. This way, we at least do try to reuse a variant if the (complex or unparsed) noexcept-spec is exactly the same. Like so? -- >8 -- Subject: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159] Here the member functions QList::g and QList::h are given the same function type since their exception specifications are equivalent according to cp_tree_equal. In doing so however this means that the type of QList::h refers to a function parameter from QList::g, which ends up confusing modules streaming. I'm not sure if modules can be fixed to handle this situation, but regardless it seems weird in principle that a function parameter can escape in such a way. The analogous situation with a trailing return type and decltype auto g(QList &other) -> decltype(f(other)); auto h(QList &other) -> decltype(f(other)); behaves better because we don't canonicalize decltype, and so the function types of g and h are non-canonical and therefore not shared. In light of this, it seems natural to treat function types with complex eh specs as non-canonical as well so that each such function declaration is given a unique function/method type node. The main benefit of type canonicalization is to speed up repeated type comparisons, but it should rare for us to repeatedly compare two otherwise compatible function types with complex exception specifications, so foregoing canonicalization should not cause any problems. To that end, this patch strengthens the ce_exact case of comp_except_specs to require identity instead of equivalence of the exception specification so that build_cp_fntype_variant doesn't reuse a variant when it shouldn't. And in build_cp_fntype_variant we need to use structural equality for types with a complex eh spec. In turn we could simplify the code responsible for adjusting unparsed eh spec variants. PR c++/115159 gcc/cp/ChangeLog: * tree.cc (build_cp_fntype_variant): Always use structural equality for types with a complex exception specification. (fixup_deferred_exception_variants): Always use structural equality for adjusted variants. * typeck.cc (comp_except_specs): Require == instead of cp_tree_equal for noexcept-spec comparison in the ce_exact case. gcc/testsuite/ChangeLog: * g++.dg/modules/noexcept-2_a.H: New test. * g++.dg/modules/noexcept-2_b.C: New test. --- gcc/cp/tree.cc | 47 +++++---------------- gcc/cp/typeck.cc | 4 +- gcc/testsuite/g++.dg/modules/noexcept-2_a.H | 24 +++++++++++ gcc/testsuite/g++.dg/modules/noexcept-2_b.C | 4 ++ 4 files changed, 41 insertions(+), 38 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_a.H create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_b.C diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc index 9d37d255d8d..93a64322418 100644 --- a/gcc/cp/tree.cc +++ b/gcc/cp/tree.cc @@ -2794,7 +2794,12 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual, /* Canonicalize the exception specification. */ tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE; - if (TYPE_STRUCTURAL_EQUALITY_P (type)) + /* Always use structural equality for function types with a complex + exception specification since their identity may depend on e.g. + whether comparing_specializations is set. */ + bool complex_eh_spec_p = (cr && cr != noexcept_true_spec + && !UNPARSED_NOEXCEPT_SPEC_P (cr)); + if (TYPE_STRUCTURAL_EQUALITY_P (type) || complex_eh_spec_p) /* Propagate structural equality. */ SET_TYPE_STRUCTURAL_EQUALITY (v); else if (TYPE_CANONICAL (type) != type || cr != raises || late) @@ -2812,55 +2817,23 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual, /* TYPE is a function or method type with a deferred exception specification that has been parsed to RAISES. Fixup all the type variants that are affected in place. Via decltype &| noexcept - tricks, the unparsed spec could have escaped into the type system. - The general case is hard to fixup canonical types for. */ + tricks, the unparsed spec could have escaped into the type system. */ void fixup_deferred_exception_variants (tree type, tree raises) { tree original = TYPE_RAISES_EXCEPTIONS (type); - tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE; gcc_checking_assert (UNPARSED_NOEXCEPT_SPEC_P (original)); - /* Though sucky, this walk will process the canonical variants - first. */ - tree prev = NULL_TREE; for (tree variant = TYPE_MAIN_VARIANT (type); - variant; prev = variant, variant = TYPE_NEXT_VARIANT (variant)) + variant; variant = TYPE_NEXT_VARIANT (variant)) if (TYPE_RAISES_EXCEPTIONS (variant) == original) { gcc_checking_assert (variant != TYPE_MAIN_VARIANT (type)); - if (!TYPE_STRUCTURAL_EQUALITY_P (variant)) - { - cp_cv_quals var_quals = TYPE_QUALS (variant); - cp_ref_qualifier rqual = type_memfn_rqual (variant); - - /* If VARIANT would become a dup (cp_check_qualified_type-wise) - of an existing variant in the variant list of TYPE after its - exception specification has been parsed, elide it. Otherwise, - build_cp_fntype_variant could use it, leading to "canonical - types differ for identical types." */ - tree v = TYPE_MAIN_VARIANT (type); - for (; v; v = TYPE_NEXT_VARIANT (v)) - if (cp_check_qualified_type (v, variant, var_quals, - rqual, cr, false)) - { - /* The main variant will not match V, so PREV will never - be null. */ - TYPE_NEXT_VARIANT (prev) = TYPE_NEXT_VARIANT (variant); - break; - } - TYPE_RAISES_EXCEPTIONS (variant) = raises; - - if (!v) - v = build_cp_fntype_variant (TYPE_CANONICAL (variant), - rqual, cr, false); - TYPE_CANONICAL (variant) = TYPE_CANONICAL (v); - } - else - TYPE_RAISES_EXCEPTIONS (variant) = raises; + SET_TYPE_STRUCTURAL_EQUALITY (variant); + TYPE_RAISES_EXCEPTIONS (variant) = raises; if (!TYPE_DEPENDENT_P (variant)) /* We no longer know that it's not type-dependent. */ diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 75b696e32e0..d535746fd43 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -1227,7 +1227,9 @@ comp_except_specs (const_tree t1, const_tree t2, int exact) if ((t1 && TREE_PURPOSE (t1)) || (t2 && TREE_PURPOSE (t2))) return (t1 && t2 - && cp_tree_equal (TREE_PURPOSE (t1), TREE_PURPOSE (t2))); + && (exact == ce_exact + ? TREE_PURPOSE (t1) == TREE_PURPOSE (t2) + : cp_tree_equal (TREE_PURPOSE (t1), TREE_PURPOSE (t2)))); if (t1 == NULL_TREE) /* T1 is ... */ return t2 == NULL_TREE || exact == ce_derived; diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_a.H b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H new file mode 100644 index 00000000000..b7144f42d7e --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H @@ -0,0 +1,24 @@ +// PR c++/115159 +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +struct QDebug; + +template void f(T); + +template struct QList { + QDebug g(QList &other) noexcept(noexcept(f(other))); + QDebug h(QList &other) noexcept(noexcept(f(other))); +}; + +struct QObjectData { + QList children; +}; + +struct QIODevice { + QObjectData d_ptr; +}; + +struct QDebug { + QDebug(QIODevice); +}; diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_b.C b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C new file mode 100644 index 00000000000..d34c63add10 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C @@ -0,0 +1,4 @@ +// PR c++/115159 +// { dg-additional-options "-fmodules-ts -fno-module-lazy" } + +import "noexcept-2_a.H"; -- 2.45.1.216.g4365c6fcf9