From patchwork Sat Jun 25 00:30:22 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 1648217 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=kII+02m9; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4LVFJx36SLz9sG2 for ; Sat, 25 Jun 2022 10:30:55 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 30725383D810 for ; Sat, 25 Jun 2022 00:30:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 30725383D810 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1656117051; bh=4HH61a1FPCPhglV1geJz17IY36f0Pj8ouDyc00AFPGk=; 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=kII+02m9FlK2gSrLLyo2A39ap2YaFMVO8R0xXAB6GoEulirCpJqFPC1V2FIqAPUA+ CgX2TPrSCzcJm5vq2qw0HAMQFIsCtuJ8/1vHBRDQNG8b3tP09yHW1ZutVWHTXKQSNQ eSzva8r5BV/PwGnZMCQc4ErrJwaxPaVZA+902LNQ= 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 161D93851C25 for ; Sat, 25 Jun 2022 00:30:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 161D93851C25 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-290-3A2rAlGzMNGQIHLqtmf5Kg-1; Fri, 24 Jun 2022 20:30:26 -0400 X-MC-Unique: 3A2rAlGzMNGQIHLqtmf5Kg-1 Received: by mail-qk1-f200.google.com with SMTP id az18-20020a05620a171200b006a708307e94so4313395qkb.14 for ; Fri, 24 Jun 2022 17:30:26 -0700 (PDT) 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=4HH61a1FPCPhglV1geJz17IY36f0Pj8ouDyc00AFPGk=; b=mBj6JyNUPi/bM9VVJ4YRRsseacCI6oWaoHH7OzQ4V/HEYGr09Hmfjx4ZCQjtigqbVE 2KRrJxI1hVq+aLZYjay6Y79dJLekZoCQwWXmgjhuzEL8FAaiK8DOhQe8PtvPG+0OAmnM SkjYTXh48qpDZ0/+H3AsJpwPYyf+HL11WdzTNoozNs51b0ADk91W4m6h1eAtPUk75VEF iKYYPgtNOpVBxytYFLmsGrx9ox+bfPHq4RDMLG9ht6MC/CE5v81WkvsSJfygJCQYM1jN bnw59sWzjoJOfHtXUgzz3FgRV9gggXC/l3ykrv5S7+ZgSdRHSSW7j61W7vLHocBsI1VZ InWQ== X-Gm-Message-State: AJIora+gQ6FPFAfdqGFuw1RUydeALLMh3JxYbid2vlxDR/JQq9soAJ3T kjXp/0yBuc51TgYkDpKVTyLQpz7E/xNOjeBYJDS+iNzPchomQOVFy6na/erobJwBWXZaJ3S6H23 PBcZccQMJpYiFABmGog== X-Received: by 2002:a0c:a9d6:0:b0:470:9d52:63aa with SMTP id c22-20020a0ca9d6000000b004709d5263aamr1327505qvb.20.1656117025355; Fri, 24 Jun 2022 17:30:25 -0700 (PDT) X-Google-Smtp-Source: AGRyM1tlk20MsEvjDgz5BeQzUaWvi74ydTkueijX14e2h6HOTo6mGQAI7z7huJiGfzyUDDZQBwJvrA== X-Received: by 2002:a0c:a9d6:0:b0:470:9d52:63aa with SMTP id c22-20020a0ca9d6000000b004709d5263aamr1327489qvb.20.1656117024931; Fri, 24 Jun 2022 17:30:24 -0700 (PDT) Received: from redhat.com ([2601:184:4780:4310::fd37]) by smtp.gmail.com with ESMTPSA id a5-20020ac85b85000000b00307cb34aa8asm2642914qta.47.2022.06.24.17.30.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Jun 2022 17:30:24 -0700 (PDT) Date: Fri, 24 Jun 2022 20:30:22 -0400 To: Jason Merrill Subject: [PATCH v2] c++: fix broken copy elision with nested TARGET_EXPRs [PR105550] Message-ID: References: <20220526150145.23850-1-polacek@redhat.com> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/2.2.5 (2022-05-16) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-13.4 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, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" On Thu, Jun 02, 2022 at 05:08:54PM -0400, Jason Merrill wrote: > On 5/26/22 11:01, Marek Polacek wrote: > > In this problem, we are failing to properly perform copy elision with > > a conditional operator, so this: > > > > constexpr A a = true ? A{} : A{}; > > > > fails with: > > > > error: 'A{((const A*)(&))}' is not a constant expression > > > > The whole initializer is > > > > TARGET_EXPR }> : TARGET_EXPR }>> > > > > where the outermost TARGET_EXPR is elided, but not the nested ones. > > Then we end up replacing the PLACEHOLDER_EXPRs with the temporaries the > > TARGET_EXPRs represent, which is precisely what should *not* happen with > > copy elision. > > > > I've tried the approach of tweaking ctx->object, but I ran into gazillion > > problems with that. I thought that I would let cxx_eval_constant_expression > > /TARGET_EXPR create a new object only when ctx->object was null, then > > adjust setting of ctx->object in places like cxx_bind_parameters_in_call > > and cxx_eval_component_reference but that failed completely. Sometimes > > ctx->object has to be reset, sometimes it cannot be reset, 'this' needed > > special handling, etc. I gave up. > > > But now that we have potential_prvalue_result_of, a simple but less > > elegant solution is the following. I thought about setting a flag on > > a TARGET_EXPR to avoid adding ctx.full_expr, but a new flag would be > > overkill and using TARGET_EXPR_DIRECT_INIT_P broke things. Sorry it's taken me so long to get back to this. > This doesn't seem like a general solution; the same issue would also apply > to ?: of TARGET_EXPR when it's a subexpression rather than the full > expression, like f(true ? A{} : B{}). You're right. > Another simple approach, but more general, would be to routinely strip > TARGET_EXPR from the operands of ?: like we do in various other places in > constexpr.c. How about this, then? Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- In this problem, we are failing to properly perform copy elision with a conditional operator, so this: constexpr A a = true ? A{} : A{}; fails with: error: 'A{((const A*)(&))}' is not a constant expression The whole initializer is TARGET_EXPR }> : TARGET_EXPR }>> where the outermost TARGET_EXPR is elided, but not the nested ones. Then we end up replacing the PLACEHOLDER_EXPRs with the temporaries the TARGET_EXPRs represent, which is precisely what should *not* happen with copy elision. I've tried the approach of tweaking ctx->object, but I ran into gazillion problems with that. I thought that I would let cxx_eval_constant_expression /TARGET_EXPR create a new object only when ctx->object was null, then adjust setting of ctx->object in places like cxx_bind_parameters_in_call and cxx_eval_component_reference but that failed completely. Sometimes ctx->object has to be reset, sometimes it cannot be reset, 'this' needed special handling, etc. I gave up. Instead, this patch strips TARGET_EXPRs from the operands of ?: like we do in various other places in constexpr.c. PR c++/105550 gcc/cp/ChangeLog: * constexpr.cc (cxx_eval_conditional_expression): Strip TARGET_EXPRs. gcc/testsuite/ChangeLog: * g++.dg/cpp1y/nsdmi-aggr16.C: Remove FIXME. * g++.dg/cpp1y/nsdmi-aggr17.C: Remove FIXME. * g++.dg/cpp0x/constexpr-elision1.C: New test. * g++.dg/cpp1y/constexpr-elision1.C: New test. --- gcc/cp/constexpr.cc | 7 +++ .../g++.dg/cpp0x/constexpr-elision1.C | 16 ++++++ .../g++.dg/cpp1y/constexpr-elision1.C | 53 +++++++++++++++++++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr16.C | 5 +- gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr17.C | 5 +- 5 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-elision1.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-elision1.C base-commit: 113844d68e94f4e9c0e946db351ba7d3d4a1335a diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 0dc94d9445d..5f7fc6f8f0c 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -3507,6 +3507,13 @@ cxx_eval_conditional_expression (const constexpr_ctx *ctx, tree t, val = TREE_OPERAND (t, 1); if (TREE_CODE (t) == IF_STMT && !val) val = void_node; + /* A TARGET_EXPR may be nested inside another TARGET_EXPR, but still + serve as the initializer for the same object as the outer TARGET_EXPR, + as in + A a = true ? A{} : A{}; + so strip the inner TARGET_EXPR so we don't materialize a temporary. */ + if (TREE_CODE (val) == TARGET_EXPR) + val = TARGET_EXPR_INITIAL (val); return cxx_eval_constant_expression (ctx, val, lval, non_constant_p, overflow_p, jump_target); } diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-elision1.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-elision1.C new file mode 100644 index 00000000000..9e7b9ec3405 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-elision1.C @@ -0,0 +1,16 @@ +// PR c++/105550 +// { dg-do compile { target c++11 } } + +template struct pair { + constexpr pair(int, int) {} +}; +template +pair minmax(const _Tp &__a, const _Tp &__b, + _Compare) { + return 0 ? pair(__b, __a) + : pair(__a, __b); +} +typedef int value_type; +typedef int compare_type; +template pair +minmax(const value_type &, const value_type &, compare_type); diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-elision1.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-elision1.C new file mode 100644 index 00000000000..b225ae29cde --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-elision1.C @@ -0,0 +1,53 @@ +// PR c++/105550 +// { dg-do compile { target c++14 } } + +struct A { + const A *p = this; +}; + +struct B { + const B *p = this; + constexpr operator A() const { return {}; } +}; + +constexpr A +bar (A) +{ + return {}; +} + +constexpr A baz() { return {}; } + +struct E { + A a1 = true ? A{} : A{}; + A a2 = true ? A{} : B{}; + A a3 = false ? A{} : B{}; + A a4 = false ? B{} : B{}; + A a5 = A{}; + A a6 = B{}; + A a7 = false ? B{} : (true ? A{} : A{}); + A a8 = false ? (true ? A{} : B{}) : (true ? A{} : A{}); + A a9 = (A{}); + A a10 = (true, A{}); + A a11 = bar (A{}); + A a12 = baz (); + A a13 = (A{}, A{}); +}; + +constexpr E e{}; + +constexpr A a1 = true ? A{} : A{}; +constexpr A a2 = true ? A{} : B{}; +constexpr A a3 = false ? A{} : B{}; +constexpr A a4 = false ? B{} : B{}; +constexpr A a5 = A{}; +constexpr A a6 = B{}; +constexpr A a7 = false ? B{} : (true ? A{} : A{}); +constexpr A a8 = false ? (true ? A{} : B{}) : (true ? A{} : A{}); +constexpr A a9 = (A{}); +constexpr A a10 = (true, A{}); +constexpr A a11 = bar (A{}); +//static_assert(a10.p == &a10, ""); // bug, 105619 +constexpr A a12 = baz (); +//static_assert(a11.p == &a11, ""); // bug, 105619 +constexpr A a13 = (A{}, A{}); diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr16.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr16.C index dc6492c1b0b..5e66bdd2370 100644 --- a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr16.C +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr16.C @@ -40,9 +40,8 @@ struct E { A d = true ? (false ? A{} : A{}) : (false ? A{} : A{}); }; -// FIXME: When fixing this, also fix nsdmi-aggr17.C. -constexpr E e; // { dg-bogus "" "PR105550" { xfail *-*-* } } -SA (e.a.p == &e.a); // { dg-bogus "" "PR105550" { xfail *-*-* } } +constexpr E e; +SA (e.a.p == &e.a); E e1 = { }; diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr17.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr17.C index fc27a2cdac7..ca9637b37eb 100644 --- a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr17.C +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr17.C @@ -56,9 +56,8 @@ struct F { A a = true ? A{x} : A{x}; }; -// FIXME: Doesn't work due to PR105550. -//constexpr F f; -//SA (f.a.p == &f.a); +constexpr F f; +SA (f.a.p == &f.a); SA (e.x == 42); F f2 = { }; F f3 = { 42 };