From patchwork Thu Dec 2 23:12:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 48436 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 AD1EA385C412 for ; Thu, 2 Dec 2021 23:12:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AD1EA385C412 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1638486768; bh=tYJZPdmbiCOihyCzmhxibYEfYEUPSaXdt6dEGwZT/OI=; 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=GFl3V5MoJvGx9dJim5gytjzkUR21dZeUkb1uETb81g0nBjBkP5rOEvJJvTDoBsehc M3OstfX2ipiyzOwI2/TWexgWTTjGFz/1gUZ7v94yqRG9taBjV/CtjLxBDpz4FRHeLv Bch2/r2BihSdeD78q/bvbVmqEjhpd6n9iz9Z+X2M= 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 [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 50F6E3858428 for ; Thu, 2 Dec 2021 23:12:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 50F6E3858428 Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-456-iaL-ygYeMmCzKmK8xVRCIA-1; Thu, 02 Dec 2021 18:12:12 -0500 X-MC-Unique: iaL-ygYeMmCzKmK8xVRCIA-1 Received: by mail-qk1-f198.google.com with SMTP id x5-20020a05620a0b4500b004679442640aso1365439qkg.20 for ; Thu, 02 Dec 2021 15:12:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=tYJZPdmbiCOihyCzmhxibYEfYEUPSaXdt6dEGwZT/OI=; b=R1HNKfHJZYMnjYHA4BBm6BTXHwHn0r4qLU+8N/nR38oe0hlf7eezbu4BqHpD6O5Lot SGWWxFtOqukVO8m4vB4x3lt2/pPV0/efQhl+yDerNSU16XEHgn8fhpvaekbFjrIvP0aX ux1pOsXqUv20jzRkxHZIYtnkPPmdgp6rziRmctPI3AyP7NRBzsMAg9aD7pxGWcZbnqx8 aq9Jk2PgYpswvhQWjJ165WEC/NwaO8UQzbBs42INL9pbTDNEe/togT7dix0BEo4epHht gqLJfKN0IyDOpuVyU62se9m+YQV3ELMmHMoHZ/EQftShtxV4CO3DE1x+H5k+4SKm8gsc PyZw== X-Gm-Message-State: AOAM531XTVTOtj2QAqLySuD4Nhxf+8NNiS3ZaxUbg/spKvQGNwFvNSR9 uGjNJjSv85nA5LzUtPCsuN9DKX2ifHokY4XSP2X1gHX64t5sRZjTeOoq1JwMRs2rb9bQVVTncpk JI4PZdPezGHh0qR/xSw== X-Received: by 2002:a05:620a:4711:: with SMTP id bs17mr15152397qkb.321.1638486731816; Thu, 02 Dec 2021 15:12:11 -0800 (PST) X-Google-Smtp-Source: ABdhPJw+qV9T2qa0R5w/8F+wGowb2dQo4cvAts8s7K56Inp9751BH2DZddyEz3jJHZpb7xpGXSyYSQ== X-Received: by 2002:a05:620a:4711:: with SMTP id bs17mr15152378qkb.321.1638486731465; Thu, 02 Dec 2021 15:12:11 -0800 (PST) Received: from redhat.com ([2601:184:4780:4310::aac2]) by smtp.gmail.com with ESMTPSA id v12sm1019755qtx.80.2021.12.02.15.12.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Dec 2021 15:12:10 -0800 (PST) Date: Thu, 2 Dec 2021 18:12:09 -0500 To: Jason Merrill Subject: [PATCH v2] c++: Fix for decltype(auto) and parenthesized expr [PR103403] Message-ID: References: <20211201151659.1497996-1-polacek@redhat.com> <27d052e3-1c4a-84b6-fb1c-a1e4066f69df@redhat.com> MIME-Version: 1.0 In-Reply-To: <27d052e3-1c4a-84b6-fb1c-a1e4066f69df@redhat.com> User-Agent: Mutt/2.1.3 (2021-09-10) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-12.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_EU, 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: Marek Polacek via Gcc-patches From: Marek Polacek Reply-To: Marek Polacek Cc: GCC Patches Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" On Wed, Dec 01, 2021 at 11:16:27PM -0500, Jason Merrill wrote: > On 12/1/21 10:16, Marek Polacek wrote: > > In r11-4758, I tried to fix this problem: > > > > int &&i = 0; > > decltype(auto) j = i; // should behave like int &&j = i; error > > > > wherein do_auto_deduction was getting confused with a REFERENCE_REF_P > > and it didn't realize its operand was a name, not an expression, and > > deduced the wrong type. > > > > Unfortunately that fix broke this: > > > > int&& r = 1; > > decltype(auto) rr = (r); > > > > where 'rr' should be 'int &' since '(r)' is an expression, not a name. But > > because I stripped the INDIRECT_REF with the r11-4758 change, we deduced > > 'rr's type as if decltype had gotten a name, resulting in 'int &&'. > > > > I suspect I thought that the REF_PARENTHESIZED_P check when setting > > 'bool id' in do_auto_deduction would handle the (r) case, but that's not > > the case; while the documentation for REF_PARENTHESIZED_P specifically says > > it can be set in INDIRECT_REF, we don't actually do so. > > > > This patch sets REF_PARENTHESIZED_P even on REFERENCE_REF_P, so that > > do_auto_deduction can use it. > > > > It also removes code in maybe_undo_parenthesized_ref that I think is > > dead -- and we don't hit it while running dg.exp. To adduce more data, > > it also looks dead here: > > https://splichal.eu/lcov/gcc/cp/semantics.c.gcov.html > > Agreed, that code is dead since r9-1417. > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and 11? > > > > PR c++/103403 > > > > gcc/cp/ChangeLog: > > > > * cp-gimplify.c (cp_fold): Don't recurse if maybe_undo_parenthesized_ref > > doesn't change its argument. > > * pt.c (do_auto_deduction): Don't strip REFERENCE_REF_P trees. Also > > REF_PARENTHESIZED_P for REFERENCE_REF_P. > > * semantics.c (force_paren_expr): Set REF_PARENTHESIZED_P on > > REFERENCE_REF_P trees too. > > (maybe_undo_parenthesized_ref): Remove dead code. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp1y/decltype-auto2.C: New test. > > * g++.dg/cpp1y/decltype-auto3.C: New test. > > --- > > gcc/cp/cp-gimplify.c | 3 ++- > > gcc/cp/pt.c | 5 ++--- > > gcc/cp/semantics.c | 18 ++++-------------- > > gcc/testsuite/g++.dg/cpp1y/decltype-auto2.C | 12 ++++++++++++ > > gcc/testsuite/g++.dg/cpp1y/decltype-auto3.C | 12 ++++++++++++ > > 5 files changed, 32 insertions(+), 18 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/decltype-auto2.C > > create mode 100644 gcc/testsuite/g++.dg/cpp1y/decltype-auto3.C > > > > diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c > > index 0a002db14e7..e3ede02a48e 100644 > > --- a/gcc/cp/cp-gimplify.c > > +++ b/gcc/cp/cp-gimplify.c > > @@ -2421,7 +2421,8 @@ cp_fold (tree x) > > if (REF_PARENTHESIZED_P (x)) > > { > > tree p = maybe_undo_parenthesized_ref (x); > > - return cp_fold (p); > > + if (p != x) > > + return cp_fold (p); > > } > > goto unary; > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > index f4b9d9673fb..c5b41b57028 100644 > > --- a/gcc/cp/pt.c > > +++ b/gcc/cp/pt.c > > @@ -29889,11 +29889,10 @@ do_auto_deduction (tree type, tree init, tree auto_node, > > else if (AUTO_IS_DECLTYPE (auto_node)) > > { > > tree stripped_init = tree_strip_any_location_wrapper (init); > > - if (REFERENCE_REF_P (stripped_init)) > > - stripped_init = TREE_OPERAND (stripped_init, 0); > > bool id = (DECL_P (stripped_init) > > || ((TREE_CODE (init) == COMPONENT_REF > > - || TREE_CODE (init) == SCOPE_REF) > > + || TREE_CODE (init) == SCOPE_REF > > + || REFERENCE_REF_P (init)) > > && !REF_PARENTHESIZED_P (init))); > > This change seems wrong; not all references are id-expressions or member > accesses. For instance, a call to a function returning a reference is not. > I think we still want to depend on the TREE_CODE, but we need to look at the > TREE_CODE of stripped_init, not init. Yes indeed, id was wrong. I didn't notice because we still deduced to the right type! This version only strips REFERENCE_REF_P when they aren't REF_PARENTHESIZED_P, so that, as the comment says, I can tell '(r)' and 'r' apart. Since we can deduce the correct type even with the wrong id, I tested this by using the two functions in decltype-auto4.C, and adding gcc_assert(id) or !id so that I can verify that id looks correct with this patch. Also add a new test case: decltype-auto4.C. It would still be nice to test more; I'm not sure how well we test [dcl.type.decltype]/1.1 and /1.2. I'm also adding decomp-decltype1.C, which fixes decltype + structured binding. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk / 11? -- >8 -- In r11-4758, I tried to fix this problem: int &&i = 0; decltype(auto) j = i; // should behave like int &&j = i; error wherein do_auto_deduction was getting confused with a REFERENCE_REF_P and it didn't realize its operand was a name, not an expression, and deduced the wrong type. Unfortunately that fix broke this: int&& r = 1; decltype(auto) rr = (r); where 'rr' should be 'int &' since '(r)' is an expression, not a name. But because I stripped the INDIRECT_REF with the r11-4758 change, we deduced 'rr's type as if decltype had gotten a name, resulting in 'int &&'. I suspect I thought that the REF_PARENTHESIZED_P check when setting 'bool id' in do_auto_deduction would handle the (r) case, but that's not the case; while the documentation for REF_PARENTHESIZED_P specifically says it can be set in INDIRECT_REF, we don't actually do so. This patch sets REF_PARENTHESIZED_P even on REFERENCE_REF_P, so that do_auto_deduction can use it. It also removes code in maybe_undo_parenthesized_ref that I think is dead -- and we don't hit it while running dg.exp. To adduce more data, it also looks dead here: https://splichal.eu/lcov/gcc/cp/semantics.c.gcov.html (It's dead since r9-1417.) Also add a fixed test for c++/81176. PR c++/103403 gcc/cp/ChangeLog: * cp-gimplify.c (cp_fold): Don't recurse if maybe_undo_parenthesized_ref doesn't change its argument. * pt.c (do_auto_deduction): Don't strip REFERENCE_REF_P trees if they are REF_PARENTHESIZED_P. Use stripped_init when checking for id-expression. * semantics.c (force_paren_expr): Set REF_PARENTHESIZED_P on REFERENCE_REF_P trees too. (maybe_undo_parenthesized_ref): Remove dead code. gcc/testsuite/ChangeLog: * g++.dg/cpp1y/decltype-auto2.C: New test. * g++.dg/cpp1y/decltype-auto3.C: New test. * g++.dg/cpp1y/decltype-auto4.C: New test. * g++.dg/cpp1z/decomp-decltype1.C: New test. --- gcc/cp/cp-gimplify.c | 3 +- gcc/cp/pt.c | 15 +++-- gcc/cp/semantics.c | 18 ++--- gcc/testsuite/g++.dg/cpp1y/decltype-auto2.C | 12 ++++ gcc/testsuite/g++.dg/cpp1y/decltype-auto3.C | 12 ++++ gcc/testsuite/g++.dg/cpp1y/decltype-auto4.C | 65 +++++++++++++++++++ gcc/testsuite/g++.dg/cpp1z/decomp-decltype1.C | 28 ++++++++ 7 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp1y/decltype-auto2.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/decltype-auto3.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/decltype-auto4.C create mode 100644 gcc/testsuite/g++.dg/cpp1z/decomp-decltype1.C base-commit: c841dc0a4dd944cf8f76f414a286aedff1e38dce diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c index 0a002db14e7..e3ede02a48e 100644 --- a/gcc/cp/cp-gimplify.c +++ b/gcc/cp/cp-gimplify.c @@ -2421,7 +2421,8 @@ cp_fold (tree x) if (REF_PARENTHESIZED_P (x)) { tree p = maybe_undo_parenthesized_ref (x); - return cp_fold (p); + if (p != x) + return cp_fold (p); } goto unary; diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 6b560952639..d3efc6ea238 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -29889,13 +29889,18 @@ do_auto_deduction (tree type, tree init, tree auto_node, } else if (AUTO_IS_DECLTYPE (auto_node)) { + /* Figure out if INIT is an unparenthesized id-expression or an + unparenthesized class member access. */ tree stripped_init = tree_strip_any_location_wrapper (init); - if (REFERENCE_REF_P (stripped_init)) + /* We need to be able to tell '(r)' and 'r' apart (when it's of + reference type). Only the latter is an id-expression. */ + if (REFERENCE_REF_P (stripped_init) + && !REF_PARENTHESIZED_P (stripped_init)) stripped_init = TREE_OPERAND (stripped_init, 0); - bool id = (DECL_P (stripped_init) - || ((TREE_CODE (init) == COMPONENT_REF - || TREE_CODE (init) == SCOPE_REF) - && !REF_PARENTHESIZED_P (init))); + const bool id = (DECL_P (stripped_init) + || ((TREE_CODE (stripped_init) == COMPONENT_REF + || TREE_CODE (stripped_init) == SCOPE_REF) + && !REF_PARENTHESIZED_P (stripped_init))); tree deduced = finish_decltype_type (init, id, complain); deduced = canonicalize_type_argument (deduced, complain); if (deduced == error_mark_node) diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index cd1956497f8..edba4b60e10 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -2049,7 +2049,8 @@ force_paren_expr (tree expr, bool even_uneval) return expr; if (TREE_CODE (expr) == COMPONENT_REF - || TREE_CODE (expr) == SCOPE_REF) + || TREE_CODE (expr) == SCOPE_REF + || REFERENCE_REF_P (expr)) REF_PARENTHESIZED_P (expr) = true; else if (DECL_P (tree_strip_any_location_wrapper (expr))) { @@ -2072,19 +2073,8 @@ maybe_undo_parenthesized_ref (tree t) if (cxx_dialect < cxx14) return t; - if (INDIRECT_REF_P (t) && REF_PARENTHESIZED_P (t)) - { - t = TREE_OPERAND (t, 0); - while (TREE_CODE (t) == NON_LVALUE_EXPR - || TREE_CODE (t) == NOP_EXPR) - t = TREE_OPERAND (t, 0); - - gcc_assert (TREE_CODE (t) == ADDR_EXPR - || TREE_CODE (t) == STATIC_CAST_EXPR); - t = TREE_OPERAND (t, 0); - } - else if ((TREE_CODE (t) == PAREN_EXPR || TREE_CODE (t) == VIEW_CONVERT_EXPR) - && REF_PARENTHESIZED_P (t)) + if ((TREE_CODE (t) == PAREN_EXPR || TREE_CODE (t) == VIEW_CONVERT_EXPR) + && REF_PARENTHESIZED_P (t)) t = TREE_OPERAND (t, 0); return t; diff --git a/gcc/testsuite/g++.dg/cpp1y/decltype-auto2.C b/gcc/testsuite/g++.dg/cpp1y/decltype-auto2.C new file mode 100644 index 00000000000..56e011e36f4 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/decltype-auto2.C @@ -0,0 +1,12 @@ +// PR c++/103403 +// { dg-do compile { target c++14 } } + +template +auto constexpr RtoL1(T&& r) -> decltype(auto) { + return (r); +}; +int main() { + int t; + int x{3}; + decltype (RtoL1(x+0)) y = t; +} diff --git a/gcc/testsuite/g++.dg/cpp1y/decltype-auto3.C b/gcc/testsuite/g++.dg/cpp1y/decltype-auto3.C new file mode 100644 index 00000000000..914e87f5b79 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/decltype-auto3.C @@ -0,0 +1,12 @@ +// PR c++/103403 +// { dg-do compile { target c++14 } } + +int main() +{ + int i = 1; + int&& r = 1; + + decltype(auto) ri = (i); + decltype(auto) rr = (r); + decltype((r)) rr2 = (r); +} diff --git a/gcc/testsuite/g++.dg/cpp1y/decltype-auto4.C b/gcc/testsuite/g++.dg/cpp1y/decltype-auto4.C new file mode 100644 index 00000000000..9765857efb1 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/decltype-auto4.C @@ -0,0 +1,65 @@ +// PR c++/103403 +// { dg-do compile { target c++14 } } + +struct false_type { static constexpr bool value = false; }; +struct true_type { static constexpr bool value = true; }; +template +struct is_same : false_type {}; +template +struct is_same : true_type {}; + +int fn (); +int &ref (); +int &&rref (); + +struct S { + int i; + int &r = i; +}; + +void +ids () +{ + const S *s = new S(); + int i; + int &ir = i; + decltype(auto) r1 = s->i; + static_assert (is_same::value, ""); + decltype(auto) r2 = s->r; + static_assert (is_same::value, ""); + decltype(auto) r3 = i; + static_assert (is_same::value, ""); + decltype(auto) r4 = ir; + static_assert (is_same::value, ""); +} + +void +nonids () +{ + const S *s = new S(); + int i; + int &ir = i; + int &&irr = 42; + decltype(auto) r1 = fn (); + static_assert (is_same::value, ""); + decltype(auto) r2 = (fn ()); + static_assert (is_same::value, ""); + decltype(auto) r3 = ref (); + static_assert (is_same::value, ""); + decltype(auto) r4 = (ref ()); + static_assert (is_same::value, ""); + decltype(auto) r5 = rref (); + static_assert (is_same::value, ""); + decltype(auto) r6 = (rref ()); + static_assert (is_same::value, ""); + decltype(auto) r8 = (s->i); + static_assert (is_same::value, ""); + decltype(auto) r9 = (s->r); + static_assert (is_same::value, ""); + decltype(auto) r10 = (i); + static_assert (is_same::value, ""); + decltype(auto) r11 = (ir); + static_assert (is_same::value, ""); + decltype(auto) r12 = (irr); + static_assert (is_same::value, ""); +} diff --git a/gcc/testsuite/g++.dg/cpp1z/decomp-decltype1.C b/gcc/testsuite/g++.dg/cpp1z/decomp-decltype1.C new file mode 100644 index 00000000000..e8ca0fc0bee --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1z/decomp-decltype1.C @@ -0,0 +1,28 @@ +// PR c++/81176 +// { dg-do compile { target c++17 } } + +namespace std { + template struct tuple_size; + template struct tuple_element; +} + +struct A { + int i; + template int& get() { return i; } +}; + +template<> struct std::tuple_size { static const int value = 2; }; +template struct std::tuple_element { using type = int; }; + +template struct same_type; +template struct same_type {}; + +void +foo (A x) +{ + auto [ a, b ] = x; + decltype(auto) c = a; + same_type{}; + same_type{}; +} +