On Thu, 5 Sep 2024, Jason Merrill wrote:
> On 9/5/24 2:28 PM, Patrick Palka wrote:
> > On Thu, 5 Sep 2024, Jason Merrill wrote:
> >
> > > On 9/5/24 1:26 PM, Patrick Palka wrote:
> > > > On Thu, 5 Sep 2024, Jason Merrill wrote:
> > > >
> > > > > On 9/5/24 10:54 AM, Patrick Palka wrote:
> > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > > > > > for trunk/14?
> > > > > >
> > > > > > -- >8 --
> > > > > >
> > > > > > A lambda within a default template argument used in some template-id
> > > > > > may have a smaller template depth than the context of the
> > > > > > template-id.
> > > > > > For example, the lambda in v1's default template argument has
> > > > > > template
> > > > > > depth 1, and in v2's has template depth 2, but the template-ids
> > > > > > v1<0>
> > > > > > and v2<0> which uses these default arguments appear in a depth 3
> > > > > > template
> > > > > > context. So add_extra_args will ultimately return args with depth 3
> > > > > > --
> > > > > > too many args for the lambda, leading to a bogus substitution.
> > > > > >
> > > > > > This patch fixes this by trimming the result of add_extra_args to
> > > > > > match
> > > > > > the template depth of the lambda. A new LAMBDA_EXPR_TEMPLATE_DEPTH
> > > > > > field
> > > > > > is added that tracks the template-ness of a lambda;
> > > > > >
> > > > > > PR c++/116567
> > > > > >
> > > > > > gcc/cp/ChangeLog:
> > > > > >
> > > > > > * pt.cc (tsubst_lambda_expr): For a deferred-substitution
> > > > > > lambda,
> > > > > > trim the augmented template arguments to match the template
> > > > > > depth
> > > > > > of the lambda.
> > > > > >
> > > > > > gcc/testsuite/ChangeLog:
> > > > > >
> > > > > > * g++.dg/cpp2a/lambda-targ7.C: New test.
> > > > > > ---
> > > > > > gcc/cp/pt.cc | 11 +++++++++
> > > > > > gcc/testsuite/g++.dg/cpp2a/lambda-targ7.C | 30
> > > > > > +++++++++++++++++++++++
> > > > > > 2 files changed, 41 insertions(+)
> > > > > > create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ7.C
> > > > > >
> > > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > > > > index 747e627f547..c49a26b4f5e 100644
> > > > > > --- a/gcc/cp/pt.cc
> > > > > > +++ b/gcc/cp/pt.cc
> > > > > > @@ -19699,6 +19699,17 @@ tsubst_lambda_expr (tree t, tree args,
> > > > > > tsubst_flags_t complain, tree in_decl)
> > > > > > LAMBDA_EXPR_EXTRA_ARGS (t) = build_extra_args (t, args,
> > > > > > complain);
> > > > > > return t;
> > > > > > }
> > > > > > + if (LAMBDA_EXPR_EXTRA_ARGS (t))
> > > > > > + {
> > > > > > + /* If we deferred substitution into this lambda, then it's
> > > > > > probably
> > > > > > from
> > > > >
> > > > > "probably" seems wrong, given that it wasn't implemented for this
> > > > > case.
> > > >
> > > > I said "probably" because in e.g.
> > > >
> > > > template<int N, auto F = []{}>
> > > > bool b = true;
> > > >
> > > > template<class T>
> > > > void f() {
> > > > b<0>;
> > > > }
> > > >
> > > > the lambda context has the same depth as the template-id context. But
> > > > as you point out, the issue is ultimately related vs unrelated
> > > > parameters rather than depth.
> > > >
> > > > >
> > > > > > + a context (e.g. default template argument context) which may
> > > > > > have
> > > > > > fewer
> > > > > > + levels than the current context it's embedded in. Adjust the
> > > > > > result
> > > > > > of
> > > > > > + add_extra_args accordingly. */
> > > > >
> > > > > Hmm, this looks like a situation of not just fewer levels, but
> > > > > potentially
> > > > > unrelated levels. "args" here is for f, which shares no template
> > > > > context
> > > > > with
> > > > > v1. What happens if your templates have non-type template parameters?
> > > >
> > > > Indeed before add_extra_args 'args' will be unrelated, but after doing
> > > > add_extra_args the innermost levels of 'args' will correspond to the
> > > > lambda's template context, and so using get_innermost_template_args
> > > > ought to get rid of the unrelated arguments, keeping only the ones
> > > > relevant to the original lambda context.
> > >
> > > Will they? The original function of add_extra_args was to reintroduce
> > > outer
> > > args that we weren't able to substitute the last time through
> > > tsubst_lambda_expr. I expect the innermost levels of 'args' to be the
> > > same
> > > before and after.
> > >
> > > Hmm, looking at add_extra_args again, I see that whether the EXTRA_ARGS go
> > > on
> > > the outside or the inside depends on whether they're dependent. How does
> > > this
> > > work other than by accident? >.>
> >
> > It's kind of a happy accident indeed :P In the cases this patch is
> > concerned with, i.e. a template-id using a default argument containing
> > a lambda, the extra args will always be considered dependent because
> > during default template argument coercion we substitute an incomplete
> > set of targs into the default targ (namely the element corresponding to
> > the default targ is NULL_TREE), which any_dependent_template_arguments_p
> > considers dependent. So add_extra_args will reliably put these captured
> > extra args as the innermost!
>
> I see that the testcases you enabled this code to handle in
> r12-2222-g2c699fd29829cd were also about lambda/requires in default template
> arguments. Can we detect this case some other way than uses_template_parms?
I think during default template argument coercion we need to set
tf_partial when in a template context. Then build_extra_args should
remember if this flag was set. Then add_extra_args should merge the
extra arguments according to whether the flag was set during deferring.
>
> Can we do the pruning added by this patch in add_extra_args instead of its
> caller?
If we're only concerned with the default template argument situation, then
it seems 'extra' will always be a full set of template arguments (even when
the template-id names a template from the current instantiation) so I think
add_extra_args just needs to substitute into 'extra' and not do any additional
pruning.
>
> Incidentally, why is having extra outer levels causing trouble? I thought we
> were generally able to safely ignore them.
I thought extra _inner_ levels are safe to ignore generally? If we have
extra outer levels then it causes the inner args to no longer be at the
right level.
For
template<auto N, auto = [] {}>
bool flag = true;
template<typename>
struct core {
template<typename>
auto func() -> bool {
return flag<0>;
}
};
auto main() -> int {
core<int> _;
[[maybe_unused]] bool flag = _.func<int>();
}
we crash during substitution into lambda's 'auto' return type which has
level 2, and 'args' after add_extra_args is {{int},{0, <empty>}}, so
we see non-type argument at level 2 index 0 where a type is expected.
(And if the lambda used N, that substitution would be similarly wrong
as well.)
How does the following look? Bootstrapped and regtested on
x86_64-pc-linux-gnu. This would be too risky to backport, so for 14 I
think we should go with the original patch.
-- >8 --
Subject: [PATCH] c++: deferring partial substitution into lambda [PR116567]
PR c++/116567
gcc/cp/ChangeLog:
* pt.cc (coerce_template_parms): Pass tf_partial when substituting
into a default template argument in a template context.
(build_extra_args): Set TREE_STATIC on the extra args if this is
a partial substitution.
(add_extra_args): Handle deferred partial substitution.
(tsubst_lammda_expr): Check for tf_partial instead of dependence
of args when determining whether to defer substitution.
(tsubst_expr) <case LAMBDA_EXPR>: Remove tf_partial early exit.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/lambda-targ7.C: New test.
---
gcc/cp/pt.cc | 33 +++++++++---------
gcc/testsuite/g++.dg/cpp2a/lambda-targ7.C | 42 +++++++++++++++++++++++
2 files changed, 58 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-targ7.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 9195a5274e1..9a9c16a47c2 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -9326,7 +9326,9 @@ coerce_template_parms (tree parms,
{
/* There must be a default arg in this case. */
arg = tsubst_template_arg (TREE_PURPOSE (parm), new_args,
- complain, in_decl);
+ complain | (processing_template_decl
+ ? tf_partial : tf_none),
+ in_decl);
/* The position of the first default template argument,
is also the number of non-defaulted arguments in NEW_INNER_ARGS.
Record that. */
@@ -13572,6 +13574,9 @@ build_extra_args (tree pattern, tree args,
tsubst_flags_t complain)
if (local_specializations)
if (tree locals = extract_local_specs (pattern, complain))
extra = tree_cons (NULL_TREE, extra, locals);
+ if (complain & tf_partial)
+ /* Remember whether this is a partial substitution. */
+ TREE_STATIC (extra) = true;
return extra;
}
@@ -13581,7 +13586,10 @@ build_extra_args (tree pattern, tree args,
tsubst_flags_t complain)
tree
add_extra_args (tree extra, tree args, tsubst_flags_t complain, tree in_decl)
{
- if (extra && TREE_CODE (extra) == TREE_LIST)
+ if (!extra)
+ return args;
+
+ if (TREE_CODE (extra) == TREE_LIST)
{
for (tree elt = TREE_CHAIN (extra); elt; elt = TREE_CHAIN (elt))
{
@@ -13599,13 +13607,11 @@ add_extra_args (tree extra, tree args, tsubst_flags_t
complain, tree in_decl)
gcc_assert (!TREE_PURPOSE (extra));
extra = TREE_VALUE (extra);
}
- if (uses_template_parms (extra))
- {
- /* This can happen after dependent substitution into a
- requires-expr or a lambda that uses constexpr if. */
- extra = tsubst_template_args (extra, args, complain, in_decl);
- args = add_outermost_template_args (args, extra);
- }
+ if (TREE_STATIC (extra))
+ /* This is a partial substitution into e.g. a requires-expr or lambda
+ inside a default template argument; we expect 'extra' to be a full
+ set of template arguments for the template context. */
+ args = tsubst_template_args (extra, args, complain, in_decl);
else
args = add_to_template_args (extra, args);
return args;
@@ -19706,7 +19712,7 @@ tsubst_lambda_expr (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
args = add_extra_args (LAMBDA_EXPR_EXTRA_ARGS (t), args, complain, in_decl);
if (processing_template_decl
- && (!in_template_context || any_dependent_template_arguments_p (args)))
+ && (!in_template_context || (complain & tf_partial)))
{
/* Defer templated substitution into a lambda-expr if we lost the
necessary template context. This may happen for a lambda-expr
@@ -21827,13 +21833,6 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
complain, tree in_decl)
case LAMBDA_EXPR:
{
- if (complain & tf_partial)
- {
- /* We don't have a full set of template arguments yet; don't touch
- the lambda at all. */
- gcc_assert (processing_template_decl);
- return t;
- }
tree r = tsubst_lambda_expr (t, args, complain, in_decl);
RETURN (build_lambda_object (r));
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-targ7.C
b/gcc/testsuite/g++.dg/cpp2a/lambda-targ7.C
new file mode 100644
index 00000000000..97b42d6ed0c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-targ7.C
@@ -0,0 +1,42 @@
+// PR c++/116567
+// { dg-do compile { target c++20 } }
+
+struct X { int n; };
+
+template<auto N, auto F = []{ return N; }>
+auto v1 = F;
+
+template<auto N, auto F = [](auto y){ return decltype(N){N.n + y}; }>
+auto v1g = F;
+
+template<class T>
+struct A {
+ template<auto N, auto F = []{ return N; }>
+ static inline auto v2 = F;
+
+ template<auto N, auto F = [](auto y){ return decltype(N){N.n + y}; }>
+ static inline auto v2g = F;
+
+ template<class U>
+ struct B {
+ template<auto N, auto F = []{ return N; }>
+ static inline auto v3 = F;
+
+ template<auto N, auto F = [](auto y){ return decltype(N){N.n + y}; }>
+ static inline auto v3g = F;
+
+ template<class V>
+ static void f() {
+ static_assert(v1<X{1}>().n == 1);
+ static_assert(v1g<X{1}>(42).n == 1 + 42);
+ static_assert(v2<X{2}>().n == 2);
+ static_assert(v2g<X{2}>(42).n == 2 + 42);
+ static_assert(v3<X{3}>().n == 3);
+ static_assert(v3g<X{3}>(42).n == 3 + 42);
+ }
+ };
+};
+
+int main() {
+ A<int>::B<int>::f<int>();
+}
--
2.46.0.519.g2e7b89e038
