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 170953858D20 for ; Fri, 3 Feb 2023 18:08:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 170953858D20 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=1675447716; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=1BGatoneKR5z94Vabr4NC7+mR+AoLKVA7hcq713Mysw=; b=b3N8LfL3jnWkWlj9n4D6JWcNtB146XpssaFuZsenZRD1/D0jg6XrZuZ24A84cMRPSAxJlB WhTOYwiXnP0CtJN16l8Fb5K5ka7fpei7o07NKf+8OSg4qvdnudHE7tGPQp4AiXU1ihnPIk lVilvtdZKQMjR5d12oiyS7m9Sr9oVKc= Received: from mail-yb1-f197.google.com (mail-yb1-f197.google.com [209.85.219.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-443-Rons5pVAOI2u52G26d1dPw-1; Fri, 03 Feb 2023 13:08:26 -0500 X-MC-Unique: Rons5pVAOI2u52G26d1dPw-1 Received: by mail-yb1-f197.google.com with SMTP id a9-20020a25af09000000b0083fa6f15c2fso5495277ybh.16 for ; Fri, 03 Feb 2023 10:08:26 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=1BGatoneKR5z94Vabr4NC7+mR+AoLKVA7hcq713Mysw=; b=tYGl30bAqgX8Y49bP8axMXUWUdr5hV/HqA25G1xYRg+pbvyKBZ0IiPzREbXxWQrjnu RaEoSw+93mlJ4mhKAxpxcOZEiiLrEL3N7AAfCBgauQ2UDdhyll1c+0ZVw6DmtqK235il eOqulwaa7Ro8WwaX5FnWIeeDv55EW23ZBj0PG5gd1mgZy9jPWuutizxh13WeXQapFtnm HnIIRlTlBs4XrcwrSTAXxlGrytW3/sU1Ssr3QSX+KZXoYFrb0z6ptIX4Ln5mgRh1DAXt uN+MLOPCC6/umdabcMGwbC/SalPzz6yr9m8+oRCD1VZFir1/klcaraP/Fekrd1OMIF6O J+qQ== X-Gm-Message-State: AO0yUKWEoB5ZYKLYA0uh0Pd/5zllg5G9El/eDgjIrFULzzzD0il0rAz9 P6aF8FEqMB/CyjJftMIEcbImnpeSYzzW62GBHsZ7qzQhkrMRXxjaextEYikT79VDno6baRKcTZp Df5ZCbHSZWd7RHKdijg== X-Received: by 2002:a05:690c:d88:b0:518:3a72:77ee with SMTP id da8-20020a05690c0d8800b005183a7277eemr10111715ywb.35.1675447705826; Fri, 03 Feb 2023 10:08:25 -0800 (PST) X-Google-Smtp-Source: AK7set8aA0lNAErhxXEsYdNYdPRWbJNmM3V/02lUUN0rveWU5q/OnK0nO7IvPG6DSww/x0fU3dt+FA== X-Received: by 2002:a05:690c:d88:b0:518:3a72:77ee with SMTP id da8-20020a05690c0d8800b005183a7277eemr10111687ywb.35.1675447705484; Fri, 03 Feb 2023 10:08:25 -0800 (PST) Received: from redhat.com (2603-7000-9500-34a5-0000-0000-0000-1db4.res6.spectrum.com. [2603:7000:9500:34a5::1db4]) by smtp.gmail.com with ESMTPSA id g17-20020ae9e111000000b006ce580c2663sm2257904qkm.35.2023.02.03.10.08.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 03 Feb 2023 10:08:24 -0800 (PST) Date: Fri, 3 Feb 2023 13:08:23 -0500 From: Marek Polacek To: Jason Merrill Cc: GCC Patches Subject: [PATCH v2] c++: wrong error with constexpr array and value-init [PR108158] Message-ID: References: <20230131023549.454983-1-polacek@redhat.com> <48936556-7f40-1b53-672c-b51cac05646f@redhat.com> MIME-Version: 1.0 In-Reply-To: <48936556-7f40-1b53-672c-b51cac05646f@redhat.com> User-Agent: Mutt/2.2.9 (2022-11-12) 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=-12.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_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 Thu, Feb 02, 2023 at 05:29:44PM -0500, Jason Merrill wrote: > 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? It would trip in constexpr-101371.C, added in r12-2304. BUT, and I would have sworn that it ICEd when I tried, it's not necessary anymore. So it looks like we can simply remove the new_ctx.object line. At least for trunk, maybe 12 too. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- 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. It no longer seems necessary to replace new_ctx.object (likely due to changes in empty class handling). PR c++/108158 gcc/cp/ChangeLog: * constexpr.cc (cxx_eval_array_reference): Don't replace new_ctx.object. gcc/testsuite/ChangeLog: * g++.dg/cpp1y/constexpr-108158.C: New test. --- gcc/cp/constexpr.cc | 4 --- gcc/testsuite/g++.dg/cpp1y/constexpr-108158.C | 32 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 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 5b31f9c27d1..564766c8a00 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -4301,10 +4301,6 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t, if (!SCALAR_TYPE_P (elem_type)) { new_ctx = *ctx; - if (ctx->object) - /* If there was no object, don't add one: it could confuse us - into thinking we're modifying a const object. */ - 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: c9aef107ce697f58a34734d82f8d2514405c9be0 -- 2.39.1