[PATCH] c++: ICE with redundant capture [PR108829]

[PATCH] c++: ICE with redundant capture [PR108829]

[Marek Polacek]

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

Re: [PATCH] c++: ICE with redundant capture [PR108829]

[Patrick Palka]

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
> 
> 

[PATCH v2] c++: ICE with redundant capture [PR108829]

[Marek Polacek]

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

Re: [PATCH v2] c++: ICE with redundant capture [PR108829]

[Patrick Palka]

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
> 
> 

Re: [PATCH v2] c++: ICE with redundant capture [PR108829]

[Patrick Palka]

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
> > 
> > 
> 

[PATCH v3] c++: ICE with redundant capture [PR108829]

[Marek Polacek]

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

Re: [PATCH v3] c++: ICE with redundant capture [PR108829]

[Jason Merrill]

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