On 11/7/20 10:59 AM, Patrick Palka wrote:
On Fri, 6 Nov 2020, Jason Merrill wrote:
On 11/5/20 8:40 PM, Patrick Palka wrote:
This patch (naively) extends the PR93907 fix to also apply to variadic
concepts invoked with a type argument pack. Without this, we ICE on
the below testcase (a variadic version of concepts-using2.C) in the same
manner as we used to on concepts-using2.C before r10-7133.
Patch series bootstrapped and regtested on x86_64-pc-linux-gnu,
and also tested against cmcstl2 and range-v3.
gcc/cp/ChangeLog:
PR c++/93907
* constraint.cc (tsubst_parameter_mapping): Also canonicalize
the type arguments of a TYPE_ARGUMENT_PACk.
gcc/testsuite/ChangeLog:
PR c++/93907
* g++.dg/cpp2a/concepts-using3.C: New test, based off of
concepts-using2.C.
---
gcc/cp/constraint.cc | 10 ++++
gcc/testsuite/g++.dg/cpp2a/concepts-using3.C | 52 ++++++++++++++++++++
2 files changed, 62 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-using3.C
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index b6f6f0d02a5..c871a8ab86a 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2252,6 +2252,16 @@ tsubst_parameter_mapping (tree map, tree args,
subst_info info)
Hmm, the
else if (ARGUMENT_PACK_P (arg))
new_arg = tsubst_argument_pack (arg, args, complain, in_decl);
just above this seems redundant, since tsubst_template_arg handles packs just
fine. In fact, I wonder why tsubst_argument_pack is used specifically
anywhere? It seems to get some edge cases better than the code in tsubst, but
the solution to that would seem to be replacing the code in tsubst with a call
to tsubst_argument_pack; then we can remove all the other calls to the
function.
They seem interchangeable here wrt handling TYPE_ARGUMENT_PACKs, but not
NONTYPE_ARGUMENT_PACKs. It looks like tsubst_template_arg ends up just
issuing an error from tsubst_expr if we try using it to substitute into
a NONTYPE_ARGUMENT_PACK.
new_arg = tsubst_template_arg (arg, args, complain, in_decl);
if (TYPE_P (new_arg))
new_arg = canonicalize_type_argument (new_arg, complain);
+ if (TREE_CODE (new_arg) == TYPE_ARGUMENT_PACK)
+ {
+ tree pack_args = ARGUMENT_PACK_ARGS (new_arg);
+ for (int i = 0; i < TREE_VEC_LENGTH (pack_args); i++)
+ {
+ tree& pack_arg = TREE_VEC_ELT (pack_args, i);
+ if (TYPE_P (pack_arg))
+ pack_arg = canonicalize_type_argument (pack_arg,
complain);
Do we need the TYPE_P here, since we already know we're in a
TYPE_ARGUMENT_PACK? That is, can an element of a TYPE_ARGUMENT_PACK be an
invalid argument to canonicalize_type_argument?
With -fconcepts-ts, the elements of a TYPE_ARGUMENT_PACK here can be
TEMPLATE_DECLs, as in e.g. line 28 of concepts/template-parm3.C.
OTOH, I wonder if we need to canonicalize non-type arguments here as well?
Hmm, I'm not sure. Not doing so should at worst result in a
satisfaction cache miss in release builds, and in checking builds should
get caught by the hash table sanitizer. I haven't been able to come up
with a testcase that demonstrates it's necessary.
I wonder if tsubst_template_arg should canonicalize rather than leave that up
to the caller? I suppose that could do a bit more work when the result is
going to end up in convert_template_argument and get canonicalized again; I
don't know if that would be significant.
That seems like it works, based on some limited testing. But there are
only two users of canonicalize_template_argument outside of
convert_template_argument itself, and the one use in unify is still
needed even with this change (or else we get many ICEs coming from
verify_unstripped_args if we try to remove it). So the benefit of such
a change seems marginal at the moment.
Then the patch is OK as is.
}
if (new_arg == error_mark_node)
return error_mark_node;
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-using3.C
b/gcc/testsuite/g++.dg/cpp2a/concepts-using3.C
new file mode 100644
index 00000000000..2c8ad40d104
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-using3.C
@@ -0,0 +1,52 @@
+// PR c++/93907
+// { dg-options -std=gnu++20 }
+
+// This testcase is a variadic version of concepts-using2.C; the only
+// difference is that 'cd' and 'ce' are now variadic concepts.
+
+template <int a> struct c {
+ static constexpr int d = a;
+ typedef c e;
+};
+template <typename> struct f;
+template <typename b> using g = typename f<b>::e;
+struct b;
+template <typename b> struct f { using e = b; };
+template <typename ai> struct m { typedef g<ai> aj; };
+template <typename b> struct n { typedef typename m<b>::aj e; };
+template <typename b> using an = typename n<b>::e;
+template <typename> constexpr bool ao = c<true>::d;
+template <typename> constexpr bool i = c<1>::d;
+template <typename> concept bb = i<b>;
+#ifdef __SIZEOF_INT128__
+using cc = __int128;
+#else
+using cc = long long;
+#endif
+template <typename...> concept cd = bb<cc>;
+template <typename... bt> concept ce = requires { requires cd<bt...>; };
+template <typename bt> concept h = ce<bt>;
+template <typename bt> concept l = h<bt>;
+template <typename> concept cl = ao<b>;
+template <typename b> concept cp = requires(b j) {
+ requires h<an<decltype(j.begin())>>;
+};
+struct o {
+ template <cl b> requires cp<b> auto operator()(b) {}
+};
+template <typename b> using cm = decltype(o{}(b()));
+template <typename bt> concept ct = l<bt>;
+template <typename da> concept dd = ct<cm<da>>;
+template <typename da> concept de = dd<da>;
+struct {
+ template <de da, typename b> void operator()(da, b);
+} di;
+struct p {
+ void begin();
+};
+template <typename> using df = p;
+template <int> void q() {
+ df<int> k;
+ int d;
+ di(k, d);
+}
