On Mon, 13 Jan 2025, Jason Merrill wrote:
> On 1/13/25 3:27 PM, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > for trunk?
>
> OK, but do we also need this in the "still packed" case earlier in the
> function?
I think so in principle, though I wasn't able to produce a testcase for
that code path due to PR86883, see comment #7 there for my attempt.
And unfortunately I missed a regression in g++.dg/gcov/pr88045.C when
testing this patch, which reveals that we don't want to strip
injected-class-name typedefs at this point because they are handled
specially during maybe_get_template_decl_from_type_decl from
convert_template_argument, so the typedef must be preserved until
coercion at instantiation time.
To that end I added a new flag to strip_typedefs that controls whether
to preserve injected-class-names. AFAICT we could instead preserve
injected-class-name typedefs always for all callers (though then we need
a small adjustment in maybe_dependent_member_ref to handle them directly
to avoid regression on class-deduction109.C) but this flag approach is
definitely safer.
-- >8 --
Subject: [PATCH v2] c++: pack expansion arg vs non-pack parm checking ICE
[PR118454]
During ahead of time template argument coercion, we handle the case of
passing a pack expansion to a non-pack parameter by breaking out early
and using the original unconverted arguments, deferring coercion until
instantiation time where we have concrete arguments.
This PR reveals we still need to strip typedefs from the original
arguments as in the ordinary case, for sake of our template argument
hashing/equivalence routines which assume template arguments went
through strip_typedefs.
At this point however we need to preserve injected-class-name typedefs
because we use them to distinguish passing an injected-class-name vs
the corresponding specialization as the argument to a template template
parameter (the former is valid, the latter isn't).
PR c++/118454
gcc/cp/ChangeLog:
* cp-tree.h (STF_KEEP_INJ_CLASS_NAME): Define.
* pt.cc (iterative_hash_template_argument) <case tcc_type>:
Clarify comment for when we'd see an alias template
specialization here.
(coerce_template_parms): Strip typedefs (besides
injected-class-names) in the pack expansion early break cases
that use the unconverted arguments.
* tree.cc (strip_typedefs): Don't strip an injected-class-name
if STF_KEEP_INJ_CLASS_NAME is set.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/variadic187.C: New test.
---
gcc/cp/cp-tree.h | 7 ++++++-
gcc/cp/pt.cc | 12 +++++++-----
gcc/cp/tree.cc | 7 ++++++-
gcc/testsuite/g++.dg/cpp0x/variadic187.C | 13 +++++++++++++
4 files changed, 32 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic187.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1b42e8ba7d8..1cfea7b78ab 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6311,9 +6311,14 @@ enum auto_deduction_context
STF_STRIP_DEPENDENT: allow the stripping of aliases with dependent
template parameters, relying on code elsewhere to report any
- appropriate diagnostics. */
+ appropriate diagnostics.
+
+ STF_KEEP_INJ_CLASS_NAME: don't strip injected-class-name typedefs
+ because we're dealing with a non-coerced template argument.
+*/
const unsigned int STF_USER_VISIBLE = 1U;
const unsigned int STF_STRIP_DEPENDENT = 1U << 1;
+const unsigned int STF_KEEP_INJ_CLASS_NAME = 1U << 2;
/* Returns the TEMPLATE_DECL associated to a TEMPLATE_TEMPLATE_PARM
node. */
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 537e4c4a494..48440420858 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -1913,9 +1913,7 @@ iterative_hash_template_arg (tree arg, hashval_t val)
// to hash differently from its TYPE_CANONICAL, to avoid hash
// collisions that compare as different in template_args_equal.
// These could be dependent specializations that strip_typedefs
- // left alone, or untouched specializations because
- // coerce_template_parms returns the unconverted template
- // arguments if it sees incomplete argument packs.
+ // left alone for example.
tree ti = TYPE_ALIAS_TEMPLATE_INFO (ats);
return hash_tmpl_and_args (TI_TEMPLATE (ti), TI_ARGS (ti));
}
@@ -9306,7 +9304,9 @@ coerce_template_parms (tree parms,
/* We don't know how many args we have yet, just use the
unconverted (and still packed) ones for now. */
ggc_free (new_inner_args);
- new_inner_args = orig_inner_args;
+ new_inner_args = strip_typedefs (orig_inner_args,
+ /*remove_attrs=*/nullptr,
+ STF_KEEP_INJ_CLASS_NAME);
arg_idx = nargs;
break;
}
@@ -9362,7 +9362,9 @@ coerce_template_parms (tree parms,
/* We don't know how many args we have yet, just
use the unconverted (but unpacked) ones for now. */
ggc_free (new_inner_args);
- new_inner_args = inner_args;
+ new_inner_args = strip_typedefs (inner_args,
+ /*remove_attrs=*/nullptr,
+ STF_KEEP_INJ_CLASS_NAME);
arg_idx = nargs;
break;
}
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index dd6e872e4e7..7247d488f6f 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -1566,7 +1566,7 @@ apply_identity_attributes (tree result, tree attribs,
bool *remove_attributes)
/* Builds a qualified variant of T that is either not a typedef variant
(the default behavior) or not a typedef variant of a user-facing type
- (if FLAGS contains STF_USER_FACING). If T is not a type, then this
+ (if FLAGS contains STF_USER_VISIBLE). If T is not a type, then this
just dispatches to strip_typedefs_expr.
E.g. consider the following declarations:
@@ -1613,6 +1613,11 @@ strip_typedefs (tree t, bool *remove_attributes /* =
NULL */,
&& !user_facing_original_type_p (t))
return t;
+ if ((flags & STF_KEEP_INJ_CLASS_NAME)
+ && CLASS_TYPE_P (t)
+ && DECL_SELF_REFERENCE_P (TYPE_NAME (t)))
+ return t;
+
if (dependent_opaque_alias_p (t))
return t;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic187.C
b/gcc/testsuite/g++.dg/cpp0x/variadic187.C
new file mode 100644
index 00000000000..af1770e4d89
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic187.C
@@ -0,0 +1,13 @@
+// PR c++/118454
+// { dg-do compile { target c++11 } }
+// { dg-additional-options --param=hash-table-verification-limit=1000 }
+
+template<class T> using identity = T;
+
+template<class T, class U0, class... Us> struct dual;
+
+template<class T, class... Ts>
+using ty1 = dual<identity<T>, Ts...>;
+
+template<class T, class... Ts>
+using ty2 = dual<T, Ts...>;
--
2.48.0.rc1.35.g1b4e9a5f8b
-- >8 --
>
> > -- >8 --
> >
> > During ahead of time template argument coercion, we handle the case of
> > passing a pack expansion to a non-pack parameter by breaking out early
> > and using the original unconverted arguments, deferring coercion until
> > instantiation time where we have concrete arguments. We still however
> > need to strip typedefs from the original arguments as in the ordinary
> > case, for sake of our template argument hashing/equivalence routines
> > which assume template arguments went through strip_typedefs.
> >
> > PR c++/118454
> >
> > gcc/cp/ChangeLog:
> >
> > * pt.cc (coerce_template_parms): Strip typedefs in the pack
> > expansion arg vs non-pack parm early break special case.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp0x/variadic187.C: New test.
> > ---
> > gcc/cp/pt.cc | 2 +-
> > gcc/testsuite/g++.dg/cpp0x/variadic187.C | 13 +++++++++++++
> > 2 files changed, 14 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic187.C
> >
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 537e4c4a494..8cdbf7f65ac 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -9362,7 +9362,7 @@ coerce_template_parms (tree parms,
> > /* We don't know how many args we have yet, just
> > use the unconverted (but unpacked) ones for now. */
> > ggc_free (new_inner_args);
> > - new_inner_args = inner_args;
> > + new_inner_args = strip_typedefs (inner_args);
> > arg_idx = nargs;
> > break;
> > }
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic187.C
> > b/gcc/testsuite/g++.dg/cpp0x/variadic187.C
> > new file mode 100644
> > index 00000000000..af1770e4d89
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/variadic187.C
> > @@ -0,0 +1,13 @@
> > +// PR c++/118454
> > +// { dg-do compile { target c++11 } }
> > +// { dg-additional-options --param=hash-table-verification-limit=1000 }
> > +
> > +template<class T> using identity = T;
> > +
> > +template<class T, class U0, class... Us> struct dual;
> > +
> > +template<class T, class... Ts>
> > +using ty1 = dual<identity<T>, Ts...>;
> > +
> > +template<class T, class... Ts>
> > +using ty2 = dual<T, Ts...>;
>
>
