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.129.124]) by sourceware.org (Postfix) with ESMTPS id 82D2E3858D28 for ; Wed, 27 Apr 2022 20:30:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 82D2E3858D28 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-491-BtZP048FOCSGEnG8UCIpIg-1; Wed, 27 Apr 2022 16:30:36 -0400 X-MC-Unique: BtZP048FOCSGEnG8UCIpIg-1 Received: by mail-qk1-f198.google.com with SMTP id s63-20020a372c42000000b0069ec0715d5eso1897556qkh.10 for ; Wed, 27 Apr 2022 13:30:34 -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=bUX/yjtHOS+eoWJXjeD8ORWvdGtFtxEf78v1Kd2tvBM=; b=dvTFOr7NSsBlVGWtdpLAicQjJDWr8dCvegQZFAK0V1k6HdYBntmkWNIFLA/O5waQzL ppdNq3cDFZKoOYRa15uomngwTGrJ1BlEi0Yj9edPN0PO4yAHd8J7CmErnk6UhLQWwTlf vFVNdigByWi2veYrz6igWffqXu8W4ve0TRDPiwgJdyLOqzY03KroKmgWDIezSqwKnO3t zAzlxF/vq7j1OfxCpBxZ0rvRAruwPkazJ+GuPJ6UG4IMcRgjK9Fm2anCM0RdLw0R4aj+ zkNatsWQG6Vl0kzW/x24G8bvpv2V++8LpsRVTw6yFtI3x1gUNSTYYKpALR0OlEj8/ru1 +sTg== X-Gm-Message-State: AOAM530rznaQU3D5YpCYBeTdw8e6NGlrTWSldIVmkKfbNiLcd4eWwVdB axRQ7792CL7BO6P+cOepYLNBwDagrrBvWQP6oLXOA0+zM+2j2r68j3TUfC7tCqdCu+Zu4Kaiyb1 Ofqz4B8cO2iZU9lhFhA== X-Received: by 2002:ac8:5a09:0:b0:2f3:69a9:8bb3 with SMTP id n9-20020ac85a09000000b002f369a98bb3mr11416429qta.121.1651091433877; Wed, 27 Apr 2022 13:30:33 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxfg3doFsWd/uGHR+NP15s7aDpizJbXgjTPClh6v1pBnO2YNW0VuSmvcDK0S7GQyIzj55pV3g== X-Received: by 2002:ac8:5a09:0:b0:2f3:69a9:8bb3 with SMTP id n9-20020ac85a09000000b002f369a98bb3mr11416398qta.121.1651091433500; Wed, 27 Apr 2022 13:30:33 -0700 (PDT) Received: from redhat.com ([2601:184:4780:4310::f826]) by smtp.gmail.com with ESMTPSA id c20-20020a05622a059400b002e1d59e68f3sm10683359qtb.48.2022.04.27.13.30.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 27 Apr 2022 13:30:32 -0700 (PDT) Date: Wed, 27 Apr 2022 16:30:30 -0400 From: Marek Polacek To: Patrick Palka Cc: GCC Patches , Jason Merrill , Jakub Jelinek Subject: [PATCH v2] c++: ICE with temporary of class type in DMI [PR100252] Message-ID: References: <20220426230226.677300-1-polacek@redhat.com> <8e0bc483-7fba-225c-807b-dbe89efceb5e@idea> MIME-Version: 1.0 In-Reply-To: <8e0bc483-7fba-225c-807b-dbe89efceb5e@idea> User-Agent: Mutt/2.1.5 (2021-12-30) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-13.8 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_LOW, 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: Wed, 27 Apr 2022 20:30:39 -0000 On Wed, Apr 27, 2022 at 11:00:46AM -0400, Patrick Palka wrote: > On Tue, 26 Apr 2022, Marek Polacek wrote: > > > Consider > > > > struct A { > > int x; > > int y = x; > > }; > > > > struct B { > > int x = 0; > > int y = A{x}.y; // #1 > > }; > > > > where for #1 we end up with > > > > {.x=(&)->x, .y=(&)->x} > > > > that is, two PLACEHOLDER_EXPRs for different types on the same level in > > a {}. This crashes because our CONSTRUCTOR_PLACEHOLDER_BOUNDARY mechanism to > > avoid replacing unrelated PLACEHOLDER_EXPRs cannot deal with it. > > > > Here's why we wound up with those PLACEHOLDER_EXPRs: When we're performing > > cp_parser_late_parsing_nsdmi for "int y = A{x}.y;" we use finish_compound_literal > > on type=A, compound_literal={((struct B *) this)->x}. When digesting this > > initializer, we call get_nsdmi which creates a PLACEHOLDER_EXPR for A -- we don't > > have any object to refer to yet. After digesting, we have > > > > {.x=((struct B *) this)->x, .y=(&)->x} > > > > and since we've created a PLACEHOLDER_EXPR inside it, we marked the whole ctor > > CONSTRUCTOR_PLACEHOLDER_BOUNDARY. f_c_l creates a TARGET_EXPR and returns > > > > TARGET_EXPR x, .y=(&)->x}> > > > > Then we get to > > > > B b = {}; > > > > and call store_init_value, which digest the {}, which produces > > > > {.x=NON_LVALUE_EXPR <0>, .y=(TARGET_EXPR )->x, .y=(&)->x}>).y} > > > > The call to replace_placeholders in store_init_value will not do anything: > > we've marked the inner { } CONSTRUCTOR_PLACEHOLDER_BOUNDARY, and it's only > > a sub-expression, so replace_placeholders does nothing, so the > > stays even though now is the perfect time to replace it because we have an > > object for it: 'b'. > > > > Later, in cp_gimplify_init_expr the *expr_p is > > > > D.2395 = {.x=(&)->x, .y=(&)->x} > > > > where D.2395 is of type A, but we crash because we hit , which > > has a different type. > > > > My idea was to replace with D.2384 in f_c_l after creating the > > TARGET_EXPR because that means we have an object we can refer to. Then clear > > CONSTRUCTOR_PLACEHOLDER_BOUNDARY because we no longer have a PLACEHOLDER_EXPR > > in the {}. Then store_init_value will be able to replace with > > 'b', and we should be good to go. > > Makes sense to me. It seems all was well until break_out_target_exprs, > called from get_nsdmi for B::y, replaced the 'this' in the initializer > > (TARGET_EXPR x, .y=(&)->x}>).y; > > with a PLACEHOLDER_EXPR; > > (TARGET_EXPR )->x, .y=(&)->x}>).y; > > This seems to be the wrong thing to do when the 'this' appears inside a > CONSTRUCTOR_PLACEHOLDER_BOUNDARY constructor because the new > PLACEHOLDER_EXPR then can't be resolved correctly. Exactly. > So in light of this I wonder if we should instead perform this handling > you added to finish_compound_literal in break_out_target_exprs / > bot_manip instead? Unfortunately that causes an ICE in gimplify_var_or_parm_decl on the new testcase I've added here. bot_manip is a different context and so I can't use parsing_nsdmi anymore, and it seems we'd replace the placeholders too aggressively in bot_manip. So I'm not sure if that's the best place. -- >8 -- Consider struct A { int x; int y = x; }; struct B { int x = 0; int y = A{x}.y; // #1 }; where for #1 we end up with {.x=(&)->x, .y=(&)->x} that is, two PLACEHOLDER_EXPRs for different types on the same level in a {}. This crashes because our CONSTRUCTOR_PLACEHOLDER_BOUNDARY mechanism to avoid replacing unrelated PLACEHOLDER_EXPRs cannot deal with it. Here's why we wound up with those PLACEHOLDER_EXPRs: When we're performing cp_parser_late_parsing_nsdmi for "int y = A{x}.y;" we use finish_compound_literal on type=A, compound_literal={((struct B *) this)->x}. When digesting this initializer, we call get_nsdmi which creates a PLACEHOLDER_EXPR for A -- we don't have any object to refer to yet. After digesting, we have {.x=((struct B *) this)->x, .y=(&)->x} and since we've created a PLACEHOLDER_EXPR inside it, we marked the whole ctor CONSTRUCTOR_PLACEHOLDER_BOUNDARY. f_c_l creates a TARGET_EXPR and returns TARGET_EXPR x, .y=(&)->x}> Then we get to B b = {}; and call store_init_value, which digest the {}, which produces {.x=NON_LVALUE_EXPR <0>, .y=(TARGET_EXPR )->x, .y=(&)->x}>).y} The call to replace_placeholders in store_init_value will not do anything: we've marked the inner { } CONSTRUCTOR_PLACEHOLDER_BOUNDARY, and it's only a sub-expression, so replace_placeholders does nothing, so the stays even though now is the perfect time to replace it because we have an object for it: 'b'. Later, in cp_gimplify_init_expr the *expr_p is D.2395 = {.x=(&)->x, .y=(&)->x} where D.2395 is of type A, but we crash because we hit , which has a different type. My idea was to replace with D.2384 in f_c_l after creating the TARGET_EXPR because that means we have an object we can refer to. Then clear CONSTRUCTOR_PLACEHOLDER_BOUNDARY because we no longer have a PLACEHOLDER_EXPR in the {}. Then store_init_value will be able to replace with 'b', and we should be good to go. PR c++/100252 gcc/cp/ChangeLog: * semantics.cc (finish_compound_literal): replace_placeholders after creating the TARGET_EXPR. gcc/testsuite/ChangeLog: * g++.dg/cpp1y/nsdmi-aggr14.C: New test. * g++.dg/cpp1y/nsdmi-aggr15.C: New test. --- gcc/cp/semantics.cc | 31 +++++++++++++++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr14.C | 46 +++++++++++++++++++++++ gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr15.C | 29 ++++++++++++++ 3 files changed, 106 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr14.C create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr15.C diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index ab48f11c9be..770369458bb 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -3296,6 +3296,37 @@ finish_compound_literal (tree type, tree compound_literal, if (TREE_CODE (compound_literal) == CONSTRUCTOR) TREE_HAS_CONSTRUCTOR (compound_literal) = false; compound_literal = get_target_expr_sfinae (compound_literal, complain); + /* We may have A{} in a NSDMI. */ + if (parsing_nsdmi ()) + { + /* Digesting the {} could have introduced a PLACEHOLDER_EXPR + referring to A. Now that we've built up a TARGET_EXPR, we + have an object we can refer to. The reason we bother doing + this here is for code like + + struct A { + int x; + int y = x; + }; + + struct B { + int x = 0; + int y = A{x}.y; // #1 + }; + + where in #1 we don't want to end up with two PLACEHOLDER_EXPRs + for different types on the same level in a {} as in 100252. */ + tree init = TARGET_EXPR_INITIAL (compound_literal); + if (TREE_CODE (init) == CONSTRUCTOR + && CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init)) + { + tree obj = TARGET_EXPR_SLOT (compound_literal); + replace_placeholders (compound_literal, obj); + /* We should have dealt with the PLACEHOLDER_EXPRs. */ + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = false; + gcc_checking_assert (!find_placeholders (init)); + } + } } else /* For e.g. int{42} just make sure it's a prvalue. */ diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr14.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr14.C new file mode 100644 index 00000000000..7d508f52b48 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr14.C @@ -0,0 +1,46 @@ +// PR c++/100252 +// { dg-do run { target c++14 } } + +#define SA(X) static_assert ((X),#X) + +struct A { + int x; + int y = x; +}; + +struct B { + int x = 0; + int y = A{x}.y; +}; + +constexpr B csb1 = { }; +SA(csb1.x == 0 && csb1.y == csb1.x); +constexpr B csb2 = { 1 }; +SA(csb2.x == 1 && csb2.y == csb2.x); +constexpr B csb3 = { 1, 2 }; +SA(csb3.x == 1 && csb3.y == 2); + +B sb1 = { }; +B sb2 = { 1 }; +B sb3 = { 1, 2}; + +int +main () +{ + if (sb1.x != 0 || sb1.x != sb1.y) + __builtin_abort(); + if (sb2.x != 1 || sb2.x != sb2.y) + __builtin_abort(); + if (sb3.x != 1 || sb3.y != 2) + __builtin_abort(); + + B b1 = { }; + B b2 = { 1 }; + B b3 = { 1, 2}; + if (b1.x != 0 || b1.x != b1.y) + __builtin_abort(); + if (b2.x != 1 || b2.x != b2.y) + __builtin_abort(); + if (b3.x != 1 || b3.y != 2) + __builtin_abort(); +} diff --git a/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr15.C b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr15.C new file mode 100644 index 00000000000..bc997bb5e1d --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr15.C @@ -0,0 +1,29 @@ +// PR c++/100252 +// { dg-do run { target c++14 } } + +struct A { + int x; + int y = x; +}; + +struct B { + int x = 0; + int y = A{x}.y; +}; + +void +g (B b1 = B{}, B b2 = B{1}, B b3 = B{1, 2}) +{ + if (b1.x != 0 || b1.y != b1.x) + __builtin_abort(); + if (b2.x != 1 || b2.y != b2.x) + __builtin_abort(); + if (b3.x != 1 || b3.y != 2) + __builtin_abort(); +} + +int +main () +{ + g (); +} base-commit: 409edcca331296b53842c50d3b789e1b1ccc05e5 -- 2.35.1