| Message ID | 20240325204448.340940-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: templated substitution into lambda-expr [PR114393] | expand |
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). 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); +}
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); } > > 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 > >
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 > > > > >
On 3/27/24 10:01, Patrick Palka wrote: > On Mon, 25 Mar 2024, Patrick Palka wrote: >> On Mon, 25 Mar 2024, Patrick Palka wrote: >>> >>> 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). I suspect the problem comes from this bit in type_unification_real: > /* First instatiate in template context, in case we still > depend on undeduced template parameters. */ > ++processing_template_decl; > substed = tsubst_template_arg (arg, full_targs, complain, > NULL_TREE); > --processing_template_decl; > if (substed != error_mark_node > && !uses_template_parms (substed)) > /* We replaced all the tparms, substitute again out of > template context. */ > substed = NULL_TREE; and perhaps we should switch to searching the argument for undeduced template parameters rather than doing a trial substitution. But the pattern of setting processing_template_decl, substituting, and clearing it again is very widespread, so we still want to handle lambdas in that context. >>> + 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); > } Hmm, unconditionally deferring would probably also help to resolve issues with local classes in generic lambdas. It might be worth going that way rather than continue to grapple with partial substitution problems. >> 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 >> @@ -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. */ as a *default* template argument. OK with that tweak. Jason
On Wed, 10 Apr 2024, Jason Merrill wrote: > On 3/27/24 10:01, Patrick Palka wrote: > > On Mon, 25 Mar 2024, Patrick Palka wrote: > > > On Mon, 25 Mar 2024, Patrick Palka wrote: > > > > > > > > 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). > > I suspect the problem comes from this bit in type_unification_real: > > > /* First instatiate in template context, in case we still > > depend on undeduced template parameters. */ > > ++processing_template_decl; > > substed = tsubst_template_arg (arg, full_targs, complain, > > NULL_TREE); > > --processing_template_decl; > > if (substed != error_mark_node > > && !uses_template_parms (substed)) > > /* We replaced all the tparms, substitute again out of > > template context. */ > > substed = NULL_TREE; > > and perhaps we should switch to searching the argument for undeduced template > parameters rather than doing a trial substitution. Interesting, you also mentioned that approach would help fix PR101463 (which I need to get back to): https://gcc.gnu.org/pipermail/gcc-patches/2024-January/642340.html > > But the pattern of setting processing_template_decl, substituting, and > clearing it again is very widespread, so we still want to handle lambdas in > that context. > > > > > + 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); > > } > > Hmm, unconditionally deferring would probably also help to resolve issues with > local classes in generic lambdas. It might be worth going that way rather > than continue to grapple with partial substitution problems. > > > > 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 > > > @@ -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. */ > > as a *default* template argument. OK with that tweak. Thanks a lot, pushed as r14-9938-g081c1e93d56d35. Unfortunately this isn't enough to fix the original testcase from the PR due a related but distinct return type confusion issue. It ultimately seems we need to defer substitution in more cases, not just when we don't have a template context. I submitted a follow-up patch at https://gcc.gnu.org/pipermail/gcc-patches/2024-April/649387.html The patch makes us defer for all dependent substitution. Unconditionally deferring would do the trick as well, but I reckon a narrower fix might be better at this point. > > Jason > >
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. */ + 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); +}