On Tue, Dec 21, 2021 at 2:03 PM Patrick Palka <ppa...@redhat.com> wrote:
>
> On Wed, Jun 30, 2021 at 4:23 PM Jason Merrill <ja...@redhat.com> wrote:
> >
> > On 6/30/21 4:18 PM, Patrick Palka wrote:
> > > On Wed, Jun 30, 2021 at 3:51 PM Jason Merrill <ja...@redhat.com> wrote:
> > >>
> > >> On 6/30/21 11:58 AM, Patrick Palka wrote:
> > >>> On Wed, 30 Jun 2021, Patrick Palka wrote:
> > >>>
> > >>>> On Fri, 25 Jun 2021, Jason Merrill wrote:
> > >>>>
> > >>>>> On 6/25/21 1:11 PM, Patrick Palka wrote:
> > >>>>>> On Fri, 25 Jun 2021, Jason Merrill wrote:
> > >>>>>>
> > >>>>>>> On 6/24/21 4:45 PM, Patrick Palka wrote:
> > >>>>>>>> In the first testcase below, during parsing of the alias template
> > >>>>>>>> ConstSpanType, transparency of alias template specializations
> > >>>>>>>> means we
> > >>>>>>>> replace SpanType<T> with SpanType's substituted definition. But
> > >>>>>>>> this
> > >>>>>>>> substitution lowers the level of the CTAD placeholder for
> > >>>>>>>> span(T()) from
> > >>>>>>>> 2 to 1, and so the later instantiantion of ConstSpanType<int>
> > >>>>>>>> erroneously substitutes this CTAD placeholder with the template
> > >>>>>>>> argument
> > >>>>>>>> at level 1 index 0, i.e. with int, before we get a chance to
> > >>>>>>>> perform the
> > >>>>>>>> CTAD.
> > >>>>>>>>
> > >>>>>>>> In light of this, it seems we should avoid level lowering when
> > >>>>>>>> substituting through through the type-id of a dependent alias
> > >>>>>>>> template
> > >>>>>>>> specialization. To that end this patch makes
> > >>>>>>>> lookup_template_class_1
> > >>>>>>>> pass tf_partial to tsubst in this situation.
> > >>>>>>>
> > >>>>>>> This makes sense, but what happens if SpanType is a member
> > >>>>>>> template, so
> > >>>>>>> that
> > >>>>>>> the levels of it and ConstSpanType don't match? Or the other way
> > >>>>>>> around?
> > >>>>>>
> > >>>>>> If SpanType<T> is a member template of say the class template A<U>
> > >>>>>> (and
> > >>>>>> thus its level is greater than ConstSpanType):
> > >>>>>>
> > >>>>>> template<class U>
> > >>>>>> struct A {
> > >>>>>> template<class T>
> > >>>>>> using SpanType = decltype(span(T()));
> > >>>>>> };
> > >>>>>>
> > >>>>>> template<class T>
> > >>>>>> using ConstSpanType = span<const typename
> > >>>>>> A<int>::SpanType<T>::value_type>;
> > >>>>>>
> > >>>>>> using type = ConstSpanType<int>;
> > >>>>>>
> > >>>>>> then this case luckily works even without the patch because
> > >>>>>> instantiate_class_template now reuses the specialization
> > >>>>>> A<int>::SpanType<T>
> > >>>>>> that was formed earlier during instantiation of A<int>, where we
> > >>>>>> substitute only a single level of template arguments, so the level of
> > >>>>>> the CTAD placeholder inside the defining-type-id of this
> > >>>>>> specialization
> > >>>>>> dropped from 3 to 2, so still more than the level of ConstSpanType.
> > >>>>>>
> > >>>>>> This luck is short-lived though, because if we replace
> > >>>>>> A<int>::SpanType<T> with say A<int>::SpanType<const T> then the
> > >>>>>> testcase
> > >>>>>> breaks again (without the patch) because we no longer can reuse that
> > >>>>>> specialization, so we instead form it on the spot by substituting two
> > >>>>>> levels of template arguments (U=int,T=T) into the defining-type-id,
> > >>>>>> causing the level of the placeholder to drop to 1. I think the patch
> > >>>>>> causes its level to remain 3 (though I guess it should really be 2).
> > >>>>>>
> > >>>>>>
> > >>>>>> For the other way around, if ConstSpanType<T> is a member template of
> > >>>>>> say the class template B<V> (and thus its level is greater than
> > >>>>>> SpanType):
> > >>>>>>
> > >>>>>> template<class T>
> > >>>>>> using SpanType = decltype(span(T()));
> > >>>>>>
> > >>>>>> template<class V>
> > >>>>>> struct B {
> > >>>>>> template<class T>
> > >>>>>> using ConstSpanType = span<const typename
> > >>>>>> SpanType<T>::value_type>;
> > >>>>>> };
> > >>>>>>
> > >>>>>> using type = B<char>::ConstSpanType<int>;
> > >>>>>>
> > >>>>>> then tf_partial doesn't help here at all; we end up substituting
> > >>>>>> 'int'
> > >>>>>> for the CTAD placeholder... What it seems we need is to _increase_
> > >>>>>> the
> > >>>>>> level of the CTAD placeholder from 2 to 3 during the dependent
> > >>>>>> substitution..
> > >>>>>>
> > >>>>>> Hmm, rather than messing with tf_partial, which is apparently only a
> > >>>>>> partial solution, maybe we should just make tsubst never substitute a
> > >>>>>> CTAD placeholder -- they should always be resolved from
> > >>>>>> do_class_deduction,
> > >>>>>> and their level doesn't really matter otherwise. (But we'd still
> > >>>>>> want
> > >>>>>> to substitute into the CLASS_PLACEHOLDER_TEMPLATE of the placeholder
> > >>>>>> in
> > >>>>>> case it's a template template parm.) Something like:
> > >>>>>>
> > >>>>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > >>>>>> index 5107bfbf9d1..dead651ed84 100644
> > >>>>>> --- a/gcc/cp/pt.c
> > >>>>>> +++ b/gcc/cp/pt.c
> > >>>>>> @@ -15552,7 +15550,8 @@ tsubst (tree t, tree args, tsubst_flags_t
> > >>>>>> complain,
> > >>>>>> tree in_decl)
> > >>>>>> levels = TMPL_ARGS_DEPTH (args);
> > >>>>>> if (level <= levels
> > >>>>>> - && TREE_VEC_LENGTH (TMPL_ARGS_LEVEL (args, level)) > 0)
> > >>>>>> + && TREE_VEC_LENGTH (TMPL_ARGS_LEVEL (args, level)) > 0
> > >>>>>> + && !template_placeholder_p (t))
> > >>>>>> {
> > >>>>>> arg = TMPL_ARG (args, level, idx);
> > >>>>>>
> > >>>>>> seems to work better.
> > >>>>>
> > >>>>> Makes sense.
> > >>>>
> > >>>> Here's a patch that implements that. I reckon it's good to have both
> > >>>> workarounds in place because the tf_partial workaround is necessary to
> > >>>> accept class-deduction93a.C below, and the tsubst workaround is
> > >>>> necessary to accept class-deduction-92b.C below.
> > >>>
> > >>> Whoops, forgot to git-add class-deduction93a.C:
> > >>>
> > >>> -- >8 --
> > >>>
> > >>> Subject: [PATCH] c++: CTAD within alias template [PR91911]
> > >>>
> > >>> In the first testcase below, during parsing of the alias template
> > >>> ConstSpanType, transparency of alias template specializations means we
> > >>> replace SpanType<T> with SpanType's substituted definition. But this
> > >>> substitution lowers the level of the CTAD placeholder for span{T()} from
> > >>> 2 to 1, and so the later instantiation of ConstSpanType<int> erroneously
> > >>> substitutes this CTAD placeholder with the template argument at level 1
> > >>> index 0, i.e. with int, before we get a chance to perform the CTAD.
> > >>>
> > >>> In light of this, it seems we should avoid level lowering when
> > >>> substituting through the type-id of a dependent alias template
> > >>> specialization. To that end this patch makes lookup_template_class_1
> > >>> pass tf_partial to tsubst in this situation.
> > >>>
> > >>> Unfortunately, using tf_partial alone isn't sufficient because the
> > >>> template context in which we perform the dependent substitution may
> > >>> have more levels than the substituted alias template and so we
> > >>> end up substituting the CTAD placeholder anyway, as in
> > >>> class-deduction92b.C below. (There, it seems we'd need to _increase_
> > >>> the level of the placeholder for span{T()} from 2 to 3 during the
> > >>> dependent substitution.) Since we never want to resolve a CTAD
> > >>> placeholder outside of CTAD proper, this patch takes the relatively
> > >>> ad-hoc approach of making tsubst explicitly avoid doing so.
> > >>>
> > >>> This tsubst workaround doesn't obviate the tf_partial workaround because
> > >>> it's still desirable to avoid prematurely level lowering a CTAD
> > >>> placeholder:
> > >>> it's less work for the compiler, and it gives us a chance to substitute
> > >>> a template placeholder that's a template template parameter with a
> > >>> concrete template template argument, as in the last testcase below.
> > >>
> > >> Hmm, what if we combine the template template parameter with the level
> > >> mismatch?
> > >
> > > So for e.g.
> > >
> > > template<class F, template<class> class Tmpl>
> > > using CallableTraitT = CallableTrait<decltype(Tmpl{F()})>;
> > >
> > > template<class>
> > > struct A {
> > > template<class F, template<class> class Tmpl>
> > > using ReturnType = typename CallableTraitT<F, Tmpl>::ReturnType;
> > > };
> > >
> > > using type = A<int>::ReturnType<int(*)(), function>;
> > > using type = int;
> > >
> > > sadly we crash, because during the dependent substitution of the
> > > innermost arguments into the defining-type-id, tf_partial means we
> > > don't lower the level of the CTAD placeholder and therefore don't
> > > substitute into CLASS_PLACEHOLDER_TEMPLATE, so
> > > CLASS_PLACEHOLDER_TEMPLATE remains a template template parm at index 1
> > > level 1 (as opposed to level 2). Later during the full
> > > instantiation, there is no such template template argument at that
> > > position (it's at index 1 level 2 rather).
> > >
> > > To handle this testcase, it seems we need a way to substitute into
> > > CTAD placeholders without lowering their level I guess.
> >
> > Or replacing their level with the appropriate level for the args we're
> > dealing with/whether tf_partial is set?
>
> That sounds like it might work for CTAD placeholders, since we never
> want to replace them via tsubst anyway. But I suppose a complete
> solution to this problem would also need to adjust the level of 'auto'
> that appears inside unevaluated lambdas (and C++23 auto(x) now too, I
> guess). And the tricky part with those is that we do sometimes want
> to replace 'auto's via tsubst, in particular during
> do_auto_deduction..
>
> I wonder if for now the v1 patch (the one consisting of just the
> lookup_template_class_1 change) can go in? I noticed that it also
> fixes a slew of (essentially duplicate) PRs about simple uses of
> unevaluated lambdas within alias templates: 100594, 92211, 103569,
> 102680, 101315, 101013, 92707. (The template_placeholder_p change in
> the v2 patch I realized is buggy -- by avoiding substitution into the
> CTAD placeholder, we fall back to level-lowering, but since level <
> levels we end up creating a CTAD placeholder with level close to
> INT_MIN).
Whoops, sorry about the thinko in the last sentence, what I should
have said is, ... since level <= levels we end up creating a CTAD
placeholder with nonpositive level.
>
>
> >
> > >>
> > >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > >>> trunk?
> > >>>
> > >>> PR c++/91911
> > >>>
> > >>> gcc/cp/ChangeLog:
> > >>>
> > >>> * pt.c (lookup_template_class_1): When looking up a dependent
> > >>> alias template specialization, pass tf_partial to tsubst.
> > >>> (tsubst) <case TEMPLATE_TYPE_PARM>: Avoid substituting a CTAD
> > >>> placeholder.
> > >>>
> > >>> gcc/testsuite/ChangeLog:
> > >>>
> > >>> * g++.dg/cpp1z/class-deduction92.C: New test.
> > >>> * g++.dg/cpp1z/class-deduction92a.C: New test.
> > >>> * g++.dg/cpp1z/class-deduction92b.C: New test.
> > >>> * g++.dg/cpp1z/class-deduction93.C: New test.
> > >>> * g++.dg/cpp1z/class-deduction93a.C: New test.
> > >>> ---
> > >>> gcc/cp/pt.c | 15 +++++++++--
> > >>> .../g++.dg/cpp1z/class-deduction92.C | 17 ++++++++++++
> > >>> .../g++.dg/cpp1z/class-deduction92a.C | 22 +++++++++++++++
> > >>> .../g++.dg/cpp1z/class-deduction92b.C | 22 +++++++++++++++
> > >>> .../g++.dg/cpp1z/class-deduction93.C | 25 +++++++++++++++++
> > >>> .../g++.dg/cpp1z/class-deduction93a.C | 27
> > >>> +++++++++++++++++++
> > >>> 6 files changed, 126 insertions(+), 2 deletions(-)
> > >>> create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction92.C
> > >>> create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction92a.C
> > >>> create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction92b.C
> > >>> create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction93.C
> > >>> create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction93a.C
> > >>>
> > >>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > >>> index f2039e09cd7..db769d59951 100644
> > >>> --- a/gcc/cp/pt.c
> > >>> +++ b/gcc/cp/pt.c
> > >>> @@ -9954,7 +9954,12 @@ lookup_template_class_1 (tree d1, tree arglist,
> > >>> tree in_decl, tree context,
> > >>> template-arguments for the template-parameters in the
> > >>> type-id of the alias template. */
> > >>>
> > >>> - t = tsubst (TREE_TYPE (gen_tmpl), arglist, complain, in_decl);
> > >>> + /* When substituting a dependent alias template specialization,
> > >>> + we pass tf_partial to avoid lowering the level of any 'auto's
> > >>> + within its type-id (91911). */
> > >>> + t = tsubst (TREE_TYPE (gen_tmpl), arglist,
> > >>> + complain | (tf_partial * is_dependent_type),
> > >>> + in_decl);
> > >>> /* Note that the call above (by indirectly calling
> > >>> register_specialization in tsubst_decl) registers the
> > >>> TYPE_DECL representing the specialization of the alias
> > >>> @@ -15544,9 +15549,15 @@ tsubst (tree t, tree args, tsubst_flags_t
> > >>> complain, tree in_decl)
> > >>> gcc_assert (TREE_VEC_LENGTH (args) > 0);
> > >>> template_parm_level_and_index (t, &level, &idx);
> > >>>
> > >>> + /* Retrieve the argument for this template parameter. */
> > >>> levels = TMPL_ARGS_DEPTH (args);
> > >>> if (level <= levels
> > >>> - && TREE_VEC_LENGTH (TMPL_ARGS_LEVEL (args, level)) > 0)
> > >>> + && TREE_VEC_LENGTH (TMPL_ARGS_LEVEL (args, level)) > 0
> > >>> + /* Avoid substituting CTAD placeholders; they get
> > >>> + resolved only during CTAD proper. We can get here
> > >>> + after dependent substitution of an alias template
> > >>> + whose defining-type-id uses CTAD (91911). */
> > >>> + && !template_placeholder_p (t))
> > >>> {
> > >>> arg = TMPL_ARG (args, level, idx);
> > >>>
> > >>> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction92.C
> > >>> b/gcc/testsuite/g++.dg/cpp1z/class-deduction92.C
> > >>> new file mode 100644
> > >>> index 00000000000..379eb960da6
> > >>> --- /dev/null
> > >>> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction92.C
> > >>> @@ -0,0 +1,17 @@
> > >>> +// PR c++/91911
> > >>> +// { dg-do compile { target c++17 } }
> > >>> +
> > >>> +template<class T>
> > >>> +struct span {
> > >>> + using value_type = T;
> > >>> + span(T);
> > >>> +};
> > >>> +
> > >>> +template<class T>
> > >>> +using SpanType = decltype(span{T()});
> > >>> +
> > >>> +template<class T>
> > >>> +using ConstSpanType = span<const typename SpanType<T>::value_type>;
> > >>> +
> > >>> +using type = ConstSpanType<int>;
> > >>> +using type = span<const int>;
> > >>> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction92a.C
> > >>> b/gcc/testsuite/g++.dg/cpp1z/class-deduction92a.C
> > >>> new file mode 100644
> > >>> index 00000000000..b9aa8f3bbf0
> > >>> --- /dev/null
> > >>> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction92a.C
> > >>> @@ -0,0 +1,22 @@
> > >>> +// PR c++/91911
> > >>> +// { dg-do compile { target c++17 } }
> > >>> +// A variant of class-deduction92.C where SpanType has more levels than
> > >>> +// ConstSpanType.
> > >>> +
> > >>> +template<class T>
> > >>> +struct span {
> > >>> + using value_type = T;
> > >>> + span(T);
> > >>> +};
> > >>> +
> > >>> +template<class>
> > >>> +struct A {
> > >>> + template<class T>
> > >>> + using SpanType = decltype(span{T()});
> > >>> +};
> > >>> +
> > >>> +template<class T>
> > >>> +using ConstSpanType = span<const typename A<int>::SpanType<const
> > >>> T>::value_type>;
> > >>> +
> > >>> +using type = ConstSpanType<int>;
> > >>> +using type = span<const int>;
> > >>> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction92b.C
> > >>> b/gcc/testsuite/g++.dg/cpp1z/class-deduction92b.C
> > >>> new file mode 100644
> > >>> index 00000000000..0ea0cef0238
> > >>> --- /dev/null
> > >>> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction92b.C
> > >>> @@ -0,0 +1,22 @@
> > >>> +// PR c++/91911
> > >>> +// { dg-do compile { target c++17 } }
> > >>> +// A variant of class-deduction92.C where SpanType has fewer levels
> > >>> than
> > >>> +// ConstSpanType.
> > >>> +
> > >>> +template<class T>
> > >>> +struct span {
> > >>> + using value_type = T;
> > >>> + span(T);
> > >>> +};
> > >>> +
> > >>> +template<class T>
> > >>> +using SpanType = decltype(span{T()});
> > >>> +
> > >>> +template<class>
> > >>> +struct B {
> > >>> + template<class T>
> > >>> + using ConstSpanType = span<const typename SpanType<T>::value_type>;
> > >>> +};
> > >>> +
> > >>> +using type = B<int>::ConstSpanType<int>;
> > >>> +using type = span<const int>;
> > >>> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction93.C
> > >>> b/gcc/testsuite/g++.dg/cpp1z/class-deduction93.C
> > >>> new file mode 100644
> > >>> index 00000000000..20504780d32
> > >>> --- /dev/null
> > >>> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction93.C
> > >>> @@ -0,0 +1,25 @@
> > >>> +// PR c++/98077
> > >>> +// { dg-do compile { target c++17 } }
> > >>> +
> > >>> +template<class R>
> > >>> +struct function {
> > >>> + template<class T> function(T);
> > >>> + using type = R;
> > >>> +};
> > >>> +
> > >>> +template<class T> function(T) -> function<decltype(T()())>;
> > >>> +
> > >>> +template<class T>
> > >>> +struct CallableTrait;
> > >>> +
> > >>> +template<class R>
> > >>> +struct CallableTrait<function<R>> { using ReturnType = R; };
> > >>> +
> > >>> +template<class F>
> > >>> +using CallableTraitT = CallableTrait<decltype(function{F()})>;
> > >>> +
> > >>> +template<class F>
> > >>> +using ReturnType = typename CallableTraitT<F>::ReturnType;
> > >>> +
> > >>> +using type = ReturnType<int(*)()>;
> > >>> +using type = int;
> > >>> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction93a.C
> > >>> b/gcc/testsuite/g++.dg/cpp1z/class-deduction93a.C
> > >>> new file mode 100644
> > >>> index 00000000000..7656e7845fe
> > >>> --- /dev/null
> > >>> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction93a.C
> > >>> @@ -0,0 +1,27 @@
> > >>> +// PR c++/98077
> > >>> +// { dg-do compile { target c++17 } }
> > >>> +// A variant of class-deduction93.C where the template placeholder is
> > >>> a template
> > >>> +// template parameter.
> > >>> +
> > >>> +template<class R>
> > >>> +struct function {
> > >>> + template<class T> function(T);
> > >>> + using type = R;
> > >>> +};
> > >>> +
> > >>> +template<class T> function(T) -> function<decltype(T()())>;
> > >>> +
> > >>> +template<class T>
> > >>> +struct CallableTrait;
> > >>> +
> > >>> +template<class R>
> > >>> +struct CallableTrait<function<R>> { using ReturnType = R; };
> > >>> +
> > >>> +template<class F, template<class> class Tmpl>
> > >>> +using CallableTraitT = CallableTrait<decltype(Tmpl{F()})>;
> > >>> +
> > >>> +template<class F, template<class> class Tmpl>
> > >>> +using ReturnType = typename CallableTraitT<F, Tmpl>::ReturnType;
> > >>> +
> > >>> +using type = ReturnType<int(*)(), function>;
> > >>> +using type = int;
> > >>>
> > >>
> > >
> >
