c++: nested lambda capturing a capture proxy [PR94376]

  Here when determining the type of the FIELD_DECL for the by-value
capture of 'i' in the inner lambda, we incorrectly give it the
type const int instead of int since the effective initializer is
the proxy for the outer capture, and this proxy is const qualified
since the outer lambda is non-mutable.

This patch fixes this by handling capturing of capture proxies specially
in lambda_capture_field_type, namely we instead consider the type of
their FIELD_DECL which unlike the proxy has the true cv-quals of the
captured entity.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

	PR c++/94376

gcc/cp/ChangeLog:

	* lambda.c (lambda_capture_field_type): Simplify by handling the
	is_this case first and specially.  When capturing by-value a
	capture proxy, consider the type of the corresponding field
	instead.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/lambda/lambda-nested9.C: New test.
---
 gcc/cp/lambda.c                               | 19 +++++++--
 .../g++.dg/cpp0x/lambda/lambda-nested9.C      | 41 +++++++++++++++++++
 2 files changed, 56 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
  

On 11/2/21 14:06, Patrick Palka wrote:
> Here when determining the type of the FIELD_DECL for the by-value
> capture of 'i' in the inner lambda, we incorrectly give it the
> type const int instead of int since the effective initializer is
> the proxy for the outer capture, and this proxy is const qualified
> since the outer lambda is non-mutable.
> 
> This patch fixes this by handling capturing of capture proxies specially
> in lambda_capture_field_type, namely we instead consider the type of
> their FIELD_DECL which unlike the proxy has the true cv-quals of the
> captured entity.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> 	PR c++/94376
> 
> gcc/cp/ChangeLog:
> 
> 	* lambda.c (lambda_capture_field_type): Simplify by handling the
> 	is_this case first and specially.  When capturing by-value a
> 	capture proxy, consider the type of the corresponding field
> 	instead.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/lambda/lambda-nested9.C: New test.
> ---
>   gcc/cp/lambda.c                               | 19 +++++++--
>   .../g++.dg/cpp0x/lambda/lambda-nested9.C      | 41 +++++++++++++++++++
>   2 files changed, 56 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
> 
> diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
> index 2e9d38bbe83..f16c121dc78 100644
> --- a/gcc/cp/lambda.c
> +++ b/gcc/cp/lambda.c
> @@ -196,7 +196,9 @@ lambda_capture_field_type (tree expr, bool explicit_init_p,
>     tree type;
>     bool is_this = is_this_parameter (tree_strip_nop_conversions (expr));
>   
> -  if (!is_this && explicit_init_p)
> +  if (is_this)
> +    type = TREE_TYPE (expr);
> +  else if (explicit_init_p)
>       {
>         tree auto_node = make_auto ();
>         
> @@ -210,7 +212,7 @@ lambda_capture_field_type (tree expr, bool explicit_init_p,
>         else
>   	type = do_auto_deduction (type, expr, auto_node);
>       }
> -  else if (!is_this && type_dependent_expression_p (expr))
> +  else if (type_dependent_expression_p (expr))
>       {
>         type = cxx_make_type (DECLTYPE_TYPE);
>         DECLTYPE_TYPE_EXPR (type) = expr;
> @@ -220,10 +222,19 @@ lambda_capture_field_type (tree expr, bool explicit_init_p,
>       }
>     else
>       {
> +      if (!by_reference_p && is_capture_proxy (expr))
> +	{
> +	  /* When capturing by-value another capture proxy from an enclosing
> +	     lambda, consider the type of the corresponding field instead,
> +	     as the proxy may be const-qualifed if the enclosing lambda is
> +	     non-mutable (PR94376).  */
> +	  gcc_assert (TREE_CODE (DECL_VALUE_EXPR (expr)) == COMPONENT_REF);
> +	  expr = TREE_OPERAND (DECL_VALUE_EXPR (expr), 1);

Is there a reason not to use DECL_CAPTURED_VARIABLE?

> +	}
> +
>         type = non_reference (unlowered_expr_type (expr));
>   
> -      if (!is_this
> -	  && (by_reference_p || TREE_CODE (type) == FUNCTION_TYPE))
> +      if (by_reference_p || TREE_CODE (type) == FUNCTION_TYPE)
>   	type = build_reference_type (type);
>       }
>   
> diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
> new file mode 100644
> index 00000000000..ff7da3b0ce1
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
> @@ -0,0 +1,41 @@
> +// PR c++/94376
> +// { dg-do compile { target c++11 } }
> +
> +int main() {
> +  // We used to incorrectly reject the first two cases.
> +  int i = 0;
> +  [=] () {
> +    [=] () mutable {
> +      ++i;
> +    };
> +  };
> +
> +#if __cpp_init_captures
> +  [j=0] () {
> +    [=] () mutable {
> +      ++j;
> +    };
> +  };
> +#endif
> +
> +  [=] () {
> +    [&] () mutable {
> +      ++i; // { dg-error "read-only" }
> +    };
> +  };
> +
> +  const int j = 0;
> +  [=] () {
> +    [=] () mutable {
> +      ++j; // { dg-error "read-only" }
> +    };
> +  };
> +
> +#if __cpp_init_captures
> +  [j=0] () {
> +    [&] () mutable {
> +      ++j; // { dg-error "read-only" }
> +    };
> +  };
> +#endif
> +}
>
  

On Thu, 18 Nov 2021, Jason Merrill wrote:

> On 11/2/21 14:06, Patrick Palka wrote:
> > Here when determining the type of the FIELD_DECL for the by-value
> > capture of 'i' in the inner lambda, we incorrectly give it the
> > type const int instead of int since the effective initializer is
> > the proxy for the outer capture, and this proxy is const qualified
> > since the outer lambda is non-mutable.
> > 
> > This patch fixes this by handling capturing of capture proxies specially
> > in lambda_capture_field_type, namely we instead consider the type of
> > their FIELD_DECL which unlike the proxy has the true cv-quals of the
> > captured entity.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
> > 
> > 	PR c++/94376
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* lambda.c (lambda_capture_field_type): Simplify by handling the
> > 	is_this case first and specially.  When capturing by-value a
> > 	capture proxy, consider the type of the corresponding field
> > 	instead.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp0x/lambda/lambda-nested9.C: New test.
> > ---
> >   gcc/cp/lambda.c                               | 19 +++++++--
> >   .../g++.dg/cpp0x/lambda/lambda-nested9.C      | 41 +++++++++++++++++++
> >   2 files changed, 56 insertions(+), 4 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
> > 
> > diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
> > index 2e9d38bbe83..f16c121dc78 100644
> > --- a/gcc/cp/lambda.c
> > +++ b/gcc/cp/lambda.c
> > @@ -196,7 +196,9 @@ lambda_capture_field_type (tree expr, bool
> > explicit_init_p,
> >     tree type;
> >     bool is_this = is_this_parameter (tree_strip_nop_conversions (expr));
> >   -  if (!is_this && explicit_init_p)
> > +  if (is_this)
> > +    type = TREE_TYPE (expr);
> > +  else if (explicit_init_p)
> >       {
> >         tree auto_node = make_auto ();
> >         @@ -210,7 +212,7 @@ lambda_capture_field_type (tree expr, bool
> > explicit_init_p,
> >         else
> >   	type = do_auto_deduction (type, expr, auto_node);
> >       }
> > -  else if (!is_this && type_dependent_expression_p (expr))
> > +  else if (type_dependent_expression_p (expr))
> >       {
> >         type = cxx_make_type (DECLTYPE_TYPE);
> >         DECLTYPE_TYPE_EXPR (type) = expr;
> > @@ -220,10 +222,19 @@ lambda_capture_field_type (tree expr, bool
> > explicit_init_p,
> >       }
> >     else
> >       {
> > +      if (!by_reference_p && is_capture_proxy (expr))
> > +	{
> > +	  /* When capturing by-value another capture proxy from an enclosing
> > +	     lambda, consider the type of the corresponding field instead,
> > +	     as the proxy may be const-qualifed if the enclosing lambda is
> > +	     non-mutable (PR94376).  */
> > +	  gcc_assert (TREE_CODE (DECL_VALUE_EXPR (expr)) == COMPONENT_REF);
> > +	  expr = TREE_OPERAND (DECL_VALUE_EXPR (expr), 1);
> 
> Is there a reason not to use DECL_CAPTURED_VARIABLE?

IIRC that'd work for normal capture proxies, but DECL_CAPTURED_VARIABLE
is empty for explicitly-initialized captures and their capture proxies
have the same issue.

> 
> > +	}
> > +
> >         type = non_reference (unlowered_expr_type (expr));
> >   -      if (!is_this
> > -	  && (by_reference_p || TREE_CODE (type) == FUNCTION_TYPE))
> > +      if (by_reference_p || TREE_CODE (type) == FUNCTION_TYPE)
> >   	type = build_reference_type (type);
> >       }
> >   diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
> > b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
> > new file mode 100644
> > index 00000000000..ff7da3b0ce1
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
> > @@ -0,0 +1,41 @@
> > +// PR c++/94376
> > +// { dg-do compile { target c++11 } }
> > +
> > +int main() {
> > +  // We used to incorrectly reject the first two cases.
> > +  int i = 0;
> > +  [=] () {
> > +    [=] () mutable {
> > +      ++i;
> > +    };
> > +  };
> > +
> > +#if __cpp_init_captures
> > +  [j=0] () {
> > +    [=] () mutable {
> > +      ++j;
> > +    };
> > +  };
> > +#endif
> > +
> > +  [=] () {
> > +    [&] () mutable {
> > +      ++i; // { dg-error "read-only" }
> > +    };
> > +  };
> > +
> > +  const int j = 0;
> > +  [=] () {
> > +    [=] () mutable {
> > +      ++j; // { dg-error "read-only" }
> > +    };
> > +  };
> > +
> > +#if __cpp_init_captures
> > +  [j=0] () {
> > +    [&] () mutable {
> > +      ++j; // { dg-error "read-only" }
> > +    };
> > +  };
> > +#endif
> > +}
> > 
> 
>
  

On 11/18/21 17:48, Patrick Palka wrote:
> On Thu, 18 Nov 2021, Jason Merrill wrote:
> 
>> On 11/2/21 14:06, Patrick Palka wrote:
>>> Here when determining the type of the FIELD_DECL for the by-value
>>> capture of 'i' in the inner lambda, we incorrectly give it the
>>> type const int instead of int since the effective initializer is
>>> the proxy for the outer capture, and this proxy is const qualified
>>> since the outer lambda is non-mutable.
>>>
>>> This patch fixes this by handling capturing of capture proxies specially
>>> in lambda_capture_field_type, namely we instead consider the type of
>>> their FIELD_DECL which unlike the proxy has the true cv-quals of the
>>> captured entity.
>>>
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>> trunk?
>>>
>>> 	PR c++/94376
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* lambda.c (lambda_capture_field_type): Simplify by handling the
>>> 	is_this case first and specially.  When capturing by-value a
>>> 	capture proxy, consider the type of the corresponding field
>>> 	instead.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp0x/lambda/lambda-nested9.C: New test.
>>> ---
>>>    gcc/cp/lambda.c                               | 19 +++++++--
>>>    .../g++.dg/cpp0x/lambda/lambda-nested9.C      | 41 +++++++++++++++++++
>>>    2 files changed, 56 insertions(+), 4 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
>>>
>>> diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c
>>> index 2e9d38bbe83..f16c121dc78 100644
>>> --- a/gcc/cp/lambda.c
>>> +++ b/gcc/cp/lambda.c
>>> @@ -196,7 +196,9 @@ lambda_capture_field_type (tree expr, bool
>>> explicit_init_p,
>>>      tree type;
>>>      bool is_this = is_this_parameter (tree_strip_nop_conversions (expr));
>>>    -  if (!is_this && explicit_init_p)
>>> +  if (is_this)
>>> +    type = TREE_TYPE (expr);
>>> +  else if (explicit_init_p)
>>>        {
>>>          tree auto_node = make_auto ();
>>>          @@ -210,7 +212,7 @@ lambda_capture_field_type (tree expr, bool
>>> explicit_init_p,
>>>          else
>>>    	type = do_auto_deduction (type, expr, auto_node);
>>>        }
>>> -  else if (!is_this && type_dependent_expression_p (expr))
>>> +  else if (type_dependent_expression_p (expr))
>>>        {
>>>          type = cxx_make_type (DECLTYPE_TYPE);
>>>          DECLTYPE_TYPE_EXPR (type) = expr;
>>> @@ -220,10 +222,19 @@ lambda_capture_field_type (tree expr, bool
>>> explicit_init_p,
>>>        }
>>>      else
>>>        {
>>> +      if (!by_reference_p && is_capture_proxy (expr))
>>> +	{
>>> +	  /* When capturing by-value another capture proxy from an enclosing
>>> +	     lambda, consider the type of the corresponding field instead,
>>> +	     as the proxy may be const-qualifed if the enclosing lambda is
>>> +	     non-mutable (PR94376).  */
>>> +	  gcc_assert (TREE_CODE (DECL_VALUE_EXPR (expr)) == COMPONENT_REF);
>>> +	  expr = TREE_OPERAND (DECL_VALUE_EXPR (expr), 1);
>>
>> Is there a reason not to use DECL_CAPTURED_VARIABLE?
> 
> IIRC that'd work for normal capture proxies, but DECL_CAPTURED_VARIABLE
> is empty for explicitly-initialized captures and their capture proxies
> have the same issue.

Then the patch is OK as is.

>>
>>> +	}
>>> +
>>>          type = non_reference (unlowered_expr_type (expr));
>>>    -      if (!is_this
>>> -	  && (by_reference_p || TREE_CODE (type) == FUNCTION_TYPE))
>>> +      if (by_reference_p || TREE_CODE (type) == FUNCTION_TYPE)
>>>    	type = build_reference_type (type);
>>>        }
>>>    diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
>>> b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
>>> new file mode 100644
>>> index 00000000000..ff7da3b0ce1
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-nested9.C
>>> @@ -0,0 +1,41 @@
>>> +// PR c++/94376
>>> +// { dg-do compile { target c++11 } }
>>> +
>>> +int main() {
>>> +  // We used to incorrectly reject the first two cases.
>>> +  int i = 0;
>>> +  [=] () {
>>> +    [=] () mutable {
>>> +      ++i;
>>> +    };
>>> +  };
>>> +
>>> +#if __cpp_init_captures
>>> +  [j=0] () {
>>> +    [=] () mutable {
>>> +      ++j;
>>> +    };
>>> +  };
>>> +#endif
>>> +
>>> +  [=] () {
>>> +    [&] () mutable {
>>> +      ++i; // { dg-error "read-only" }
>>> +    };
>>> +  };
>>> +
>>> +  const int j = 0;
>>> +  [=] () {
>>> +    [=] () mutable {
>>> +      ++j; // { dg-error "read-only" }
>>> +    };
>>> +  };
>>> +
>>> +#if __cpp_init_captures
>>> +  [j=0] () {
>>> +    [&] () mutable {
>>> +      ++j; // { dg-error "read-only" }
>>> +    };
>>> +  };
>>> +#endif
>>> +}
>>>
>>
>>
>