* [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).