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.133.124]) by sourceware.org (Postfix) with ESMTPS id 960913858C52 for ; Thu, 2 Feb 2023 22:29:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 960913858C52 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1675376997; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/8AgUaOY71YW6MwhhzF0DzwOzX4JBH3KGBBs8aaVL8k=; b=bv8f5OuqdgmgJNC2t48LudZWSE8T3F3183wlWbPLnXzIoNFuo6n1lnnjtUIHVz9uK/cm+o mxFmfha63ZCYzOgEExCPnLBm8P7+h8m54scbzej3LgrXgtFSdve/4M00Ojwcgba86h/oYw bENrXIvly53cHSBMHXlzZfokyqOUYTA= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-396-Estney9zMy-LHm1-j_rlmQ-1; Thu, 02 Feb 2023 17:29:47 -0500 X-MC-Unique: Estney9zMy-LHm1-j_rlmQ-1 Received: by mail-qt1-f198.google.com with SMTP id f14-20020ac86ece000000b003b816f57232so1722118qtv.10 for ; Thu, 02 Feb 2023 14:29:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=/8AgUaOY71YW6MwhhzF0DzwOzX4JBH3KGBBs8aaVL8k=; b=qN+YMFO0Dnxg+rXLZ67YztSisZK5ySUYP2y8kSTk1R5YhuK+rpm5WX/7YQdb5NacPQ PGrVBrXerg1GE+rBbZuQVvZUEa0VOYFiuK4Jvy+DtGP+vT0aF/AVocUWKxOEhwzglL58 NF0kE6kzTGT+Yc6V4RFJlzjIqnw1UcJQBorvxekqRwVGKc8Q1G46TDqKNauOZVc2tOE0 nI5+n8oqYFfakdMzn8Eo2fI0jpOfZVVx5TOiDu3Y9nN9/D9kHBSUsKAeRn1QCV1znZIR UdSJcXCCpEVrG0l/g1Vlf6q6orGDtKoFa8DgC7b5Alg0/l8eV+tDEz0EY6C5tZcjuAAK uYSg== X-Gm-Message-State: AO0yUKVJE54fP46xOuxxoqz8x/MApcQ59AYR4TrHoIkargWd0qRJ0vcG vj6uEGCu5BYWaXpm/YtLe1rQF2ZMo9dc2WVq5l5edjD5bKskrehOI5UCjmNPZR0I4Im5hI1SNA2 SokxBG3/0HT+C/2kblQ== X-Received: by 2002:ac8:590b:0:b0:3b6:43ae:d5a3 with SMTP id 11-20020ac8590b000000b003b643aed5a3mr13278168qty.26.1675376986679; Thu, 02 Feb 2023 14:29:46 -0800 (PST) X-Google-Smtp-Source: AK7set/VXyCDt/IgBlyzl0PNFxOMhTjHtW0/D0Jl/MEzhzRhGyBt7WBbO3QOYxLo9tT94en9/UBysg== X-Received: by 2002:ac8:590b:0:b0:3b6:43ae:d5a3 with SMTP id 11-20020ac8590b000000b003b643aed5a3mr13278139qty.26.1675376986320; Thu, 02 Feb 2023 14:29:46 -0800 (PST) Received: from [192.168.1.108] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id b19-20020a05620a271300b0071a291f0a4asm620790qkp.27.2023.02.02.14.29.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 02 Feb 2023 14:29:45 -0800 (PST) Message-ID: <48936556-7f40-1b53-672c-b51cac05646f@redhat.com> Date: Thu, 2 Feb 2023 17:29:44 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH] c++: wrong error with constexpr array and value-init [PR108158] To: Marek Polacek , GCC Patches References: <20230131023549.454983-1-polacek@redhat.com> From: Jason Merrill In-Reply-To: <20230131023549.454983-1-polacek@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 1/30/23 21:35, Marek Polacek wrote: > In this test case, we find ourselves evaluating 't' which is > ((const struct carray *) this)->data_[VIEW_CONVERT_EXPR(index)] > in cxx_eval_array_reference. ctx->object is non-null, a RESULT_DECL, so > we replace it with 't': > > new_ctx.object = t; // result_decl replaced > > and then we go to cxx_eval_constant_expression to evaluate an > AGGR_INIT_EXPR, where we end up evaluating an INIT_EXPR (which is in the > body of the constructor for seed_or_index): > > ((struct seed_or_index *) this)->value_ = NON_LVALUE_EXPR <0> > > whereupon in cxx_eval_store_expression we go to the probe loop > where the 'this' is evaluated to > > ze_set.tables_.first_table_.data_[0] > > so the 'object' is ze_set, but that isn't in ctx->global->get_value_ptr > so we fail with a bogus error. ze_set is not there because it comes > from a different constexpr context (it's not in cv_cache either). > > The problem started with r12-2304 where I added the new_ctx.object > replacement. That was to prevent a type mismatch: the type of 't' > and ctx.object were different. > > It seems clear that we shouldn't have replaced ctx.object here. > The cxx_eval_array_reference I mentioned earlier is called from > cxx_eval_store_expression: > 6257 init = cxx_eval_constant_expression (&new_ctx, init, vc_prvalue, > 6258 non_constant_p, overflow_p); > which already created a new context, whose .object we should be > using unless, for instance, INIT contained a.b and we're evaluating > the 'a' part, which I think was the case for r12-2304; in that case > ctx.object has to be something different. > > A relatively safe fix should be to check the types before replacing > ctx.object, as in the below. Agreed. I'm trying to understand when the replacement could ever make sense, since 't' is not the target, it's the initializer. The replacement comes from Patrick's fix for 98295, but that testcase no longer hits that code (likely due to changes in empty class handling). If you add a gcc_checking_assert (false) to the replacement, does anything trip it? > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > PR c++/108158 > > gcc/cp/ChangeLog: > > * constexpr.cc (cxx_eval_array_reference): Don't replace > new_ctx.object if its type is the same as elem_type. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp1y/constexpr-108158.C: New test. > --- > gcc/cp/constexpr.cc | 8 +++-- > gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C | 32 +++++++++++++++++++ > 2 files changed, 38 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index be99bec17e7..00582cfffe2 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -4301,9 +4301,13 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t, > if (!SCALAR_TYPE_P (elem_type)) > { > new_ctx = *ctx; > - if (ctx->object) > + if (ctx->object > + && !same_type_ignoring_top_level_qualifiers_p > + (elem_type, TREE_TYPE (ctx->object))) > /* If there was no object, don't add one: it could confuse us > - into thinking we're modifying a const object. */ > + into thinking we're modifying a const object. Similarly, if > + the types are the same, replacing .object could lead to a > + failure to evaluate it (c++/108158). */ > new_ctx.object = t; > new_ctx.ctor = build_constructor (elem_type, NULL); > ctx = &new_ctx; > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C > new file mode 100644 > index 00000000000..e5f5e9954e5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C > @@ -0,0 +1,32 @@ > +// PR c++/108158 > +// { dg-do compile { target c++14 } } > + > +template struct carray { > + T data_[N]{}; > + constexpr T operator[](long index) const { return data_[index]; } > +}; > +struct seed_or_index { > +private: > + long value_ = 0; > +}; > +template struct pmh_tables { > + carray first_table_; > + template > + constexpr void lookup(KeyType, HasherType) const { > + first_table_[0]; > + } > +}; > +template struct unordered_set { > + int equal_; > + carray keys_; > + pmh_tables tables_; > + constexpr unordered_set() : equal_{} {} > + template > + constexpr auto lookup(KeyType key, Hasher hash) const { > + tables_.lookup(key, hash); > + return keys_; > + } > +}; > +constexpr unordered_set<3> ze_set; > +constexpr auto nocount = ze_set.lookup(4, int()); > +constexpr auto nocount2 = unordered_set<3>{}.lookup(4, int()); > > base-commit: 897a0502056e6cc6613f26e0b22d1c1e06b1490f