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