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