Message ID | 20211215203612.4095222-1-ppalka@redhat.com |
---|---|
State | New |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 6DA133858D28 for <patchwork@sourceware.org>; Wed, 15 Dec 2021 20:36:51 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6DA133858D28 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1639600611; bh=SlsO/OqmDD/RtSzYsReVRWuI3ebhieOV56DliwBoGSA=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=H9TuWphijbFGIO2okF0WJ0X4xQel0dDmKJ/PSs229JcYTfLk18XZ7AFKxTglPOzmZ 7PpyT7A+Kl2Zw2CPm78nh4vWuJuUizc7CSCm/4stAfuNaBFuGUg3IrNJuQP0wGZ0u7 V3jHUgg//8Nex/PbjqVmfKgdk5n0l//NOA+gz29A= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org 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 350373858D28 for <gcc-patches@gcc.gnu.org>; Wed, 15 Dec 2021 20:36:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 350373858D28 Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-604-1olbcEOYMnCTOrsT013usw-1; Wed, 15 Dec 2021 15:36:21 -0500 X-MC-Unique: 1olbcEOYMnCTOrsT013usw-1 Received: by mail-qk1-f197.google.com with SMTP id bi22-20020a05620a319600b00468606d7e7fso19893904qkb.10 for <gcc-patches@gcc.gnu.org>; Wed, 15 Dec 2021 12:36:21 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=SlsO/OqmDD/RtSzYsReVRWuI3ebhieOV56DliwBoGSA=; b=lTdAy/pZJX8GtbHiMURsoOCmaZz5omK92nUzcAy24WmAR4E6fJOnm4wvxtYp8engOo ylMz4WdZrxxxaR5EjC5Jc/nUKK7DeEJOF4x4S+E6cxJL8k9yfrFsr7MHsmV2uFc0ikeS hFxG65GhLYIT799I3Hv+HMZYHE37Zl2KA3pjpudD7+JDP7djBRTxwLrgl819LZU7sK/L 3A6ogbHvJHjWbQLhJY6UB7n1+fxR85A1IYyaHsH8y46vCXKvRTurf45ZYukrWBk13OPz UKOAIJvKIsPkY3lRCln6XPblIAouCA2ctXXy0S+mwlGbU69lV0xhm4ZW2G1dqi+tI5Pa 6J5w== X-Gm-Message-State: AOAM5337mINNcenBW88iBTGkUVVNG8T/HANadU6vDSq+nCNq9ZhobYqY n1B6th5C/IngjiRSw0pPsCC+5vRkBjVtcymjlGXmat1wnCnO6nTzENGG1PRbUe+ma/fjKuQSRD1 vawP10GUUr3RE756AaGT2Bm9gAjQIUlT+HzOBilBzCcoIrMqo90eqHAB2HkHuMfNKNoY= X-Received: by 2002:ad4:4ea6:: with SMTP id ed6mr12789246qvb.54.1639600581005; Wed, 15 Dec 2021 12:36:21 -0800 (PST) X-Google-Smtp-Source: ABdhPJycWR+SsCOKk3riz/OoPasHO5/FnXdCxi5eHpXpv6BxZUhv5t1+7piEcUr93n+dNgiIR3SWew== X-Received: by 2002:ad4:4ea6:: with SMTP id ed6mr12789222qvb.54.1639600580735; Wed, 15 Dec 2021 12:36:20 -0800 (PST) Received: from localhost.localdomain (ool-457d493a.dyn.optonline.net. [69.125.73.58]) by smtp.gmail.com with ESMTPSA id bj30sm1648109qkb.58.2021.12.15.12.36.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Dec 2021 12:36:20 -0800 (PST) To: gcc-patches@gcc.gnu.org Subject: [PATCH] c++: nested lambda capturing a capture proxy, part 2 [PR94376] Date: Wed, 15 Dec 2021 15:36:12 -0500 Message-Id: <20211215203612.4095222-1-ppalka@redhat.com> X-Mailer: git-send-email 2.34.1.182.ge773545c7f MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" X-Spam-Status: No, score=-16.0 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_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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 <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Patrick Palka via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Patrick Palka <ppalka@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
c++: nested lambda capturing a capture proxy, part 2 [PR94376]
|
|
Commit Message
Patrick Palka
Dec. 15, 2021, 8:36 p.m. UTC
The r12-5403 fix apparently doesn't handle the case where the inner lambda explicitly rather implicitly captures the capture proxy from the outer lambda, and so we still reject the first example in the testcase below. The reason is that compared to an implicit capture, the effective initializer for an explicit capture is wrapped in a location wrapper (pointing to the source location of the explicit capture), and this wrapper foils the is_capture_proxy check. The simplest fix appears to be to strip location wrappers accordingly. Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? PR c++/94376 gcc/cp/ChangeLog: * lambda.c (lambda_capture_field_type): Strip location wrappers before checking for a capture proxy. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/lambda/lambda-nested9a.C: New test. --- gcc/cp/lambda.c | 1 + .../g++.dg/cpp0x/lambda/lambda-nested9a.C | 42 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C
Comments
On 12/15/21 15:36, Patrick Palka wrote: > The r12-5403 fix apparently doesn't handle the case where the inner > lambda explicitly rather implicitly captures the capture proxy from > the outer lambda, and so we still reject the first example in the > testcase below. > > The reason is that compared to an implicit capture, the effective > initializer for an explicit capture is wrapped in a location wrapper > (pointing to the source location of the explicit capture), and this > wrapper foils the is_capture_proxy check. The simplest fix appears to > be to strip location wrappers accordingly. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? > > PR c++/94376 > > gcc/cp/ChangeLog: > > * lambda.c (lambda_capture_field_type): Strip location wrappers > before checking for a capture proxy. I think either is_capture_proxy should strip location wrappers or gcc_checking_assert that it doesn't see one. > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/lambda/lambda-nested9a.C: New test. > --- > gcc/cp/lambda.c | 1 + > .../g++.dg/cpp0x/lambda/lambda-nested9a.C | 42 +++++++++++++++++++ > 2 files changed, 43 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C > > diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c > index c39a2bca416..7f2f927bda2 100644 > --- a/gcc/cp/lambda.c > +++ b/gcc/cp/lambda.c > @@ -221,6 +221,7 @@ lambda_capture_field_type (tree expr, bool explicit_init_p, > } > else > { > + STRIP_ANY_LOCATION_WRAPPER (expr); > if (!by_reference_p && is_capture_proxy (expr)) > { > /* When capturing by-value another capture proxy from an enclosing > diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C > new file mode 100644 > index 00000000000..d62f8f0c952 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C > @@ -0,0 +1,42 @@ > +// PR c++/94376 > +// Like lambda-nested9.C but using explicit captures in the inner lambda. > +// { dg-do compile { target c++11 } } > + > +int main() { > + // We used to incorrectly reject the first two cases. > + int i = 0; > + [=] () { > + [i] () mutable { > + ++i; > + }; > + }; > + > +#if __cpp_init_captures > + [j=0] () { > + [j] () mutable { > + ++j; > + }; > + }; > +#endif > + > + [=] () { > + [&i] () mutable { > + ++i; // { dg-error "read-only" } > + }; > + }; > + > + const int j = 0; > + [=] () { > + [j] () mutable { > + ++j; // { dg-error "read-only" } > + }; > + }; > + > +#if __cpp_init_captures > + [j=0] () { > + [&j] () mutable { > + ++j; // { dg-error "read-only" "" { target c++14 } } > + }; > + }; > +#endif > +}
On Wed, 15 Dec 2021, Jason Merrill wrote: > On 12/15/21 15:36, Patrick Palka wrote: > > The r12-5403 fix apparently doesn't handle the case where the inner > > lambda explicitly rather implicitly captures the capture proxy from > > the outer lambda, and so we still reject the first example in the > > testcase below. > > > > The reason is that compared to an implicit capture, the effective > > initializer for an explicit capture is wrapped in a location wrapper > > (pointing to the source location of the explicit capture), and this > > wrapper foils the is_capture_proxy check. The simplest fix appears to > > be to strip location wrappers accordingly. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > > trunk? > > > > PR c++/94376 > > > > gcc/cp/ChangeLog: > > > > * lambda.c (lambda_capture_field_type): Strip location wrappers > > before checking for a capture proxy. > > I think either is_capture_proxy should strip location wrappers or > gcc_checking_assert that it doesn't see one. Good idea, here's v2 which adds an assert to is_capture_proxy. Only one other caller, mark_const_cap_r, had to be adjusted. -- >8 -- The r12-5403 fix apparently doesn't handle the case where the inner lambda explicitly rather implicitly captures the capture proxy from the outer lambda, which causes us to reject the first example in the testcase below. This is because compared to an implicit capture, the effective initializer for an explicit capture is wrapped in a location wrapper (pointing to the capture list), and this wrapper foils the is_capture_proxy check added in r12-5403. The simplest fix appears to be to strip location wrappers accordingly before checking is_capture_proxy. To help prevent against other occurrences of this kind of bug, this patch also makes is_capture_proxy assert it doesn't see a location wrapper. Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? PR c++/94376 gcc/cp/ChangeLog: * lambda.c (lambda_capture_field_type): Strip location wrappers before checking for a capture proxy. (is_capture_proxy): Assert that we don't see a location wrapper. (mark_const_cap_r): Don't call is_constant_capture_proxy on a location wrapper. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/lambda/lambda-nested9a.C: New test. --- gcc/cp/lambda.c | 7 ++++ .../g++.dg/cpp0x/lambda/lambda-nested9a.C | 42 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c index c39a2bca416..d14e12c48f0 100644 --- a/gcc/cp/lambda.c +++ b/gcc/cp/lambda.c @@ -221,6 +221,7 @@ lambda_capture_field_type (tree expr, bool explicit_init_p, } else { + STRIP_ANY_LOCATION_WRAPPER (expr); if (!by_reference_p && is_capture_proxy (expr)) { /* When capturing by-value another capture proxy from an enclosing @@ -246,6 +247,10 @@ lambda_capture_field_type (tree expr, bool explicit_init_p, bool is_capture_proxy (tree decl) { + /* Location wrappers should be stripped or otherwise handled by the + caller before using this predicate. */ + gcc_checking_assert (!location_wrapper_p (decl)); + return (VAR_P (decl) && DECL_HAS_VALUE_EXPR_P (decl) && !DECL_ANON_UNION_VAR_P (decl) @@ -1496,6 +1501,8 @@ mark_const_cap_r (tree *t, int *walk_subtrees, void *data) *walk_subtrees = 0; } } + else if (location_wrapper_p (*t)) + /* is_capture_proxy dislikes location wrappers. */; else if (is_constant_capture_proxy (*t)) var = DECL_CAPTURED_VARIABLE (*t); diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C new file mode 100644 index 00000000000..d62f8f0c952 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C @@ -0,0 +1,42 @@ +// PR c++/94376 +// Like lambda-nested9.C but using explicit captures in the inner lambda. +// { dg-do compile { target c++11 } } + +int main() { + // We used to incorrectly reject the first two cases. + int i = 0; + [=] () { + [i] () mutable { + ++i; + }; + }; + +#if __cpp_init_captures + [j=0] () { + [j] () mutable { + ++j; + }; + }; +#endif + + [=] () { + [&i] () mutable { + ++i; // { dg-error "read-only" } + }; + }; + + const int j = 0; + [=] () { + [j] () mutable { + ++j; // { dg-error "read-only" } + }; + }; + +#if __cpp_init_captures + [j=0] () { + [&j] () mutable { + ++j; // { dg-error "read-only" "" { target c++14 } } + }; + }; +#endif +}
On 12/16/21 11:28, Patrick Palka wrote: > On Wed, 15 Dec 2021, Jason Merrill wrote: > >> On 12/15/21 15:36, Patrick Palka wrote: >>> The r12-5403 fix apparently doesn't handle the case where the inner >>> lambda explicitly rather implicitly captures the capture proxy from >>> the outer lambda, and so we still reject the first example in the >>> testcase below. >>> >>> The reason is that compared to an implicit capture, the effective >>> initializer for an explicit capture is wrapped in a location wrapper >>> (pointing to the source location of the explicit capture), and this >>> wrapper foils the is_capture_proxy check. The simplest fix appears to >>> be to strip location wrappers accordingly. >>> >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for >>> trunk? >>> >>> PR c++/94376 >>> >>> gcc/cp/ChangeLog: >>> >>> * lambda.c (lambda_capture_field_type): Strip location wrappers >>> before checking for a capture proxy. >> >> I think either is_capture_proxy should strip location wrappers or >> gcc_checking_assert that it doesn't see one. > > Good idea, here's v2 which adds an assert to is_capture_proxy. Only one > other caller, mark_const_cap_r, had to be adjusted. OK. > -- >8 -- > > The r12-5403 fix apparently doesn't handle the case where the inner > lambda explicitly rather implicitly captures the capture proxy from > the outer lambda, which causes us to reject the first example in the > testcase below. > > This is because compared to an implicit capture, the effective initializer > for an explicit capture is wrapped in a location wrapper (pointing to the > capture list), and this wrapper foils the is_capture_proxy check added in > r12-5403. > > The simplest fix appears to be to strip location wrappers accordingly > before checking is_capture_proxy. To help prevent against other > occurrences of this kind of bug, this patch also makes is_capture_proxy > assert it doesn't see a location wrapper. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk? > > PR c++/94376 > > gcc/cp/ChangeLog: > > * lambda.c (lambda_capture_field_type): Strip location wrappers > before checking for a capture proxy. > (is_capture_proxy): Assert that we don't see a location wrapper. > (mark_const_cap_r): Don't call is_constant_capture_proxy on a > location wrapper. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/lambda/lambda-nested9a.C: New test. > --- > gcc/cp/lambda.c | 7 ++++ > .../g++.dg/cpp0x/lambda/lambda-nested9a.C | 42 +++++++++++++++++++ > 2 files changed, 49 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C > > diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c > index c39a2bca416..d14e12c48f0 100644 > --- a/gcc/cp/lambda.c > +++ b/gcc/cp/lambda.c > @@ -221,6 +221,7 @@ lambda_capture_field_type (tree expr, bool explicit_init_p, > } > else > { > + STRIP_ANY_LOCATION_WRAPPER (expr); > if (!by_reference_p && is_capture_proxy (expr)) > { > /* When capturing by-value another capture proxy from an enclosing > @@ -246,6 +247,10 @@ lambda_capture_field_type (tree expr, bool explicit_init_p, > bool > is_capture_proxy (tree decl) > { > + /* Location wrappers should be stripped or otherwise handled by the > + caller before using this predicate. */ > + gcc_checking_assert (!location_wrapper_p (decl)); > + > return (VAR_P (decl) > && DECL_HAS_VALUE_EXPR_P (decl) > && !DECL_ANON_UNION_VAR_P (decl) > @@ -1496,6 +1501,8 @@ mark_const_cap_r (tree *t, int *walk_subtrees, void *data) > *walk_subtrees = 0; > } > } > + else if (location_wrapper_p (*t)) > + /* is_capture_proxy dislikes location wrappers. */; > else if (is_constant_capture_proxy (*t)) > var = DECL_CAPTURED_VARIABLE (*t); > > diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C > new file mode 100644 > index 00000000000..d62f8f0c952 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C > @@ -0,0 +1,42 @@ > +// PR c++/94376 > +// Like lambda-nested9.C but using explicit captures in the inner lambda. > +// { dg-do compile { target c++11 } } > + > +int main() { > + // We used to incorrectly reject the first two cases. > + int i = 0; > + [=] () { > + [i] () mutable { > + ++i; > + }; > + }; > + > +#if __cpp_init_captures > + [j=0] () { > + [j] () mutable { > + ++j; > + }; > + }; > +#endif > + > + [=] () { > + [&i] () mutable { > + ++i; // { dg-error "read-only" } > + }; > + }; > + > + const int j = 0; > + [=] () { > + [j] () mutable { > + ++j; // { dg-error "read-only" } > + }; > + }; > + > +#if __cpp_init_captures > + [j=0] () { > + [&j] () mutable { > + ++j; // { dg-error "read-only" "" { target c++14 } } > + }; > + }; > +#endif > +}
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c index c39a2bca416..7f2f927bda2 100644 --- a/gcc/cp/lambda.c +++ b/gcc/cp/lambda.c @@ -221,6 +221,7 @@ lambda_capture_field_type (tree expr, bool explicit_init_p, } else { + STRIP_ANY_LOCATION_WRAPPER (expr); if (!by_reference_p && is_capture_proxy (expr)) { /* When capturing by-value another capture proxy from an enclosing diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C new file mode 100644 index 00000000000..d62f8f0c952 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9a.C @@ -0,0 +1,42 @@ +// PR c++/94376 +// Like lambda-nested9.C but using explicit captures in the inner lambda. +// { dg-do compile { target c++11 } } + +int main() { + // We used to incorrectly reject the first two cases. + int i = 0; + [=] () { + [i] () mutable { + ++i; + }; + }; + +#if __cpp_init_captures + [j=0] () { + [j] () mutable { + ++j; + }; + }; +#endif + + [=] () { + [&i] () mutable { + ++i; // { dg-error "read-only" } + }; + }; + + const int j = 0; + [=] () { + [j] () mutable { + ++j; // { dg-error "read-only" } + }; + }; + +#if __cpp_init_captures + [j=0] () { + [&j] () mutable { + ++j; // { dg-error "read-only" "" { target c++14 } } + }; + }; +#endif +}