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 [216.145.221.124]) by sourceware.org (Postfix) with ESMTPS id DBFE53858D33 for ; Tue, 25 Apr 2023 14:55:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DBFE53858D33 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1682434553; 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=jbDmQVXnV30UQXnLOSgdyAFz7ijoBl5KiUWFe6dj1H0=; b=RVmdPF/6kSM3/RDjq8aPEGd5xxBsThtuYeFO6fWbMc89upa/bDCsI087RzOJw6K6s0s0YN ntAamRYdKTXFe1LcD9R7xNTjixgmtCtS3g33elKMWqeL4HZSYvG8NHpE/GRNTWsifmPsqj LYRllKe7FPM0paFDcF4BMn4UX+gp8KQ= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-2-s7I0ptkvN0iF08t7rBUvhA-1; Tue, 25 Apr 2023 10:55:51 -0400 X-MC-Unique: s7I0ptkvN0iF08t7rBUvhA-1 Received: by mail-qv1-f71.google.com with SMTP id 6a1803df08f44-5e7223aab07so35817886d6.0 for ; Tue, 25 Apr 2023 07:55:51 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682434550; x=1685026550; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=jbDmQVXnV30UQXnLOSgdyAFz7ijoBl5KiUWFe6dj1H0=; b=HxJS3nds8+kWYVSVGD9V3CWDNORiqFXghQMYJltXu0O4z5mXirRqze8CUG/TRF7aGM HCC/LeGg4ONoyA6k+8wovrhqcIe0Ajdr1JhvhhnZxAlEFQzk73hCrdSWZxGl6c5UqFg0 1kyjg1ilMRXtmer2oYNkDiVpayon4bQ6JEGZfQMkWKGb4tLTqIOD+oZuD+238igKgieq 3NWCfuTX//l+jNW4v+JMdpx9knZqb9QRZ4IZu+8CKpmaSGcfSncg2bfP4TDyrf4NU6ud 4WWirg49/+72KvZuqtzHE1Y+IqmVTsJUlFuUYERimqJIZZbLyrCamXaUNRj3ybwgHaRg Kvgw== X-Gm-Message-State: AAQBX9dK6zEtg606t6QNQ3wDw7i0JO6atEBvoym2xDhqJAcys2I8qmzN d2prG3UilomW8N25np/f1sNtZ1zcY8J4E4tmM6VfGJbFuyN/KsvCRKscoRn2j/HkEJ2bJrNwZcq +NJK0hac4wAR7iF+oBg== X-Received: by 2002:ad4:5f4c:0:b0:5a9:c758:ba0e with SMTP id p12-20020ad45f4c000000b005a9c758ba0emr34166333qvg.16.1682434550491; Tue, 25 Apr 2023 07:55:50 -0700 (PDT) X-Google-Smtp-Source: AKy350YMb41aqKL4mNv18Z0y9KSvlrAILkj2Yn86Hii8MHbcZ354Xwb6Ohjk0egBqdafqi4arfU2ug== X-Received: by 2002:ad4:5f4c:0:b0:5a9:c758:ba0e with SMTP id p12-20020ad45f4c000000b005a9c758ba0emr34166294qvg.16.1682434550079; Tue, 25 Apr 2023 07:55:50 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id m4-20020a05620a214400b0074d1d3b2143sm4375713qkm.118.2023.04.25.07.55.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Apr 2023 07:55:49 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Tue, 25 Apr 2023 10:55:49 -0400 (EDT) To: Patrick Palka cc: Jason Merrill , gcc-patches@gcc.gnu.org Subject: Re: [pushed] c++: constant, array, lambda, template [PR108975] In-Reply-To: <162739c5-32da-bc17-bae5-47df5d89be59@idea> Message-ID: <7f027e55-1066-7e41-9158-e4d0189cbd83@idea> References: <20230317233249.1406928-1-jason@redhat.com> <162739c5-32da-bc17-bae5-47df5d89be59@idea> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-13.9 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,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE 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 Sat, 18 Mar 2023, Patrick Palka wrote: > On Fri, 17 Mar 2023, Jason Merrill via Gcc-patches wrote: > > > Tested x86_64-pc-linux-gnu, applying to trunk. > > > > -- 8< -- > > > > When a lambda refers to a constant local variable in the enclosing scope, we > > tentatively capture it, but if we end up pulling out its constant value, we > > go back at the end of the lambda and prune any unneeded captures. Here > > while parsing the template we decided that the dim capture was unneeded, > > because we folded it away, but then we brought back the use in the template > > trees that try to preserve the source representation with added type info. > > So then when we tried to instantiate that use, we couldn't find what it was > > trying to use, and crashed. > > > > Fixed by not trying to prune when parsing a template; we'll prune at > > instantiation time. > > > > PR c++/108975 > > > > gcc/cp/ChangeLog: > > > > * lambda.cc (prune_lambda_captures): Don't bother in a template. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp0x/lambda/lambda-const11.C: New test. > > --- > > gcc/cp/lambda.cc | 3 +++ > > gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const11.C | 14 ++++++++++++++ > > 2 files changed, 17 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const11.C > > > > diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc > > index 212990a21bf..9925209b2ed 100644 > > --- a/gcc/cp/lambda.cc > > +++ b/gcc/cp/lambda.cc > > @@ -1760,6 +1760,9 @@ prune_lambda_captures (tree body) > > if (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lam) == CPLD_NONE) > > /* No default captures, and we don't prune explicit captures. */ > > return; > > + /* Don't bother pruning in a template, we'll prune at instantiation time. */ > > + if (dependent_type_p (TREE_TYPE (lam))) > > + return; > > > > hash_map const_vars; > > > > diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const11.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const11.C > > new file mode 100644 > > index 00000000000..26af75bf132 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const11.C > > @@ -0,0 +1,14 @@ > > +// PR c++/108975 > > +// { dg-do compile { target c++11 } } > > + > > +template > > +void f() { > > + constexpr int dim = 1; > > + auto l = [&] { > > + int n[dim * 1]; > > + }; > > + // In f, we shouldn't actually capture dim. > > + static_assert (sizeof(l) == 1, ""); > > +} > > + > > +template void f(); > > Sadly I think more is needed to fix the generic lambda case, we still > crash for: > > template > void f() { > constexpr int dim = 1; > auto l = [&] (auto) { > int n[dim * 1]; > }; > l(0); > // In f, we shouldn't actually capture dim. > static_assert (sizeof(l) == 1, ""); > } > > template void f(); > > It seems prune_lambda_captures doesn't thoroughly walk the lambda body, > and so fails to find all uses of constant captures such as in 'int n[dim * 1]' > or 'using type = decltype(g())' (in the original testcase the > constant capture use occurred inside an alias declaration). I was curious why we ICE only with a by-ref capture and not with a by-value capture, and it turns out the difference is that we consider a by-ref capture of the constant variable 'dim' to be value-dependent, which prevents us from constant folding its uses ahead of time. So another approach to fixing the generic lambda version of this testcase would be to make value_dependent_expression_p treat by-value and by-ref captures of constant variables more similarly. What do you think of the following? -- >8 -- Subject: [PATCH] c++: value dependence of by-ref lambda capture [PR108975] We are still ICEing in the generic lambda version of the testcase from this PR even after r13-6743-g6f90de97634d6f due to the by-ref capture of the constant local variable 'dim' being considered value-dependent when regenerating the lambda (at which point processing_template_decl is set since the lambda is generic), which prevents us from constant folding its uses. Later when calling prune_lambda_captures, we end up not thoroughly walking the body of the lambda and miss the non-folded uses of 'dim' within the array bounds and using-decl, and we prematurely prune the capture. We could fix this by making prune_lambda_captures walk the body of the lambda more thoroughly so that it finds these non-folded uses of 'dim', but ideally we should be able to constant fold all uses of 'dim' ahead of time and prune the implicit capture after all. To that end this patch makes value_dependent_expression_p consistently return false for such captures of constant local variables even if they are by-ref, allowing their uses to get constant folded ahead of time. It seems we just need to relax the conservative early exit for reference variables (added by r5-5022-g51d72abe5ea04e) when DECL_HAS_VALUE_EXPR_P. Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? PR c++/108975 gcc/cp/ChangeLog: * pt.cc (value_dependent_expression_p) : Suppress conservative early exit for reference variables when DECL_HAS_VALUE_EXPR_P. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/lambda/lambda-const11a.C: New test. --- gcc/cp/pt.cc | 7 ++++--- .../g++.dg/cpp0x/lambda/lambda-const11a.C | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const11a.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 3e5f0104056..a668825b5f9 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -27959,9 +27959,7 @@ value_dependent_expression_p (tree expression) case VAR_DECL: /* A constant with literal type and is initialized with an expression that is value-dependent. */ - if (DECL_DEPENDENT_INIT_P (expression) - /* FIXME cp_finish_decl doesn't fold reference initializers. */ - || TYPE_REF_P (TREE_TYPE (expression))) + if (DECL_DEPENDENT_INIT_P (expression)) return true; if (DECL_HAS_VALUE_EXPR_P (expression)) { @@ -27976,6 +27974,9 @@ value_dependent_expression_p (tree expression) && value_expr == error_mark_node)) return true; } + else if (TYPE_REF_P (TREE_TYPE (expression))) + /* FIXME cp_finish_decl doesn't fold reference initializers. */ + return true; return false; case DYNAMIC_CAST_EXPR: diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const11a.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const11a.C new file mode 100644 index 00000000000..d3a6de8a138 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-const11a.C @@ -0,0 +1,21 @@ +// PR c++/108975 +// A version of lambda-const11.C using a generic lambda. +// { dg-do compile { target c++11 } } + +template void g(); +template struct A { }; + +template +void f() { + constexpr int dim = 1; + auto f = [&](auto) { + int n[dim * 1]; + using ty1 = decltype(g()); + using ty2 = A; + }; + // In f, we shouldn't actually capture dim. + static_assert (sizeof(f) == 1, ""); + f(0); +} + +template void f(); -- 2.40.0.374.g7580f92ffa