From: Jason Merrill <jason@redhat.com>
To: Patrick Palka <ppalka@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: CTAD within alias template [PR91911]
Date: Wed, 30 Jun 2021 15:50:58 -0400 [thread overview]
Message-ID: <7f944b48-7c8f-755a-8cea-0880cbda4ccc@redhat.com> (raw)
In-Reply-To: <67855539-bf99-ac7-a995-35871cd23712@idea>
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?
> 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;
>
next prev parent reply other threads:[~2021-06-30 19:51 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-24 20:45 Patrick Palka
2021-06-25 15:42 ` Jason Merrill
2021-06-25 17:11 ` Patrick Palka
2021-06-25 20:24 ` Jason Merrill
2021-06-30 15:55 ` Patrick Palka
2021-06-30 15:58 ` Patrick Palka
2021-06-30 19:50 ` Jason Merrill [this message]
2021-06-30 20:18 ` Patrick Palka
2021-06-30 20:23 ` Jason Merrill
2021-12-21 19:03 ` Patrick Palka
2021-12-21 19:08 ` Patrick Palka
2021-12-22 5:22 ` Jason Merrill
2022-01-03 15:24 ` Patrick Palka
2022-01-06 18:25 ` Patrick Palka
2022-01-19 16:08 ` Patrick Palka
2022-01-19 19:26 ` Jason Merrill
2022-01-19 22:32 ` Patrick Palka
2022-01-20 3:35 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7f944b48-7c8f-755a-8cea-0880cbda4ccc@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ppalka@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).