* [PATCH] c++: ICE with redundant capture [PR108829]
@ 2023-02-17 18:58 Marek Polacek
2023-02-17 20:00 ` Patrick Palka
0 siblings, 1 reply; 7+ messages in thread
From: Marek Polacek @ 2023-02-17 18:58 UTC (permalink / raw)
To: Jason Merrill, GCC Patches
Here we crash in is_capture_proxy:
/* Location wrappers should be stripped or otherwise handled by the
caller before using this predicate. */
gcc_checking_assert (!location_wrapper_p (decl));
so fixed as the comment suggests. We only crash with the redundant
capture:
int abyPage = [=, abyPage] { ... }
because prune_lambda_captures is only called when there was a default
capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/12?
PR c++/108829
gcc/cp/ChangeLog:
* lambda.cc (var_to_maybe_prune): Strip location wrappers before
checking for a capture proxy.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/lambda/lambda-108829.C: New test.
---
gcc/cp/lambda.cc | 1 +
gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C | 11 +++++++++++
2 files changed, 12 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C
diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
index c752622816d..a12b9c183c2 100644
--- a/gcc/cp/lambda.cc
+++ b/gcc/cp/lambda.cc
@@ -1700,6 +1700,7 @@ var_to_maybe_prune (tree cap)
return NULL_TREE;
tree init = TREE_VALUE (cap);
+ STRIP_ANY_LOCATION_WRAPPER (init);
if (is_normal_capture_proxy (init))
init = DECL_CAPTURED_VARIABLE (init);
if (decl_constant_var_p (init))
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C
new file mode 100644
index 00000000000..e621a0d14d0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C
@@ -0,0 +1,11 @@
+// PR c++/108829
+// { dg-do compile { target c++11 } }
+
+template <int>
+void f(void) {
+ constexpr int IDX_PAGE_SIZE = 4096;
+ int abyPage = [=, abyPage] { return IDX_PAGE_SIZE; }(); // { dg-error "redundant" }
+}
+void h() {
+ f<1>();
+}
base-commit: 6245441e124846d0c3551f312d2feef598fe251c
--
2.39.2
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] c++: ICE with redundant capture [PR108829] 2023-02-17 18:58 [PATCH] c++: ICE with redundant capture [PR108829] Marek Polacek @ 2023-02-17 20:00 ` Patrick Palka 2023-02-17 21:06 ` [PATCH v2] " Marek Polacek 0 siblings, 1 reply; 7+ messages in thread From: Patrick Palka @ 2023-02-17 20:00 UTC (permalink / raw) To: Marek Polacek; +Cc: Jason Merrill, GCC Patches On Fri, 17 Feb 2023, Marek Polacek via Gcc-patches wrote: > Here we crash in is_capture_proxy: > > /* Location wrappers should be stripped or otherwise handled by the > caller before using this predicate. */ > gcc_checking_assert (!location_wrapper_p (decl)); > > so fixed as the comment suggests. We only crash with the redundant > capture: > > int abyPage = [=, abyPage] { ... } > > because prune_lambda_captures is only called when there was a default > capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. It's weird that we even get this far in var_to_maybe_prune. Shouldn't LAMBDA_CAPTURE_EXPLICIT_P be true for abyPage? > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/12? > > PR c++/108829 > > gcc/cp/ChangeLog: > > * lambda.cc (var_to_maybe_prune): Strip location wrappers before > checking for a capture proxy. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/lambda/lambda-108829.C: New test. > --- > gcc/cp/lambda.cc | 1 + > gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C | 11 +++++++++++ > 2 files changed, 12 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > > diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc > index c752622816d..a12b9c183c2 100644 > --- a/gcc/cp/lambda.cc > +++ b/gcc/cp/lambda.cc > @@ -1700,6 +1700,7 @@ var_to_maybe_prune (tree cap) > return NULL_TREE; > > tree init = TREE_VALUE (cap); > + STRIP_ANY_LOCATION_WRAPPER (init); > if (is_normal_capture_proxy (init)) > init = DECL_CAPTURED_VARIABLE (init); > if (decl_constant_var_p (init)) > diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > new file mode 100644 > index 00000000000..e621a0d14d0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > @@ -0,0 +1,11 @@ > +// PR c++/108829 > +// { dg-do compile { target c++11 } } > + > +template <int> > +void f(void) { > + constexpr int IDX_PAGE_SIZE = 4096; > + int abyPage = [=, abyPage] { return IDX_PAGE_SIZE; }(); // { dg-error "redundant" } > +} > +void h() { > + f<1>(); > +} > > base-commit: 6245441e124846d0c3551f312d2feef598fe251c > -- > 2.39.2 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] c++: ICE with redundant capture [PR108829] 2023-02-17 20:00 ` Patrick Palka @ 2023-02-17 21:06 ` Marek Polacek 2023-02-17 21:27 ` Patrick Palka 0 siblings, 1 reply; 7+ messages in thread From: Marek Polacek @ 2023-02-17 21:06 UTC (permalink / raw) To: Patrick Palka; +Cc: Jason Merrill, GCC Patches On Fri, Feb 17, 2023 at 03:00:39PM -0500, Patrick Palka wrote: > On Fri, 17 Feb 2023, Marek Polacek via Gcc-patches wrote: > > > Here we crash in is_capture_proxy: > > > > /* Location wrappers should be stripped or otherwise handled by the > > caller before using this predicate. */ > > gcc_checking_assert (!location_wrapper_p (decl)); > > > > so fixed as the comment suggests. We only crash with the redundant > > capture: > > > > int abyPage = [=, abyPage] { ... } > > > > because prune_lambda_captures is only called when there was a default > > capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. > > It's weird that we even get this far in var_to_maybe_prune. Shouldn't > LAMBDA_CAPTURE_EXPLICIT_P be true for abyPage? Ug, I was seduced by the ostensible obviousness and failed to notice that check. In that light, the correct fix ought to be this. Thanks! Bootstrap/regtest running on x86_64-pc-linux-gnu, ok for trunk if it passes? -- >8 -- Here we crash in is_capture_proxy: /* Location wrappers should be stripped or otherwise handled by the caller before using this predicate. */ gcc_checking_assert (!location_wrapper_p (decl)); We only crash with the redundant capture: int abyPage = [=, abyPage] { ... } because prune_lambda_captures is only called when there was a default capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. The problem is that LAMBDA_CAPTURE_EXPLICIT_P wasn't propagated correctly and so var_to_maybe_prune proceeded where it shouldn't. PR c++/108829 gcc/cp/ChangeLog: * pt.cc (tsubst_lambda_expr): Propagate LAMBDA_CAPTURE_EXPLICIT_P. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/lambda/lambda-108829.C: New test. --- gcc/cp/pt.cc | 4 ++++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C | 11 +++++++++++ 2 files changed, 15 insertions(+) create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index b1ac7d4beb4..f747ce877b5 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -19992,6 +19992,10 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) if (id_equal (DECL_NAME (field), "__this")) LAMBDA_EXPR_THIS_CAPTURE (r) = field; } + + if (LAMBDA_EXPR_CAPTURE_LIST (r)) + LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (r)) + = LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (t)); } tree type = begin_lambda_type (r); diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C new file mode 100644 index 00000000000..e621a0d14d0 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C @@ -0,0 +1,11 @@ +// PR c++/108829 +// { dg-do compile { target c++11 } } + +template <int> +void f(void) { + constexpr int IDX_PAGE_SIZE = 4096; + int abyPage = [=, abyPage] { return IDX_PAGE_SIZE; }(); // { dg-error "redundant" } +} +void h() { + f<1>(); +} base-commit: 5fea1be820508e1fbc610d1a54b61c1add33c36f -- 2.39.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] c++: ICE with redundant capture [PR108829] 2023-02-17 21:06 ` [PATCH v2] " Marek Polacek @ 2023-02-17 21:27 ` Patrick Palka 2023-02-17 21:32 ` Patrick Palka 0 siblings, 1 reply; 7+ messages in thread From: Patrick Palka @ 2023-02-17 21:27 UTC (permalink / raw) To: Marek Polacek; +Cc: Patrick Palka, Jason Merrill, GCC Patches On Fri, 17 Feb 2023, Marek Polacek wrote: > On Fri, Feb 17, 2023 at 03:00:39PM -0500, Patrick Palka wrote: > > On Fri, 17 Feb 2023, Marek Polacek via Gcc-patches wrote: > > > > > Here we crash in is_capture_proxy: > > > > > > /* Location wrappers should be stripped or otherwise handled by the > > > caller before using this predicate. */ > > > gcc_checking_assert (!location_wrapper_p (decl)); > > > > > > so fixed as the comment suggests. We only crash with the redundant > > > capture: > > > > > > int abyPage = [=, abyPage] { ... } > > > > > > because prune_lambda_captures is only called when there was a default > > > capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. > > > > It's weird that we even get this far in var_to_maybe_prune. Shouldn't > > LAMBDA_CAPTURE_EXPLICIT_P be true for abyPage? > > Ug, I was seduced by the ostensible obviousness and failed to notice > that check. In that light, the correct fix ought to be this. Thanks! > > Bootstrap/regtest running on x86_64-pc-linux-gnu, ok for trunk if it > passes? > > -- >8 -- > Here we crash in is_capture_proxy: > > /* Location wrappers should be stripped or otherwise handled by the > caller before using this predicate. */ > gcc_checking_assert (!location_wrapper_p (decl)); > > We only crash with the redundant capture: > > int abyPage = [=, abyPage] { ... } > > because prune_lambda_captures is only called when there was a default > capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. > > The problem is that LAMBDA_CAPTURE_EXPLICIT_P wasn't propagated > correctly and so var_to_maybe_prune proceeded where it shouldn't. > > PR c++/108829 > > gcc/cp/ChangeLog: > > * pt.cc (tsubst_lambda_expr): Propagate LAMBDA_CAPTURE_EXPLICIT_P. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/lambda/lambda-108829.C: New test. > --- > gcc/cp/pt.cc | 4 ++++ > gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C | 11 +++++++++++ > 2 files changed, 15 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index b1ac7d4beb4..f747ce877b5 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -19992,6 +19992,10 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) > if (id_equal (DECL_NAME (field), "__this")) > LAMBDA_EXPR_THIS_CAPTURE (r) = field; > } > + > + if (LAMBDA_EXPR_CAPTURE_LIST (r)) > + LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (r)) > + = LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (t)); I'm not sure how the flag works for pack captures but it looks like this would only propagate the flag to the last expanded capture if the capture was originally a pack. > } > > tree type = begin_lambda_type (r); > diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > new file mode 100644 > index 00000000000..e621a0d14d0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > @@ -0,0 +1,11 @@ > +// PR c++/108829 > +// { dg-do compile { target c++11 } } > + > +template <int> > +void f(void) { > + constexpr int IDX_PAGE_SIZE = 4096; > + int abyPage = [=, abyPage] { return IDX_PAGE_SIZE; }(); // { dg-error "redundant" } > +} > +void h() { > + f<1>(); > +} > > base-commit: 5fea1be820508e1fbc610d1a54b61c1add33c36f > -- > 2.39.2 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] c++: ICE with redundant capture [PR108829] 2023-02-17 21:27 ` Patrick Palka @ 2023-02-17 21:32 ` Patrick Palka 2023-02-17 22:42 ` [PATCH v3] " Marek Polacek 0 siblings, 1 reply; 7+ messages in thread From: Patrick Palka @ 2023-02-17 21:32 UTC (permalink / raw) To: Patrick Palka; +Cc: Marek Polacek, Jason Merrill, GCC Patches On Fri, 17 Feb 2023, Patrick Palka wrote: > On Fri, 17 Feb 2023, Marek Polacek wrote: > > > On Fri, Feb 17, 2023 at 03:00:39PM -0500, Patrick Palka wrote: > > > On Fri, 17 Feb 2023, Marek Polacek via Gcc-patches wrote: > > > > > > > Here we crash in is_capture_proxy: > > > > > > > > /* Location wrappers should be stripped or otherwise handled by the > > > > caller before using this predicate. */ > > > > gcc_checking_assert (!location_wrapper_p (decl)); > > > > > > > > so fixed as the comment suggests. We only crash with the redundant > > > > capture: > > > > > > > > int abyPage = [=, abyPage] { ... } > > > > > > > > because prune_lambda_captures is only called when there was a default > > > > capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. > > > > > > It's weird that we even get this far in var_to_maybe_prune. Shouldn't > > > LAMBDA_CAPTURE_EXPLICIT_P be true for abyPage? > > > > Ug, I was seduced by the ostensible obviousness and failed to notice > > that check. In that light, the correct fix ought to be this. Thanks! > > > > Bootstrap/regtest running on x86_64-pc-linux-gnu, ok for trunk if it > > passes? > > > > -- >8 -- > > Here we crash in is_capture_proxy: > > > > /* Location wrappers should be stripped or otherwise handled by the > > caller before using this predicate. */ > > gcc_checking_assert (!location_wrapper_p (decl)); > > > > We only crash with the redundant capture: > > > > int abyPage = [=, abyPage] { ... } > > > > because prune_lambda_captures is only called when there was a default > > capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. > > > > The problem is that LAMBDA_CAPTURE_EXPLICIT_P wasn't propagated > > correctly and so var_to_maybe_prune proceeded where it shouldn't. > > > > PR c++/108829 > > > > gcc/cp/ChangeLog: > > > > * pt.cc (tsubst_lambda_expr): Propagate LAMBDA_CAPTURE_EXPLICIT_P. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp0x/lambda/lambda-108829.C: New test. > > --- > > gcc/cp/pt.cc | 4 ++++ > > gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C | 11 +++++++++++ > > 2 files changed, 15 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index b1ac7d4beb4..f747ce877b5 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -19992,6 +19992,10 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) > > if (id_equal (DECL_NAME (field), "__this")) > > LAMBDA_EXPR_THIS_CAPTURE (r) = field; > > } > > + > > + if (LAMBDA_EXPR_CAPTURE_LIST (r)) > > + LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (r)) > > + = LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (t)); > > I'm not sure how the flag works for pack captures but it looks like > this would only propagate the flag to the last expanded capture if > the capture was originally a pack. Testcase: template<int, class... Ts> void f(Ts... ts) { constexpr int IDX_PAGE_SIZE = 4096; int abyPage = [=, ts...] { return IDX_PAGE_SIZE; }(); } void h() { f<1>(0, 1); } > > > } > > > > tree type = begin_lambda_type (r); > > diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > > new file mode 100644 > > index 00000000000..e621a0d14d0 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > > @@ -0,0 +1,11 @@ > > +// PR c++/108829 > > +// { dg-do compile { target c++11 } } > > + > > +template <int> > > +void f(void) { > > + constexpr int IDX_PAGE_SIZE = 4096; > > + int abyPage = [=, abyPage] { return IDX_PAGE_SIZE; }(); // { dg-error "redundant" } > > +} > > +void h() { > > + f<1>(); > > +} > > > > base-commit: 5fea1be820508e1fbc610d1a54b61c1add33c36f > > -- > > 2.39.2 > > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3] c++: ICE with redundant capture [PR108829] 2023-02-17 21:32 ` Patrick Palka @ 2023-02-17 22:42 ` Marek Polacek 2023-02-20 2:43 ` Jason Merrill 0 siblings, 1 reply; 7+ messages in thread From: Marek Polacek @ 2023-02-17 22:42 UTC (permalink / raw) To: Patrick Palka; +Cc: Jason Merrill, GCC Patches On Fri, Feb 17, 2023 at 04:32:50PM -0500, Patrick Palka wrote: > On Fri, 17 Feb 2023, Patrick Palka wrote: > > > On Fri, 17 Feb 2023, Marek Polacek wrote: > > > > > On Fri, Feb 17, 2023 at 03:00:39PM -0500, Patrick Palka wrote: > > > > On Fri, 17 Feb 2023, Marek Polacek via Gcc-patches wrote: > > > > > > > > > Here we crash in is_capture_proxy: > > > > > > > > > > /* Location wrappers should be stripped or otherwise handled by the > > > > > caller before using this predicate. */ > > > > > gcc_checking_assert (!location_wrapper_p (decl)); > > > > > > > > > > so fixed as the comment suggests. We only crash with the redundant > > > > > capture: > > > > > > > > > > int abyPage = [=, abyPage] { ... } > > > > > > > > > > because prune_lambda_captures is only called when there was a default > > > > > capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. > > > > > > > > It's weird that we even get this far in var_to_maybe_prune. Shouldn't > > > > LAMBDA_CAPTURE_EXPLICIT_P be true for abyPage? > > > > > > Ug, I was seduced by the ostensible obviousness and failed to notice > > > that check. In that light, the correct fix ought to be this. Thanks! > > > > > > Bootstrap/regtest running on x86_64-pc-linux-gnu, ok for trunk if it > > > passes? > > > > > > -- >8 -- > > > Here we crash in is_capture_proxy: > > > > > > /* Location wrappers should be stripped or otherwise handled by the > > > caller before using this predicate. */ > > > gcc_checking_assert (!location_wrapper_p (decl)); > > > > > > We only crash with the redundant capture: > > > > > > int abyPage = [=, abyPage] { ... } > > > > > > because prune_lambda_captures is only called when there was a default > > > capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. > > > > > > The problem is that LAMBDA_CAPTURE_EXPLICIT_P wasn't propagated > > > correctly and so var_to_maybe_prune proceeded where it shouldn't. > > > > > > PR c++/108829 > > > > > > gcc/cp/ChangeLog: > > > > > > * pt.cc (tsubst_lambda_expr): Propagate LAMBDA_CAPTURE_EXPLICIT_P. > > > > > > gcc/testsuite/ChangeLog: > > > > > > * g++.dg/cpp0x/lambda/lambda-108829.C: New test. > > > --- > > > gcc/cp/pt.cc | 4 ++++ > > > gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C | 11 +++++++++++ > > > 2 files changed, 15 insertions(+) > > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > > > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > > index b1ac7d4beb4..f747ce877b5 100644 > > > --- a/gcc/cp/pt.cc > > > +++ b/gcc/cp/pt.cc > > > @@ -19992,6 +19992,10 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) > > > if (id_equal (DECL_NAME (field), "__this")) > > > LAMBDA_EXPR_THIS_CAPTURE (r) = field; > > > } > > > + > > > + if (LAMBDA_EXPR_CAPTURE_LIST (r)) > > > + LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (r)) > > > + = LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (t)); > > > > I'm not sure how the flag works for pack captures but it looks like > > this would only propagate the flag to the last expanded capture if > > the capture was originally a pack. > > Testcase: > > template<int, class... Ts> > void f(Ts... ts) { > constexpr int IDX_PAGE_SIZE = 4096; > int abyPage = [=, ts...] { return IDX_PAGE_SIZE; }(); > } > void h() { > f<1>(0, 1); > } Thanks a lot for the testacase. Revised patch below. Look OK? Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- Here we crash in is_capture_proxy: /* Location wrappers should be stripped or otherwise handled by the caller before using this predicate. */ gcc_checking_assert (!location_wrapper_p (decl)); We only crash with the redundant capture: int abyPage = [=, abyPage] { ... } because prune_lambda_captures is only called when there was a default capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. The problem is that LAMBDA_CAPTURE_EXPLICIT_P wasn't propagated correctly and so var_to_maybe_prune proceeded where it shouldn't. PR c++/108829 gcc/cp/ChangeLog: * pt.cc (prepend_one_capture): Set LAMBDA_CAPTURE_EXPLICIT_P. (tsubst_lambda_expr): Pass LAMBDA_CAPTURE_EXPLICIT_P to prepend_one_capture. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/lambda/lambda-108829-2.C: New test. * g++.dg/cpp0x/lambda/lambda-108829.C: New test. Co-Authored by: Patrick Palka <ppalka@redhat.com> --- gcc/cp/pt.cc | 9 ++++++--- gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C | 11 +++++++++++ gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C | 11 +++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index b1ac7d4beb4..1a071e95004 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -19870,10 +19870,11 @@ tsubst_non_call_postfix_expression (tree t, tree args, /* Subroutine of tsubst_lambda_expr: add the FIELD/INIT capture pair to the LAMBDA_EXPR_CAPTURE_LIST passed in LIST. Do deduction for a previously - dependent init-capture. */ + dependent init-capture. EXPLICIT_P is true if the original list had + explicit captures. */ static void -prepend_one_capture (tree field, tree init, tree &list, +prepend_one_capture (tree field, tree init, tree &list, bool explicit_p, tsubst_flags_t complain) { if (tree auto_node = type_uses_auto (TREE_TYPE (field))) @@ -19893,6 +19894,7 @@ prepend_one_capture (tree field, tree init, tree &list, cp_apply_type_quals_to_decl (cp_type_quals (type), field); } list = tree_cons (field, init, list); + LAMBDA_CAPTURE_EXPLICIT_P (list) = explicit_p; } /* T is a LAMBDA_EXPR. Generate a new LAMBDA_EXPR for the current @@ -19982,12 +19984,13 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) prepend_one_capture (TREE_VEC_ELT (field, i), TREE_VEC_ELT (init, i), LAMBDA_EXPR_CAPTURE_LIST (r), + LAMBDA_CAPTURE_EXPLICIT_P (cap), complain); } else { prepend_one_capture (field, init, LAMBDA_EXPR_CAPTURE_LIST (r), - complain); + LAMBDA_CAPTURE_EXPLICIT_P (cap), complain); if (id_equal (DECL_NAME (field), "__this")) LAMBDA_EXPR_THIS_CAPTURE (r) = field; diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C new file mode 100644 index 00000000000..4e24470514d --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C @@ -0,0 +1,11 @@ +// PR c++/108829 +// { dg-do compile { target c++11 } } + +template<int, class... Ts> +void f(Ts... ts) { + constexpr int IDX_PAGE_SIZE = 4096; + int abyPage = [=, ts...] { return IDX_PAGE_SIZE; }(); // { dg-error "redundant" } +} +void h() { + f<1>(0, 1); +} diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C new file mode 100644 index 00000000000..e621a0d14d0 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C @@ -0,0 +1,11 @@ +// PR c++/108829 +// { dg-do compile { target c++11 } } + +template <int> +void f(void) { + constexpr int IDX_PAGE_SIZE = 4096; + int abyPage = [=, abyPage] { return IDX_PAGE_SIZE; }(); // { dg-error "redundant" } +} +void h() { + f<1>(); +} base-commit: 5fea1be820508e1fbc610d1a54b61c1add33c36f -- 2.39.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] c++: ICE with redundant capture [PR108829] 2023-02-17 22:42 ` [PATCH v3] " Marek Polacek @ 2023-02-20 2:43 ` Jason Merrill 0 siblings, 0 replies; 7+ messages in thread From: Jason Merrill @ 2023-02-20 2:43 UTC (permalink / raw) To: Marek Polacek, Patrick Palka; +Cc: GCC Patches On 2/17/23 14:42, Marek Polacek wrote: > On Fri, Feb 17, 2023 at 04:32:50PM -0500, Patrick Palka wrote: >> On Fri, 17 Feb 2023, Patrick Palka wrote: >> >>> On Fri, 17 Feb 2023, Marek Polacek wrote: >>> >>>> On Fri, Feb 17, 2023 at 03:00:39PM -0500, Patrick Palka wrote: >>>>> On Fri, 17 Feb 2023, Marek Polacek via Gcc-patches wrote: >>>>> >>>>>> Here we crash in is_capture_proxy: >>>>>> >>>>>> /* Location wrappers should be stripped or otherwise handled by the >>>>>> caller before using this predicate. */ >>>>>> gcc_checking_assert (!location_wrapper_p (decl)); >>>>>> >>>>>> so fixed as the comment suggests. We only crash with the redundant >>>>>> capture: >>>>>> >>>>>> int abyPage = [=, abyPage] { ... } >>>>>> >>>>>> because prune_lambda_captures is only called when there was a default >>>>>> capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. >>>>> >>>>> It's weird that we even get this far in var_to_maybe_prune. Shouldn't >>>>> LAMBDA_CAPTURE_EXPLICIT_P be true for abyPage? >>>> >>>> Ug, I was seduced by the ostensible obviousness and failed to notice >>>> that check. In that light, the correct fix ought to be this. Thanks! >>>> >>>> Bootstrap/regtest running on x86_64-pc-linux-gnu, ok for trunk if it >>>> passes? >>>> >>>> -- >8 -- >>>> Here we crash in is_capture_proxy: >>>> >>>> /* Location wrappers should be stripped or otherwise handled by the >>>> caller before using this predicate. */ >>>> gcc_checking_assert (!location_wrapper_p (decl)); >>>> >>>> We only crash with the redundant capture: >>>> >>>> int abyPage = [=, abyPage] { ... } >>>> >>>> because prune_lambda_captures is only called when there was a default >>>> capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. >>>> >>>> The problem is that LAMBDA_CAPTURE_EXPLICIT_P wasn't propagated >>>> correctly and so var_to_maybe_prune proceeded where it shouldn't. >>>> >>>> PR c++/108829 >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> * pt.cc (tsubst_lambda_expr): Propagate LAMBDA_CAPTURE_EXPLICIT_P. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> * g++.dg/cpp0x/lambda/lambda-108829.C: New test. >>>> --- >>>> gcc/cp/pt.cc | 4 ++++ >>>> gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C | 11 +++++++++++ >>>> 2 files changed, 15 insertions(+) >>>> create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C >>>> >>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc >>>> index b1ac7d4beb4..f747ce877b5 100644 >>>> --- a/gcc/cp/pt.cc >>>> +++ b/gcc/cp/pt.cc >>>> @@ -19992,6 +19992,10 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) >>>> if (id_equal (DECL_NAME (field), "__this")) >>>> LAMBDA_EXPR_THIS_CAPTURE (r) = field; >>>> } >>>> + >>>> + if (LAMBDA_EXPR_CAPTURE_LIST (r)) >>>> + LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (r)) >>>> + = LAMBDA_CAPTURE_EXPLICIT_P (LAMBDA_EXPR_CAPTURE_LIST (t)); >>> >>> I'm not sure how the flag works for pack captures but it looks like >>> this would only propagate the flag to the last expanded capture if >>> the capture was originally a pack. >> >> Testcase: >> >> template<int, class... Ts> >> void f(Ts... ts) { >> constexpr int IDX_PAGE_SIZE = 4096; >> int abyPage = [=, ts...] { return IDX_PAGE_SIZE; }(); >> } >> void h() { >> f<1>(0, 1); >> } > > Thanks a lot for the testacase. Revised patch below. Look OK? > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. > -- >8 -- > Here we crash in is_capture_proxy: > > /* Location wrappers should be stripped or otherwise handled by the > caller before using this predicate. */ > gcc_checking_assert (!location_wrapper_p (decl)); > > We only crash with the redundant capture: > > int abyPage = [=, abyPage] { ... } > > because prune_lambda_captures is only called when there was a default > capture, and with [=] only abyPage won't be in LAMBDA_EXPR_CAPTURE_LIST. > > The problem is that LAMBDA_CAPTURE_EXPLICIT_P wasn't propagated > correctly and so var_to_maybe_prune proceeded where it shouldn't. > > PR c++/108829 > > gcc/cp/ChangeLog: > > * pt.cc (prepend_one_capture): Set LAMBDA_CAPTURE_EXPLICIT_P. > (tsubst_lambda_expr): Pass LAMBDA_CAPTURE_EXPLICIT_P to > prepend_one_capture. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp0x/lambda/lambda-108829-2.C: New test. > * g++.dg/cpp0x/lambda/lambda-108829.C: New test. > > Co-Authored by: Patrick Palka <ppalka@redhat.com> > --- > gcc/cp/pt.cc | 9 ++++++--- > gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C | 11 +++++++++++ > gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C | 11 +++++++++++ > 3 files changed, 28 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C > create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index b1ac7d4beb4..1a071e95004 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -19870,10 +19870,11 @@ tsubst_non_call_postfix_expression (tree t, tree args, > > /* Subroutine of tsubst_lambda_expr: add the FIELD/INIT capture pair to the > LAMBDA_EXPR_CAPTURE_LIST passed in LIST. Do deduction for a previously > - dependent init-capture. */ > + dependent init-capture. EXPLICIT_P is true if the original list had > + explicit captures. */ > > static void > -prepend_one_capture (tree field, tree init, tree &list, > +prepend_one_capture (tree field, tree init, tree &list, bool explicit_p, > tsubst_flags_t complain) > { > if (tree auto_node = type_uses_auto (TREE_TYPE (field))) > @@ -19893,6 +19894,7 @@ prepend_one_capture (tree field, tree init, tree &list, > cp_apply_type_quals_to_decl (cp_type_quals (type), field); > } > list = tree_cons (field, init, list); > + LAMBDA_CAPTURE_EXPLICIT_P (list) = explicit_p; > } > > /* T is a LAMBDA_EXPR. Generate a new LAMBDA_EXPR for the current > @@ -19982,12 +19984,13 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl) > prepend_one_capture (TREE_VEC_ELT (field, i), > TREE_VEC_ELT (init, i), > LAMBDA_EXPR_CAPTURE_LIST (r), > + LAMBDA_CAPTURE_EXPLICIT_P (cap), > complain); > } > else > { > prepend_one_capture (field, init, LAMBDA_EXPR_CAPTURE_LIST (r), > - complain); > + LAMBDA_CAPTURE_EXPLICIT_P (cap), complain); > > if (id_equal (DECL_NAME (field), "__this")) > LAMBDA_EXPR_THIS_CAPTURE (r) = field; > diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C > new file mode 100644 > index 00000000000..4e24470514d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829-2.C > @@ -0,0 +1,11 @@ > +// PR c++/108829 > +// { dg-do compile { target c++11 } } > + > +template<int, class... Ts> > +void f(Ts... ts) { > + constexpr int IDX_PAGE_SIZE = 4096; > + int abyPage = [=, ts...] { return IDX_PAGE_SIZE; }(); // { dg-error "redundant" } > +} > +void h() { > + f<1>(0, 1); > +} > diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > new file mode 100644 > index 00000000000..e621a0d14d0 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-108829.C > @@ -0,0 +1,11 @@ > +// PR c++/108829 > +// { dg-do compile { target c++11 } } > + > +template <int> > +void f(void) { > + constexpr int IDX_PAGE_SIZE = 4096; > + int abyPage = [=, abyPage] { return IDX_PAGE_SIZE; }(); // { dg-error "redundant" } > +} > +void h() { > + f<1>(); > +} > > base-commit: 5fea1be820508e1fbc610d1a54b61c1add33c36f ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-20 9:45 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-17 18:58 [PATCH] c++: ICE with redundant capture [PR108829] Marek Polacek 2023-02-17 20:00 ` Patrick Palka 2023-02-17 21:06 ` [PATCH v2] " Marek Polacek 2023-02-17 21:27 ` Patrick Palka 2023-02-17 21:32 ` Patrick Palka 2023-02-17 22:42 ` [PATCH v3] " Marek Polacek 2023-02-20 2:43 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).