From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 8263C385BF83 for ; Mon, 6 Apr 2020 21:39:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8263C385BF83 Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-235-iPWXspATNVCuXwsWg5FaAQ-1; Mon, 06 Apr 2020 17:39:54 -0400 X-MC-Unique: iPWXspATNVCuXwsWg5FaAQ-1 Received: by mail-qk1-f200.google.com with SMTP id w21so1194351qkj.18 for ; Mon, 06 Apr 2020 14:39:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=VwAfP9+B4FkraabMd73NysJicjGjTgwNq8OrA5wslG8=; b=bEPHsu5O64DIaev6xZmSIhlEbBnY6MKbMLXQmpZxCdjDZgUeGwSZh/q8dU2yH+0mQf s0ui5EjvGVD3KuryUZ1MHFyb51C2Z3dEmDPNdWvbNzhVs0ZhPTH+eXc6TiTDaITkiynl y3+kCA+Vt0Y7EBnejC5uC9wxoUKkz6VXv5z8WhHEtgjqXE76g3YkSNjeFrBvVM27fdZS BVhbhNmb0B1N3ZjhUUGMbsuDgD+afNuORJNlbrQ0c1TpQGEn8dO+KQbk7qlvoJKW5rn9 pWeBZ+PYs0JhEAYMRwm7/7AxqOq9TBnx94i6dKEDA59kdaE4yiPR+JPhcNv0qfoA7Fo7 MTTg== X-Gm-Message-State: AGi0PuZJ66MY0WuLyMR1dt71uHsIQiwYaHe9E7O9/q0VYKunztPtUl0T vwGWODh5+ap5dglvBlkfa6L/oq331TDLi7sX+18uqspnSxuW8MxSiYWoG75PmD6BlssWeBFuM7B qoRel0DAW9axCbXj7Iw== X-Received: by 2002:a37:9684:: with SMTP id y126mr23360323qkd.151.1586209193630; Mon, 06 Apr 2020 14:39:53 -0700 (PDT) X-Google-Smtp-Source: APiQypLfgR1RbFtuKKoCldytucCtevQriqwxXF8Xd9ufllwuf7sGVhbNeCWHQMJx1ukNftv67WOyEw== X-Received: by 2002:a37:9684:: with SMTP id y126mr23360301qkd.151.1586209193230; Mon, 06 Apr 2020 14:39:53 -0700 (PDT) Received: from [192.168.1.148] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id 10sm15272164qtt.54.2020.04.06.14.39.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 06 Apr 2020 14:39:52 -0700 (PDT) Subject: Re: [PATCH] c++: Fix usage of CONSTRUCTOR_PLACEHOLDER_BOUNDARY inside array initializers [90996] To: Patrick Palka , gcc-patches@gcc.gnu.org References: <20200406190756.1798784-1-ppalka@redhat.com> From: Jason Merrill Message-ID: Date: Mon, 6 Apr 2020 17:39:51 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.0 MIME-Version: 1.0 In-Reply-To: <20200406190756.1798784-1-ppalka@redhat.com> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-30.0 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, 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: Mon, 06 Apr 2020 21:39:58 -0000 On 4/6/20 3:07 PM, Patrick Palka wrote: > This PR reports that since the introduction of the > CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag, we are sometimes failing to resolve > PLACEHOLDER_EXPRs inside array initializers that refer to some inner > constructor. In the testcase in the PR, we have as the initializer for "S c[];" > the following > > {{.a=(int &) &_ZGR1c_, .b={*(&)->a}}} > > where CONSTRUCTOR_PLACEHOLDER_BOUNDARY is set on the second outermost > constructor. However, we pass the whole initializer to replace_placeholders in > store_init_value, and so due to the flag being set on the second outermost ctor > it avoids recursing into the innermost constructor and we fail to resolve the > PLACEHOLDER_EXPR within. > > To fix this, we could perhaps either call replace_placeholders in more places, > or we could change where we set CONSTRUCTOR_PLACEHOLDER_BOUNDARY. This patch > takes the latter approach -- when building up an array initializer, it bubbles > any CONSTRUCTOR_PLACEHOLDER_BOUNDARY flag set on the element initializers up to > the array initializer. Doing so shouldn't create any new PLACEHOLDER_EXPR > resolution ambiguities because we don't deal with PLACEHOLDER_EXPRs of array > type in the frontend, as far as I can tell. Interesting. Yes, that sounds like it should work. > Does this look OK to comit after testing? Yes. Though I'm seeing "after testing" a lot; what testing are you doing before sending patches? > gcc/cp/ChangeLog: > > PR c++/90996 > * tree.c (replace_placeholders): Look through all handled components, > not just COMPONENT_REFs. > * typeck2.c (process_init_constructor_array): Propagate > CONSTRUCTOR_PLACEHOLDER_BOUNDARY up from each element initializer to > the array initializer. > > gcc/testsuite/ChangeLog: > > PR c++/90996 > * g++.dg/cpp1y/pr90996.C: New test. > --- > gcc/cp/tree.c | 2 +- > gcc/cp/typeck2.c | 18 ++++++++++++++++++ > gcc/testsuite/g++.dg/cpp1y/pr90996.C | 17 +++++++++++++++++ > 3 files changed, 36 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr90996.C > > diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c > index 5eb0dcd717a..d1192b7e094 100644 > --- a/gcc/cp/tree.c > +++ b/gcc/cp/tree.c > @@ -3247,7 +3247,7 @@ replace_placeholders (tree exp, tree obj, bool *seen_p /*= NULL*/) > > /* If the object isn't a (member of a) class, do nothing. */ > tree op0 = obj; > - while (TREE_CODE (op0) == COMPONENT_REF) > + while (handled_component_p (op0)) > op0 = TREE_OPERAND (op0, 0); > if (!CLASS_TYPE_P (strip_array_types (TREE_TYPE (op0)))) > return exp; > diff --git a/gcc/cp/typeck2.c b/gcc/cp/typeck2.c > index cf1cb5ace66..fe844bc08bb 100644 > --- a/gcc/cp/typeck2.c > +++ b/gcc/cp/typeck2.c > @@ -1488,6 +1488,17 @@ process_init_constructor_array (tree type, tree init, int nested, int flags, > = massage_init_elt (TREE_TYPE (type), ce->value, nested, flags, > complain); > > + if (TREE_CODE (ce->value) == CONSTRUCTOR > + && CONSTRUCTOR_PLACEHOLDER_BOUNDARY (ce->value)) > + { > + /* Shift CONSTRUCTOR_PLACEHOLDER_BOUNDARY from the element initializer > + up to the array initializer, so that the call to > + replace_placeholders from store_init_value can resolve any > + PLACEHOLDER_EXPRs within this element initializer. */ > + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (ce->value) = 0; > + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = 1; > + } > + > gcc_checking_assert > (ce->value == error_mark_node > || (same_type_ignoring_top_level_qualifiers_p > @@ -1516,6 +1527,13 @@ process_init_constructor_array (tree type, tree init, int nested, int flags, > /* The default zero-initialization is fine for us; don't > add anything to the CONSTRUCTOR. */ > next = NULL_TREE; > + else if (TREE_CODE (next) == CONSTRUCTOR > + && CONSTRUCTOR_PLACEHOLDER_BOUNDARY (next)) > + { > + /* As above. */ > + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (next) = 0; > + CONSTRUCTOR_PLACEHOLDER_BOUNDARY (init) = 1; > + } > } > else if (!zero_init_p (TREE_TYPE (type))) > next = build_zero_init (TREE_TYPE (type), > diff --git a/gcc/testsuite/g++.dg/cpp1y/pr90996.C b/gcc/testsuite/g++.dg/cpp1y/pr90996.C > new file mode 100644 > index 00000000000..780cbb4e3ac > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/pr90996.C > @@ -0,0 +1,17 @@ > +// PR c++/90996 > +// { dg-do compile { target c++14 } } > + > +struct S > +{ > + int &&a = 2; > + int b[1] {a}; > +}; > + > +S c[2][2] {{{5}}}; > + > +struct T > +{ > + S c[2][2] {{{7}}}; > +}; > + > +T d {}; >