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 986873858D33 for ; Wed, 1 Mar 2023 17:54:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 986873858D33 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=1677693261; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pU1X63OjsG9TVJkpTLiSJqg/4dhsp0aFk2qTIYSbAIM=; b=dl2yJ1+otyNacPQNAz8frfNxv1v5MZGmYZFjUWB1RQVVDSPDtIrZWnXYvDRv7p9KmwUCLC +CwGJvSA9THN29NsRuGE8w+1tv2pqeyqsgLGJqi0oU8jwYaNaSiQmNIgsN51l+xl67qcWg LhIkx4JTa2JZtRiUYRl3/nVKL2Fzmpo= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-94-FjHjIgBAMY6B-2Ql74wBqw-1; Wed, 01 Mar 2023 12:54:19 -0500 X-MC-Unique: FjHjIgBAMY6B-2Ql74wBqw-1 Received: by mail-qv1-f70.google.com with SMTP id d27-20020a0caa1b000000b00572765a687cso7415289qvb.19 for ; Wed, 01 Mar 2023 09:54:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1677693259; h=content-transfer-encoding:in-reply-to:from:references:cc: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=pU1X63OjsG9TVJkpTLiSJqg/4dhsp0aFk2qTIYSbAIM=; b=1NjC22kq8Ryfu/2CznoCQa9VQZSNct88dt2RzkAtJdJXOzQf890oYQUgjkhqzElgAF WBGbk5Z1YodSzjX9q5nHX/Cny/ynS7EDmsLMx6MX7JgrEPrBV8xIbt7FgPOGc9/Eoc1H HJQvMnavkunadyTRyUeEjOvEDJRP89q43wEWCRw9kTje9UwE375/J/4vYIC6V/b4FfoA kiRkAcJ+IFvxgQ3U2tNUZ0lPpG8gV6g5v/rXETlmYA5R3iFWLe6GV4zgvU2xa/d4gAUG MbSsoNyMF3tRW/sIW+blr/eJB8M+AkVkbwOh0pReuYVSDR04e2Ojk4/pfVxTOjuXg4cI 6QiQ== X-Gm-Message-State: AO0yUKXhVIm/xB/2FPIRFM7VvdZtC4dDBbbTjH77UeInFZ3Jdrno/mDy 09pF33j6MS7DN1MyAXC/5s5dALgvY27eeQaaiZye8VczyK4QTTUsfin+GEdpi2gkgwFNEcElD12 7heV/dZbVpzO8Qxx0ZA== X-Received: by 2002:a05:622a:4d3:b0:3b8:6442:2575 with SMTP id q19-20020a05622a04d300b003b864422575mr12393127qtx.49.1677693258702; Wed, 01 Mar 2023 09:54:18 -0800 (PST) X-Google-Smtp-Source: AK7set9Gi2os/Y1pWYUdQdNwO141Jq4O9JQ5ByB+GSsUeacasc98O+sqEkltSMxqNxbdb4TQjxyk1A== X-Received: by 2002:a05:622a:4d3:b0:3b8:6442:2575 with SMTP id q19-20020a05622a04d300b003b864422575mr12393100qtx.49.1677693258324; Wed, 01 Mar 2023 09:54:18 -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 k8-20020a05620a142800b0073ba211e765sm9294990qkj.19.2023.03.01.09.54.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 01 Mar 2023 09:54:17 -0800 (PST) Message-ID: <8d9f318b-c96a-bcec-a78b-2d3d371db114@redhat.com> Date: Wed, 1 Mar 2023 12:54:16 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [PATCH] c++: unevaluated array new-expr size constantness [PR108219] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org References: <20230222194509.3606756-1-ppalka@redhat.com> <91252711-83ec-0ded-1955-e809c6c437da@redhat.com> <4e5ba586-7d17-346b-f6ee-42d8c6358225@redhat.com> <44659ce5-beeb-bb9e-f820-68235f20f0ed@idea> From: Jason Merrill In-Reply-To: <44659ce5-beeb-bb9e-f820-68235f20f0ed@idea> 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.6 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 3/1/23 12:20, Patrick Palka wrote: > On Wed, 1 Mar 2023, Jason Merrill wrote: > >> On 3/1/23 10:32, Patrick Palka wrote: >>> On Mon, 27 Feb 2023, Jason Merrill wrote: >>> >>>> On 2/22/23 14:45, Patrick Palka wrote: >>>>> Here we're mishandling the unevaluated array new-expressions due to a >>>>> supposed non-constant array size ever since r12-5253-g4df7f8c79835d569, >>>>> made us no longer perform constant evaluation of non-manifestly-constant >>>>> expressions within unevaluated contexts. This shouldn't make a >>>>> difference here since the array sizes are constant literals, except >>>>> they're actually NON_LVALUE_EXPR location wrappers wrapping INTEGER_CST, >>>>> wrappers which used to get stripped as part of constant evaluation and >>>>> now no longer do. Moreover it means build_vec_init can't constant fold >>>>> the 'maxindex' passed from build_new_1 (since it uses >>>>> maybe_constant_value >>>>> with mce_unknown). >>>> >>>> Hmm, now that you mention it I think the >>>> >>>> if (manifestly_const_eval != mce_unknown) >>>> >>>> change in maybe_constant_value isn't quite right, we don't want to force >>>> evaluation in unevaluated mce_false context either. >>> >>> Ah, makes sense. Fixed in the below patch. >>> >>>> >>>>> This patch fixes the first issue by making maybe_constant_value and >>>>> fold_non_dependent_expr_template shortcut handling location wrappers >>>>> around constant nodes, and the second issue by using fold_build2_loc >>>>> instead of cp_build_binary_op when computing the maxindex to pass to >>>>> build_vec_init. >>>> >>>> Maybe in unevaluated mce_unknown/false context maybe_constant_value should >>>> call fold? >>> >>> That seems like a good compromise between proper constant evaluation >>> and not constant evaluating at all, though I wonder how 'fold' behaves >>> w.r.t. to undefined behavior such as division by zero and signed overflow? >> >> 'fold' doesn't fold division by zero, but I think we should only return the >> result of 'fold' at this point if it is in fact constant, not if it's a >> non-constant simplification. > > Sounds good, I wasn't sure if 'fold' could return a non-constant > simplification. Yep, it also folds e.g. x*1 to x. > I suppose we want to be pretty conservative with the > constantness test, so I went with CONSTANT_CLASS_P && !TREE_OVERFLOW. Makes sense. > Like so? Smoke tested so far, bootstrap and regtest on > x86_64-pc-linu-xgnu in progress. > > -- >8 -- > > Subject: [PATCH] c++: unevaluated array new-expr size constantness [PR108219] > > Here we're mishandling the unevaluated array new-expressions due to a > supposed non-constant array size ever since r12-5253-g4df7f8c79835d569 > made us no longer perform constant evaluation of non-manifestly-constant > expressions within unevaluated contexts. This shouldn't make a > difference here since the array sizes are constant literals, except > these sizes are expressed as NON_LVALUE_EXPR location wrappers around > INTEGER_CST, wrappers which used to get stripped as part of constant > evaluation and now no longer do. Moreover it means build_vec_init can't > constant fold the 'maxindex' passed from build_new_1 (since it uses > maybe_constant_value with mce_unknown). > > This patch fixes this by making maybe_constant_value and > fold_non_dependent_expr at least try folding simple unevaluated operands > via fold(), which will evaluate simple arithmetic, look through location > wrappers, perform integral conversions, etc. > > Co-authored-by: Jason Merrill > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk/12? > > PR c++/108219 > PR c++/108218 > > gcc/cp/ChangeLog: > > * constexpr.cc (maybe_constant_value): Move up early exit > test for unevaluated operands. Try reducing an unevaluated > operand to a constant via fold. > (fold_non_dependent_expr_template): Add early exit test for > CONSTANT_CLASS_P nodes. Try reducing an unevaluated operand > to a constant via fold. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/new6.C: New test. > * g++.dg/cpp2a/concepts-new1.C: New test. > --- > gcc/cp/constexpr.cc | 23 +++++++++++++++++----- > gcc/testsuite/g++.dg/cpp0x/new6.C | 13 ++++++++++++ > gcc/testsuite/g++.dg/cpp2a/concepts-new1.C | 13 ++++++++++++ > 3 files changed, 44 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/new6.C > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-new1.C > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index b4d3e95bbd5..324968050ba 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -8523,6 +8523,14 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */, > /* No caching or evaluation needed. */ > return t; > > + /* Don't constant evaluate an unevaluated non-manifestly-constant operand, > + but at least try folding simple expressions to a constant. */ > + if (cp_unevaluated_operand && manifestly_const_eval != mce_true) > + { > + tree r = fold (t); > + return CONSTANT_CLASS_P (r) && !TREE_OVERFLOW (r) ? r : t; > + } > + > if (manifestly_const_eval != mce_unknown) > return cxx_eval_outermost_constant_expr (t, true, true, > manifestly_const_eval, false, decl); > @@ -8544,10 +8552,6 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */, > return r; > } > > - /* Don't evaluate an unevaluated operand. */ > - if (cp_unevaluated_operand) > - return t; > - > uid_sensitive_constexpr_evaluation_checker c; > r = cxx_eval_outermost_constant_expr (t, true, true, > manifestly_const_eval, false, decl); > @@ -8612,8 +8616,17 @@ fold_non_dependent_expr_template (tree t, tsubst_flags_t complain, > return t; > } > > + if (CONSTANT_CLASS_P (t)) > + /* No evaluation needed. */ > + return t; > + /* Don't constant evaluate an unevaluated non-manifestly-constant operand, > + but at least try folding simple expressions to a constant. */ > if (cp_unevaluated_operand && !manifestly_const_eval) > - return t; > + { > + tree r = fold (t); > + return CONSTANT_CLASS_P (r) && !TREE_OVERFLOW (r) ? r : t; These two lines could be factored into a fold_to_constant (inline?) function. OK with that change. > + } > > tree r = cxx_eval_outermost_constant_expr (t, true, true, > mce_value (manifestly_const_eval), > diff --git a/gcc/testsuite/g++.dg/cpp0x/new6.C b/gcc/testsuite/g++.dg/cpp0x/new6.C > new file mode 100644 > index 00000000000..d8f11441423 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/new6.C > @@ -0,0 +1,13 @@ > +// PR c++/108218 > +// { dg-do compile { target c++11 } } > + > +template > +void f() { > + decltype(new int[-1]) p; // { dg-error "negative" } > + decltype(new int[0-1]) q; // { dg-error "negative" } > + decltype(new int[1*-1]) r; // { dg-error "negative" } > +} > + > +decltype(new int[-1]) p; // { dg-error "negative" } > +decltype(new int[0-1]) q; // { dg-error "negative" } > +decltype(new int[1*-1]) r; // { dg-error "negative" } > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-new1.C b/gcc/testsuite/g++.dg/cpp2a/concepts-new1.C > new file mode 100644 > index 00000000000..62007205108 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-new1.C > @@ -0,0 +1,13 @@ > +// PR c++/108219 > +// { dg-do compile { target c++20 } } > + > +template > +concept C = requires { new T[1]{{ 42 }}; }; > + > +template > +concept D = requires { new T[2][1]{{{ 42 }}, {{ 42 }}}; }; > + > +struct A { A(int); }; > + > +static_assert(C); > +static_assert(D);