On 12/21/21 14:08, Patrick Palka wrote:
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 nonpositive level.
- 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'
+ in its type-id which might correspond to CTAD placeholders. */
+ t = tsubst (TREE_TYPE (gen_tmpl), arglist,
+ complain | (is_dependent_type * tf_partial),
+ in_decl);
So, if we aren't changing any containing template scopes from dependent
to non-dependent, we don't want to mess with levels.
I think is_dependent_type is a bit too broad here; I'd expect this could
cause trouble when e.g. instantiating a class A<int> with a member
template B and we have both B<U> and an auto in the signature of a
member template. I think what we want to check is whether the outermost
args are dependent.
It would also be safer to handle adding tf_partial for alias templates
in instantiate_alias_template rather than lookup_template_class.
Perhaps in alias_ctad_tweaks as well.
I tried adding an assert that tf_partial is set any time we see
dependent outermost args; I expected to need to override that for
deduction guide rewriting, but also seem to hit problems in concepts and
TTP. Attached in case you're interested; I don't think this is going to
become a patch suitable for GCC 12. The use of
uses_template_parms_level was a kludge because dependent_template_arg_p
returns true for null args.
Jason
From 40d663979886285e47a2149507e02cb052c221be Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Tue, 21 Dec 2021 17:28:12 -0500
Subject: [PATCH 1/4] assert
To: gcc-patches@gcc.gnu.org
---
gcc/cp/cp-tree.h | 1 +
gcc/cp/pt.c | 15 ++++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 5fc9e5efdab..a1edba2123a 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5515,6 +5515,7 @@ enum tsubst_flags {
(build_target_expr and friends) */
tf_norm = 1 << 11, /* Build diagnostic information during
constraint normalization. */
+ tf_ctad = 1 << 12, /* Building a deduction guide. */
/* Convenient substitution flags combinations. */
tf_warning_or_error = tf_warning | tf_error
};
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index a115e1d128c..ef83588e2a9 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -15708,6 +15708,10 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
if (complain & tf_partial)
return t;
+ if (!(complain & tf_ctad)
+ && uses_template_parms_level (TMPL_ARGS_LEVEL (args, 1), 1))
+ gcc_assert (false);
+
/* If we get here, we must have been looking at a parm for a
more deeply nested template. Make a new version of this
template parameter, but with a lower level. */
@@ -21545,6 +21549,9 @@ instantiate_alias_template (tree tmpl, tree args, tsubst_flags_t complain)
/*require_all_args=*/true,
/*use_default_args=*/true);
+ if (args == error_mark_node)
+ return error_mark_node;
+
/* FIXME check for satisfaction in check_instantiated_args. */
if (flag_concepts
&& !any_dependent_template_arguments_p (args)
@@ -21561,6 +21568,10 @@ instantiate_alias_template (tree tmpl, tree args, tsubst_flags_t complain)
if (!push_tinst_level (tmpl, args))
return error_mark_node;
+
+ if (uses_template_parms_level (TMPL_ARGS_LEVEL (args, 1), 1))
+ complain |= tf_partial;
+
tree r = instantiate_template (tmpl, args, complain);
pop_tinst_level ();
@@ -29026,6 +29037,8 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com
location_t loc;
tree fn_tmpl = NULL_TREE;
+ complain |= tf_ctad;
+
if (outer_args)
{
++processing_template_decl;
@@ -29427,7 +29440,7 @@ alias_ctad_tweaks (tree tmpl, tree uguides)
constraint we don't need to deduce them again, we can just check whether
the deduction produced the desired result. */
- tsubst_flags_t complain = tf_warning_or_error;
+ tsubst_flags_t complain = tf_warning_or_error | tf_partial;
tree atype = TREE_TYPE (tmpl);
tree aguides = NULL_TREE;
tree atparms = INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS (tmpl));
--
2.27.0
