Re: [PATCH] c++: templated substitution into lambda-expr [PR114393]

[Patrick Palka <ppalka@redhat.com>]

On Wed, 27 Mar 2024, Patrick Palka wrote:

> On Mon, 25 Mar 2024, Patrick Palka wrote:
> 
> > On Mon, 25 Mar 2024, Patrick Palka wrote:
> > 
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > > for trunk?
> > > 
> > > -- >8 --
> > > 
> > > The below testcases use a lambda-expr as a template argument and they
> > > all trip over the below added tsubst_lambda_expr sanity check ultimately
> > > because current_template_parms is empty, which causes push_template_decl
> > > to return error_mark_node from the call to begin_lambda_type.  Were it
> > > not for the sanity check this silent error_mark_node result leads to
> > > nonsensical errors down the line, or silent breakage.
> > > 
> > > In the first testcase, we hit this assert during instantiation of the
> > > dependent alias template-id c1_t<_Data> from instantiate_template, which
> > > clears current_template_parms via push_to_top_level.  Similar story for
> > > the second testcase.  For the third testcase we hit the assert during
> > > partial instantiation of the member template from instantiate_class_template
> > > which similarly calls push_to_top_level.
> > > 
> > > These testcases illustrate that templated substitution into a lambda-expr
> > > is not always possible, in particular when we lost the relevant template
> > > context.  I experimented with recovering the template context by making
> > > tsubst_lambda_expr fall back to using scope_chain->prev->template_parms if
> > > current_template_parms is empty which worked but seemed like a hack.  I
> > > also experimented with preserving the template context by keeping
> > > current_template_parms set during instantiate_template for a dependent
> > > specialization which also worked but it's at odds with the fact that we
> > > cache dependent specializations (and so they should be independent of
> > > the template context).
> > > 
> > > So instead of trying to make such substitution work, this patch uses the
> > > extra-args mechanism to defer templated substitution into a lambda-expr
> > > when we lost the relevant template context.
> > > 
> > > 	PR c++/114393
> > > 	PR c++/107457
> > > 	PR c++/93595
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > > 	* cp-tree.h (LAMBDA_EXPR_EXTRA_ARGS):
> > > 	(struct GTY):
> > > 	* module.cc (trees_out::core_vals) <case LAMBDA_EXPR>: Stream
> > > 	LAMBDA_EXPR_EXTRA_ARGS.
> > > 	(trees_in::core_vals) <case LAMBDA_EXPR>: Likewise.
> > > 	* pt.cc (has_extra_args_mechanism_p):
> > > 	(tsubst_lambda_expr):
> > 
> > Whoops, this version of the patch has an incomplete ChangeLog entry.
> > 
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	* g++.dg/cpp2a/lambda-targ2.C: New test.
> > > 	* g++.dg/cpp2a/lambda-targ3.C: New test.
> > > 	* g++.dg/cpp2a/lambda-targ4.C: New test.
> > > ---
> > >  gcc/cp/cp-tree.h                          |  5 +++++
> > >  gcc/cp/module.cc                          |  2 ++
> > >  gcc/cp/pt.cc                              | 20 ++++++++++++++++++--
> > >  gcc/testsuite/g++.dg/cpp2a/lambda-targ2.C | 19 +++++++++++++++++++
> > >  gcc/testsuite/g++.dg/cpp2a/lambda-targ3.C | 12 ++++++++++++
> > >  gcc/testsuite/g++.dg/cpp2a/lambda-targ4.C | 12 ++++++++++++
> > >  6 files changed, 68 insertions(+), 2 deletions(-)
> > >  create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ2.C
> > >  create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ3.C
> > >  create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ4.C
> > > 
> > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > > index c29a5434492..27100537038 100644
> > > --- a/gcc/cp/cp-tree.h
> > > +++ b/gcc/cp/cp-tree.h
> > > @@ -1538,6 +1538,10 @@ enum cp_lambda_default_capture_mode_type {
> > >  #define LAMBDA_EXPR_REGEN_INFO(NODE) \
> > >    (((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->regen_info)
> > >  
> > > +/* Like PACK_EXPANSION_EXTRA_ARGS, for lambda-expressions.  */
> > > +#define LAMBDA_EXPR_EXTRA_ARGS(NODE) \
> > > +  (((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->extra_args)
> > > +
> > >  /* The closure type of the lambda, which is also the type of the
> > >     LAMBDA_EXPR.  */
> > >  #define LAMBDA_EXPR_CLOSURE(NODE) \
> > > @@ -1550,6 +1554,7 @@ struct GTY (()) tree_lambda_expr
> > >    tree this_capture;
> > >    tree extra_scope;
> > >    tree regen_info;
> > > +  tree extra_args;
> > >    vec<tree, va_gc> *pending_proxies;
> > >    location_t locus;
> > >    enum cp_lambda_default_capture_mode_type default_capture_mode : 2;
> > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > index 52c60cf370c..1cd890909e3 100644
> > > --- a/gcc/cp/module.cc
> > > +++ b/gcc/cp/module.cc
> > > @@ -6312,6 +6312,7 @@ trees_out::core_vals (tree t)
> > >        WT (((lang_tree_node *)t)->lambda_expression.this_capture);
> > >        WT (((lang_tree_node *)t)->lambda_expression.extra_scope);
> > >        WT (((lang_tree_node *)t)->lambda_expression.regen_info);
> > > +      WT (((lang_tree_node *)t)->lambda_expression.extra_args);
> > >        /* pending_proxies is a parse-time thing.  */
> > >        gcc_assert (!((lang_tree_node *)t)->lambda_expression.pending_proxies);
> > >        if (state)
> > > @@ -6814,6 +6815,7 @@ trees_in::core_vals (tree t)
> > >        RT (((lang_tree_node *)t)->lambda_expression.this_capture);
> > >        RT (((lang_tree_node *)t)->lambda_expression.extra_scope);
> > >        RT (((lang_tree_node *)t)->lambda_expression.regen_info);
> > > +      RT (((lang_tree_node *)t)->lambda_expression.extra_args);
> > >        /* lambda_expression.pending_proxies is NULL  */
> > >        ((lang_tree_node *)t)->lambda_expression.locus
> > >  	= state->read_location (*this);
> > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > index 8cf0d5b7a8d..b1a9ee2b385 100644
> > > --- a/gcc/cp/pt.cc
> > > +++ b/gcc/cp/pt.cc
> > > @@ -3855,7 +3855,8 @@ has_extra_args_mechanism_p (const_tree t)
> > >    return (PACK_EXPANSION_P (t) /* PACK_EXPANSION_EXTRA_ARGS  */
> > >  	  || TREE_CODE (t) == REQUIRES_EXPR /* REQUIRES_EXPR_EXTRA_ARGS  */
> > >  	  || (TREE_CODE (t) == IF_STMT
> > > -	      && IF_STMT_CONSTEXPR_P (t))); /* IF_STMT_EXTRA_ARGS  */
> > > +	      && IF_STMT_CONSTEXPR_P (t)) /* IF_STMT_EXTRA_ARGS  */
> > > +	  || TREE_CODE (t) == LAMBDA_EXPR) /* LAMBDA_EXPR_EXTRA_ARGS  */;
> > >  }
> > >  
> > >  /* Structure used to track the progress of find_parameter_packs_r.  */
> > > @@ -19571,6 +19572,18 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
> > >    tree oldfn = lambda_function (t);
> > >    in_decl = oldfn;
> > >  
> > > +  args = add_extra_args (LAMBDA_EXPR_EXTRA_ARGS (t), args, complain, in_decl);
> > > +  if (processing_template_decl && !in_template_context)
> > > +    {
> > > +      /* Defer templated substitution into a lambda-expr when arguments
> > > +	 are dependent or when we lost the necessary template context,
> > > +	 which may happen for a lambda-expr used as a template argument.  */
> > 
> > And this comment is stale (an earlier version of the patch also deferred
> > for dependent arguments even when current_template_parms is non-empty,
> > which I backed out to make the fix as narrow as possible).
> 
> FWIW I also experimented with unconditionally deferring templated
> substitution into a lambda-expr (i.e. iff processing_template_decl)
> which passed bootstrap+regtest, and turns out to also fix the
> (non-regression) PR114167.  I didn't analyze the underlying issue
> very closely though, there might very well be a better way to fix
> that particular non-regression PR.
> 
> One downside of unconditionally deferring is that it'd mean less
> ahead-of-time checking of uninvoked deeply-nested generic lambdas,
> e.g.:
> 
>   int main() {
>     [](auto x) {
>       [](auto) {
>         [](auto) { decltype(x)::fail; }; // not diagnosed anymore
>       };
>     }(0);
>   }

Ping.

> 
> > 
> > Better version:
> > 
> > -- >8 --
> > 
> > Subject: [PATCH] c++: templated substitution into lambda-expr [PR114393]
> > 
> > 	PR c++/114393
> > 	PR c++/107457
> > 	PR c++/93595
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* cp-tree.h (LAMBDA_EXPR_EXTRA_ARGS): Define.
> > 	(tree_lambda_expr::extra_args): New field.
> > 	* module.cc (trees_out::core_vals) <case LAMBDA_EXPR>: Stream
> > 	LAMBDA_EXPR_EXTRA_ARGS.
> > 	(trees_in::core_vals) <case LAMBDA_EXPR>: Likewise.
> > 	* pt.cc (has_extra_args_mechanism_p): Return true for LAMBDA_EXPR.
> > 	(tsubst_lambda_expr): Use LAMBDA_EXPR_EXTRA_ARGS to defer templated
> > 	substitution into a lambda-expr if we lost the template context.
> > 	Add sanity check for error_mark_node result from begin_lambda_type.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp2a/lambda-targ2.C: New test.
> > 	* g++.dg/cpp2a/lambda-targ3.C: New test.
> > 	* g++.dg/cpp2a/lambda-targ4.C: New test.
> > ---
> >  gcc/cp/cp-tree.h                          |  5 +++++
> >  gcc/cp/module.cc                          |  2 ++
> >  gcc/cp/pt.cc                              | 20 ++++++++++++++++++--
> >  gcc/testsuite/g++.dg/cpp2a/lambda-targ2.C | 19 +++++++++++++++++++
> >  gcc/testsuite/g++.dg/cpp2a/lambda-targ3.C | 12 ++++++++++++
> >  gcc/testsuite/g++.dg/cpp2a/lambda-targ4.C | 12 ++++++++++++
> >  6 files changed, 68 insertions(+), 2 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ2.C
> >  create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ3.C
> >  create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ4.C
> > 
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index c29a5434492..27100537038 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -1538,6 +1538,10 @@ enum cp_lambda_default_capture_mode_type {
> >  #define LAMBDA_EXPR_REGEN_INFO(NODE) \
> >    (((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->regen_info)
> >  
> > +/* Like PACK_EXPANSION_EXTRA_ARGS, for lambda-expressions.  */
> > +#define LAMBDA_EXPR_EXTRA_ARGS(NODE) \
> > +  (((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->extra_args)
> > +
> >  /* The closure type of the lambda, which is also the type of the
> >     LAMBDA_EXPR.  */
> >  #define LAMBDA_EXPR_CLOSURE(NODE) \
> > @@ -1550,6 +1554,7 @@ struct GTY (()) tree_lambda_expr
> >    tree this_capture;
> >    tree extra_scope;
> >    tree regen_info;
> > +  tree extra_args;
> >    vec<tree, va_gc> *pending_proxies;
> >    location_t locus;
> >    enum cp_lambda_default_capture_mode_type default_capture_mode : 2;
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 52c60cf370c..1cd890909e3 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -6312,6 +6312,7 @@ trees_out::core_vals (tree t)
> >        WT (((lang_tree_node *)t)->lambda_expression.this_capture);
> >        WT (((lang_tree_node *)t)->lambda_expression.extra_scope);
> >        WT (((lang_tree_node *)t)->lambda_expression.regen_info);
> > +      WT (((lang_tree_node *)t)->lambda_expression.extra_args);
> >        /* pending_proxies is a parse-time thing.  */
> >        gcc_assert (!((lang_tree_node *)t)->lambda_expression.pending_proxies);
> >        if (state)
> > @@ -6814,6 +6815,7 @@ trees_in::core_vals (tree t)
> >        RT (((lang_tree_node *)t)->lambda_expression.this_capture);
> >        RT (((lang_tree_node *)t)->lambda_expression.extra_scope);
> >        RT (((lang_tree_node *)t)->lambda_expression.regen_info);
> > +      RT (((lang_tree_node *)t)->lambda_expression.extra_args);
> >        /* lambda_expression.pending_proxies is NULL  */
> >        ((lang_tree_node *)t)->lambda_expression.locus
> >  	= state->read_location (*this);
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 8cf0d5b7a8d..c25bdd283f1 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -3855,7 +3855,8 @@ has_extra_args_mechanism_p (const_tree t)
> >    return (PACK_EXPANSION_P (t) /* PACK_EXPANSION_EXTRA_ARGS  */
> >  	  || TREE_CODE (t) == REQUIRES_EXPR /* REQUIRES_EXPR_EXTRA_ARGS  */
> >  	  || (TREE_CODE (t) == IF_STMT
> > -	      && IF_STMT_CONSTEXPR_P (t))); /* IF_STMT_EXTRA_ARGS  */
> > +	      && IF_STMT_CONSTEXPR_P (t)) /* IF_STMT_EXTRA_ARGS  */
> > +	  || TREE_CODE (t) == LAMBDA_EXPR) /* LAMBDA_EXPR_EXTRA_ARGS  */;
> >  }
> >  
> >  /* Structure used to track the progress of find_parameter_packs_r.  */
> > @@ -19571,6 +19572,18 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
> >    tree oldfn = lambda_function (t);
> >    in_decl = oldfn;
> >  
> > +  args = add_extra_args (LAMBDA_EXPR_EXTRA_ARGS (t), args, complain, in_decl);
> > +  if (processing_template_decl && !in_template_context)
> > +    {
> > +      /* Defer templated substitution into a lambda-expr when we lost the
> > +	 necessary template context, which may happen for a lambda-expr
> > +	 used as a template argument.  */
> > +      t = copy_node (t);
> > +      LAMBDA_EXPR_EXTRA_ARGS (t) = NULL_TREE;
> > +      LAMBDA_EXPR_EXTRA_ARGS (t) = build_extra_args (t, args, complain);
> > +      return t;
> > +    }
> > +
> >    tree r = build_lambda_expr ();
> >  
> >    LAMBDA_EXPR_LOCATION (r)
> > @@ -19662,7 +19675,10 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
> >  
> >    tree type = begin_lambda_type (r);
> >    if (type == error_mark_node)
> > -    return error_mark_node;
> > +    {
> > +      gcc_checking_assert (!(complain & tf_error) || seen_error ());
> > +      return error_mark_node;
> > +    }
> >  
> >    if (LAMBDA_EXPR_EXTRA_SCOPE (t))
> >      record_lambda_scope (r);
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-targ2.C b/gcc/testsuite/g++.dg/cpp2a/lambda-targ2.C
> > new file mode 100644
> > index 00000000000..41b8d8749f2
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/lambda-targ2.C
> > @@ -0,0 +1,19 @@
> > +// PR c++/114393
> > +// { dg-do compile { target c++20 } }
> > +
> > +template <auto _DescriptorFn> struct c1 {};
> > +
> > +template <class _Descriptor, auto t = [] { return _Descriptor(); }>
> > +inline constexpr auto b_v = t;
> > +
> > +template <class _Tag>
> > +using c1_t = c1<b_v<int>>;
> > +
> > +template <class _Data>
> > +constexpr auto g(_Data __data) {
> > +  return c1_t<_Data>{};
> > +}
> > +
> > +void f() {
> > +  auto &&b = g(0);
> > +}
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-targ3.C b/gcc/testsuite/g++.dg/cpp2a/lambda-targ3.C
> > new file mode 100644
> > index 00000000000..31d08add277
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/lambda-targ3.C
> > @@ -0,0 +1,12 @@
> > +// PR c++/107457
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<class T>
> > +using lambda = decltype([]() {});
> > +
> > +template<class R, class F = lambda<R>>
> > +void test() { }
> > +
> > +int main() {
> > +  test<int>();
> > +}
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-targ4.C b/gcc/testsuite/g++.dg/cpp2a/lambda-targ4.C
> > new file mode 100644
> > index 00000000000..341a1aa5bb1
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/lambda-targ4.C
> > @@ -0,0 +1,12 @@
> > +// PR c++/93595
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<int>
> > +struct bad {
> > +  template<class T, auto = []{ return T(); }>
> > +  static void f(T) { }
> > +};
> > +
> > +int main() {
> > +  bad<0>::f(0);
> > +}
> > -- 
> > 2.44.0.325.g11c821f2f2
> > 
> > 
>