On 1/19/22 17:32, Patrick Palka wrote:
On Wed, 19 Jan 2022, Jason Merrill wrote:

On 1/3/22 10:24, Patrick Palka wrote:
On Wed, 22 Dec 2021, Jason Merrill wrote:

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.

Ah yeah, I see what you mean...


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.

Interesting.  In light of this general problem, I wonder if representing
autos as template parameters with one level greater than the template
depth, while convenient, is ultimately not the best approach?

Back to the original issue concerning CTAD within alias templates,
here's an approach that seems to work well.  The idea is to treat
CTAD placeholders less like template parameters, by giving them
the special level 0, and by making tsubst avoid substituting them
like template parameters.  With this approach the representation
of a CTAD placeholder no longer depends on the template depth, so
alias template transparency for alias templates that contain CTAD
just works.

I tried extending this approach to all autos (i.e. giving all
autos a level of 0 and adjusting tsubst accordingly), and it nearly
works modulo handling concepts TS auto... deduction:

    tuple<auto...> x = tuple<int, char>{}.

since unify_pack_expansion isn't prepared to see a parameter pack of
level 0.  This is likely fixable, but I suppose it'd be good to first
get confirmation that this is a good approach before continuing down
this patch.

Below the patch that implements this approach only for CTAD
placeholders.  Attached is an incremental WIP diff that additionally
extends the approach to all autos, which passes testing modulo the
concept TS thing.

-- >8 --

Subject: [PATCH] c++: CTAD inside 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 its instantiated definition.  But this
instantiation 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.

Although we represent CTAD placeholders as template parameters, we never
actually want to replace them via tsubst.  So this patch adjusts tsubst
to handle CTAD placeholders by simply substituting the template and
returning a new CTAD placeholder.  Moreover, this means that the level
of a CTAD placeholder doesn't matter, so we may as well give them all
the same level.  This patch gives them the special level 0.

The change in grokdeclarator makes us reject an invalid function
return type consisting of a CTAD placeholder sooner (as in pr88187.C),
which helps guarantee that splice_late_return_type doesn't see or need
to handle a erroneous CTAD placeholder return type.

The change in tsubst_decl removes a CHECKING_P workaround added in 2017,
which would otherwise now get triggered for variables with CTAD placeholder
types (since their level is 0).  Alternatively, we could just guard the
workaround with !template_placeholder_p if that's preferable.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

        PR c++/91911

gcc/cp/ChangeLog:

        * decl.c (grokdeclarator): Diagnose CTAD placeholder in function
        return type even when !funcdecl_p.
        * pt.c (keep_template_parm): Punt on a level 0 template parm.
        (tsubst_decl) <case VAR_DECL>: Remove CHECKING_P workaround.
        (tsubst) <case TEMPLATE_TYPE_PARM>: Handle CTAD placeholders
        specially.
        (make_auto_1): Add defaulted 'level' parameter.
        (make_template_placeholder): Pass 0 as 'level' to make_auto_1.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp1z/class-deduction100.C: New test.
        * g++.dg/cpp1z/class-deduction100a.C: New test.
        * g++.dg/cpp1z/class-deduction100b.C: New test.
        * g++.dg/cpp1z/class-deduction101.C: New test.
        * g++.dg/cpp1z/class-deduction101a.C: New test.
        * g++.dg/cpp1z/class-deduction101b.C: New test.
---
   gcc/cp/decl.c                                 |  6 +-
   gcc/cp/pt.c                                   | 60 ++++++++-----------
   .../g++.dg/cpp1z/class-deduction100.C         | 17 ++++++
   .../g++.dg/cpp1z/class-deduction100a.C        | 22 +++++++
   .../g++.dg/cpp1z/class-deduction100b.C        | 22 +++++++
   .../g++.dg/cpp1z/class-deduction101.C         | 25 ++++++++
   .../g++.dg/cpp1z/class-deduction101a.C        | 27 +++++++++
   .../g++.dg/cpp1z/class-deduction101b.C        | 30 ++++++++++
   gcc/testsuite/g++.dg/other/pr88187.C          |  2 +-
   9 files changed, 173 insertions(+), 38 deletions(-)
   create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction100.C
   create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction100a.C
   create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction100b.C
   create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction101.C
   create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction101a.C
   create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction101b.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 0b71c00f5ab..406a9163ffd 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -12635,11 +12635,11 @@ grokdeclarator (const cp_declarator *declarator,
                if (!tmpl)
                  if (tree late_auto = type_uses_auto (late_return_type))
                    tmpl = CLASS_PLACEHOLDER_TEMPLATE (late_auto);
-               if (tmpl && funcdecl_p)
+               if (tmpl)
                  {
-                   if (!dguide_name_p (unqualified_id))
+                   if (!funcdecl_p || !dguide_name_p (unqualified_id))
                      {
-                       error_at (declarator->id_loc, "deduced class "
+                       error_at (typespec_loc, "deduced class "
                                  "type %qD in function return type",
                                  DECL_NAME (tmpl));
                        inform (DECL_SOURCE_LOCATION (tmpl),

This seems like it could go in separately.

Like so?

Yes (OK).

-- >8

Subject: [PATCH] c++: consistently diagnose bare CTAD placeholder as fn return
  type

Relax slightly the existing code for diagnosing a bare CTAD placeholder
as the return type of a function declarator to also handle the abstract
declarator case.

gcc/cp/ChangeLog:

        * decl.cc (grokdeclarator): Diagnose CTAD placeholder in
        function return type even when !funcdecl_p.

gcc/testsuite/ChangeLog:

        * g++.dg/other/pr88187.C: Adjust expected diagnostic accordingly.
---
  gcc/cp/decl.cc                       | 6 +++---
  gcc/testsuite/g++.dg/other/pr88187.C | 2 +-
  2 files changed, 4 insertions(+), 4 deletions(-)

...

Isn't this code still needed for non-CTAD autos?

Probably only in theory -- the safeguard was added over 4 years ago so I
presume it's dead code by now.  Shall we keep it and guard it with
!template_placeholder_p (auto_node) instead?

Ah, I didn't read closely enough to see that we were already asserting that it was dead code. Go ahead and remove it, then; the patch is OK.

Jason

Reply via email to