From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by sourceware.org (Postfix) with ESMTP id E18C13857C59 for ; Wed, 9 Sep 2020 19:33:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org E18C13857C59 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-550-8hw6jZmwOtOytwU2UtUVtw-1; Wed, 09 Sep 2020 15:33:48 -0400 X-MC-Unique: 8hw6jZmwOtOytwU2UtUVtw-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0F14E801AEE for ; Wed, 9 Sep 2020 19:33:48 +0000 (UTC) Received: from redhat.com (ovpn-116-75.rdu2.redhat.com [10.10.116.75]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A1CD327BD8; Wed, 9 Sep 2020 19:33:47 +0000 (UTC) Date: Wed, 9 Sep 2020 15:33:45 -0400 From: Marek Polacek To: Jason Merrill Cc: GCC Patches Subject: Re: [PATCH v2] c++: Fix ICE in reshape_init with init-list [PR95164] Message-ID: <20200909193345.GZ5926@redhat.com> References: <20200904213922.2543048-1-polacek@redhat.com> <966f88ec-b6d8-0803-ef1a-fbff81583cc9@redhat.com> MIME-Version: 1.0 In-Reply-To: <966f88ec-b6d8-0803-ef1a-fbff81583cc9@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0.002 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-14.1 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_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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, 09 Sep 2020 19:33:52 -0000 On Mon, Sep 07, 2020 at 06:23:01PM -0400, Jason Merrill via Gcc-patches wrote: > On 9/4/20 5:39 PM, Marek Polacek wrote: > > This patch fixes a long-standing bug in reshape_init_r. Since r209314 > > we implement DR 1467 which handles list-initialization with a single > > initializer of the same type as the target. In this test this causes > > a crash in reshape_init_r when we're processing a constructor that has > > undergone the DR 1467 transformation. > > > > Take e.g. the > > > > foo({{1, {H{k}}}}); > > > > line in the attached test. {H{k}} initializes the field b of H in I. > > H{k} is a functional cast, so has TREE_HAS_CONSTRUCTOR set, so is > > COMPOUND_LITERAL_P. We perform the DR 1467 transformation and turn > > {H{k}} into H{k}. Then we attempt to reshape H{k} again and since > > first_initializer_p is null and it's COMPOUND_LITERAL_P, we go here: > > > > else if (COMPOUND_LITERAL_P (stripped_init)) > > gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init)); > > It looks to me like the bug is here: > > > /* [dcl.init.aggr] > > All implicit type conversions (clause _conv_) are considered when > > initializing the aggregate member with an initializer from an > > initializer-list. If the initializer can initialize a member, > > the member is initialized. Otherwise, if the member is itself a > > non-empty subaggregate, brace elision is assumed and the > > initializer is considered for the initialization of the first > > member of the subaggregate. */ > > if (TREE_CODE (init) != CONSTRUCTOR > > /* But don't try this for the first initializer, since that would > > be looking through the > > outermost braces; A a2 = { a1 }; is not a > > valid aggregate initialization. */ > > && !first_initializer_p > > && (same_type_ignoring_top_level_qualifiers_p (type, TREE_TYPE (init)) > > || can_convert_arg (type, TREE_TYPE (init), init, LOOKUP_NORMAL, > > complain))) > > { > > d->cur++; > > return init; > > } > > We ought to handle H{k} here, treat it as the initializer for the member, > and not get as far as the code you quote above. Like this? When we have a COMPOUND_LITERAL_P, then I think we don't need to check cxx11, or CLASS_TYPE, or d.end - d.cur, because that's inherent. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10? -- >8 -- This patch fixes a long-standing bug in reshape_init_r. Since r209314 we implement DR 1467 which handles list-initialization with a single initializer of the same type as the target. In this test this causes a crash in reshape_init_r when we're processing a constructor that has undergone the DR 1467 transformation. Take e.g. the foo({{1, {H{k}}}}); line in the attached test. {H{k}} initializes the field b of H in I. H{k} is a functional cast, so has TREE_HAS_CONSTRUCTOR set, so is COMPOUND_LITERAL_P. We perform the DR 1467 transformation and turn {H{k}} into H{k}. Then we attempt to reshape H{k} again and since first_initializer_p is null and it's COMPOUND_LITERAL_P, we go here: else if (COMPOUND_LITERAL_P (stripped_init)) gcc_assert (!BRACE_ENCLOSED_INITIALIZER_P (stripped_init)); then complain about the missing braces, go to reshape_init_class and ICE on gcc_checking_assert (d->cur->index == get_class_binding (type, id)); because due to the missing { } we're looking for 'b' in H, but that's not found. So we have to be prepared to handle an initializer whose outer braces have been removed due to DR 1467. gcc/cp/ChangeLog: PR c++/95164 * decl.c (reshape_init_r): When we've found a missing set of braces as a result of the DR 1467 transformation, don't reshape again. gcc/testsuite/ChangeLog: PR c++/95164 * g++.dg/cpp0x/initlist123.C: New test. --- gcc/cp/decl.c | 8 ++++- gcc/testsuite/g++.dg/cpp0x/initlist123.C | 39 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/initlist123.C diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 31d68745844..6565cd7199b 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -6466,7 +6466,13 @@ reshape_init_r (tree type, reshape_iter *d, tree first_initializer_p, non-empty subaggregate, brace elision is assumed and the initializer is considered for the initialization of the first member of the subaggregate. */ - if (TREE_CODE (init) != CONSTRUCTOR + if ((TREE_CODE (init) != CONSTRUCTOR + /* If we previously elided the braces around the single element + of an initializer list when initializing an object of the same + class type, don't report missing braces or reshape again. In + this case the braces had been enclosing a compound literal or + functional cast with aggregate, e.g. {S{}} -> S{}. */ + || COMPOUND_LITERAL_P (init)) /* But don't try this for the first initializer, since that would be looking through the outermost braces; A a2 = { a1 }; is not a valid aggregate initialization. */ diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist123.C b/gcc/testsuite/g++.dg/cpp0x/initlist123.C new file mode 100644 index 00000000000..29f037f07ef --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/initlist123.C @@ -0,0 +1,39 @@ +// PR c++/95164 +// { dg-do compile { target c++11 } } +// { dg-options "-Wmissing-braces" } + +struct H { + int a; +}; + +struct X : H { }; + +struct I { + int c; + H b; +}; +struct E { I d; }; +void foo(E); + +template +void fn () +{ + int a = 42; + int &k = a; + + foo({1, {H{k}}}); // { dg-warning "missing braces around initializer for .I." } + foo({1, {X{k}}}); // { dg-warning "missing braces around initializer for .I." } + + foo({{1, {k}}}); + foo({{1, {N}}}); + + foo({{1, H{k}}}); + foo({{1, H{N}}}); + foo({{1, X{k}}}); + foo({{1, X{N}}}); + + foo({{1, {H{k}}}}); + foo({{1, {H{N}}}}); + foo({{1, {X{k}}}}); + foo({{1, {X{N}}}}); +} base-commit: 919373a6bfff415db7676c9f92a356ddfc501dfe -- 2.26.2