On Mon, 30 Mar 2020, Patrick Palka wrote:
> On Thu, 26 Mar 2020, Jason Merrill wrote:
>
> > On 3/22/20 9:21 PM, Patrick Palka wrote:
> > > This patch relaxes an assertion in tsubst_default_argument that exposes a
> > > latent
> > > bug in how we substitute an array type into a cv-qualified wildcard
> > > function
> > > parameter type. Concretely, the latent bug is that given the function
> > > template
> > >
> > > template<typename T> void foo(const T t);
> > >
> > > one would expect the type of foo<int[]> to be void(const int*), but we
> > > (seemingly prematurely) strip function parameter types of their top-level
> > > cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead end
> > > up
> > > obtaining void(int*) as the type of foo<int[]> after substitution and
> > > decaying.
> > >
> > > We still however correctly substitute into and decay the formal parameter
> > > type,
> > > obtaining const int* as the type of t after substitution. But this then
> > > leads
> > > to us tripping over the assert in tsubst_default_argument that verifies
> > > the
> > > formal parameter type and the function type are consistent.
> > >
> > > Assuming it's too late at this stage to fix the substitution bug, we can
> > > still
> > > relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this
> > > look
> > > OK?
> >
> > This is core issues 1001/1322, which have not been resolved. Clang does the
> > substitution the way you suggest; EDG rejects the testcase because the two
> > substitutions produce different results. I think it would make sense to
> > follow the EDG behavior until this issue is actually resolved.
>
> Here is what I have so far towards that end. When substituting into the
> PARM_DECLs of a function decl, we now additionally check if the
> aforementioned Core issues are relevant and issue a (fatal) diagnostic
> if so. This patch checks this in tsubst_decl <case PARM_DECL> rather
> than in tsubst_function_decl for efficiency reasons, so that we don't
> have to perform another traversal over the DECL_ARGUMENTS /
> TYPE_ARG_TYPES just to implement this check.
>
> Is something like this what you have in mind?
Oops, noticed a few bugs in the patch as soon as I hit send. Please
consider this patch instead, which compared to the previous patch
declares unqual_expanded_types in the correct scope and refrains from
doing a potentially incorrect CSE of "cp_type_quals (type)".
-- >8 --
gcc/cp/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* pt.c (tsubst_decl) <case PARM_DECL>: Detect and reject the case where
the function type depends on whether we strip top-level qualifiers from
the type of T before or after substitution.
gcc/testsuite/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* g++.dg/template/array33.C: New test.
* g++.dg/template/array34.C: New test.
---
gcc/cp/pt.c | 68 +++++++++++++++++++++++++
gcc/testsuite/g++.dg/template/array33.C | 39 ++++++++++++++
gcc/testsuite/g++.dg/template/array34.C | 63 +++++++++++++++++++++++
3 files changed, 170 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/template/array33.C
create mode 100644 gcc/testsuite/g++.dg/template/array34.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 8564eb11df4..a1efaef2c1e 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14046,6 +14046,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain)
}
}
+ tree unqual_expanded_types = NULL_TREE;
+
/* Loop through all of the parameters we'll build. When T is
a function parameter pack, LEN is the number of expanded
types in EXPANDED_TYPES; otherwise, LEN is 1. */
@@ -14072,6 +14074,72 @@ tsubst_decl (tree t, tree args, tsubst_flags_t
complain)
/* We're dealing with a normal parameter. */
type = tsubst (TREE_TYPE (t), args, complain, in_decl);
+ /* Determine whether the function type after substitution into the
+ type of the function parameter T depends on the resolution of
+ Core issues 1001/1322, which have not been resolved. In
+ particular, the following detects when the resulting function
+ type depends on whether we strip top-level qualifiers from the
+ type of T before or after substitution.
+
+ This can happen only when the dependent type of T is a
+ cv-qualified wildcard type, and substitution yields a
+ (cv-qualified) array type before array-to-pointer conversion. */
+ if (TREE_CODE (type) == ARRAY_TYPE
+ && cv_qualified_p (type)
+ && DECL_FUNCTION_SCOPE_P (t)
+ && !DECL_TEMPLATE_PARM_P (t))
+ {
+ tree type_pattern = (PACK_EXPANSION_P (TREE_TYPE (t))
+ ? PACK_EXPANSION_PATTERN (TREE_TYPE (t))
+ : TREE_TYPE (t));
+ if (WILDCARD_TYPE_P (type_pattern)
+ && cv_qualified_p (type_pattern))
+ {
+ /* Substitute into the corresponding wildcard type that is
+ stripped of its own top-level cv-qualifiers. */
+ tree unqual_type;
+ if (PACK_EXPANSION_P (TREE_TYPE (t)))
+ {
+ if (unqual_expanded_types == NULL_TREE)
+ {
+ tree unqual_pack_expansion
+ = copy_node (TREE_TYPE (t));
+ PACK_EXPANSION_PATTERN (unqual_pack_expansion)
+ = cv_unqualified (type_pattern);
+ unqual_expanded_types
+ = tsubst_pack_expansion (unqual_pack_expansion,
+ args, tf_none, in_decl);
+ }
+ unqual_type = TREE_VEC_ELT (unqual_expanded_types, i);
+ }
+ else
+ {
+ tree unqual_type_pattern = cv_unqualified
(type_pattern);
+ unqual_type = tsubst (unqual_type_pattern, args,
+ tf_none, in_decl);
+ }
+ /* Check if the top-level cv-qualifiers on the wildcard type
+ make a difference in the resulting type. */
+ int type_quals = cp_type_quals (type);
+ int unqual_type_quals = cp_type_quals (unqual_type);
+ if (type_quals & ~unqual_type_quals
+ & (TYPE_QUAL_CONST|TYPE_QUAL_VOLATILE))
+ {
+ sorry ("the type of function %qD after substitution "
+ "depends on the resolution of Core issues "
+ "1001 and 1332",
+ DECL_NAME (DECL_CONTEXT (t)));
+ if (PACK_EXPANSION_P (TREE_TYPE (t)))
+ inform (input_location, "when substituting into the "
+ "function parameter pack %q#D", t);
+ else
+ inform (input_location, "when substituting into the "
+ "function parameter %q#D", t);
+ type = error_mark_node;
+ }
+ }
+ }
+
type = type_decays_to (type);
TREE_TYPE (r) = type;
cp_apply_type_quals_to_decl (cp_type_quals (type), r);
diff --git a/gcc/testsuite/g++.dg/template/array33.C
b/gcc/testsuite/g++.dg/template/array33.C
new file mode 100644
index 00000000000..20b005bc865
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/array33.C
@@ -0,0 +1,39 @@
+// Reject some instantiations whose result depend on the resolution of //
+// Core issues 1001 and 1332, which are not yet resolved.
+// { dg-do compile }
+// { dg-additional-options "-Wno-volatile" }
+
+template<typename T>
+void foo0(T t = 0); // { dg-bogus "" }
+
+template<typename T>
+void foo1(const T = 0); // { dg-message "sorry,
unimplemented" }
+
+template<typename T>
+void foo2(volatile T t = 0); // { dg-message "sorry, unimplemented" }
+
+template<typename T>
+void foo3(const volatile T t = 0); // { dg-message "sorry, unimplemented" }
+
+int main()
+{
+ foo0<char[]>(); // { dg-bogus "" }
+ foo0<const char[]>(); // { dg-bogus "" }
+ foo0<volatile char[]>(); // { dg-bogus "" }
+ foo0<const volatile char[]>(); // { dg-bogus "" }
+
+ foo1<char[]>(); // { dg-message "required from here" }
+ foo1<const char[]>(); // { dg-bogus "" }
+ foo1<volatile char[]>(); // { dg-message "required from here" }
+ foo1<const volatile char[]>(); // { dg-bogus "" }
+
+ foo2<char[]>(); // { dg-message "required from here" }
+ foo2<const char[]>(); // { dg-message "required from
here" }
+ foo2<volatile char[]>(); // { dg-bogus "" }
+ foo2<const volatile char[]>(); // { dg-bogus "" }
+
+ foo3<char[]>(); // { dg-message "required from here" }
+ foo3<const char[]>(); // { dg-message "required from
here" }
+ foo3<volatile char[]>(); // { dg-message "required from here" }
+ foo3<const volatile char[]>(); // { dg-bogus "" }
+}
diff --git a/gcc/testsuite/g++.dg/template/array34.C
b/gcc/testsuite/g++.dg/template/array34.C
new file mode 100644
index 00000000000..316ea2f6407
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/array34.C
@@ -0,0 +1,63 @@
+// Reject some instantiations whose result depend on the resolution of
+// Core issues 1001 and 1332, which are not yet resolved.
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wno-volatile" }
+
+template<typename... Ts>
+void foo0(Ts... t); // { dg-bogus "" }
+
+template<typename... Ts>
+void foo1(const Ts... t); // { dg-message "sorry, unimplemented" }
+
+template<typename... Ts>
+void foo2(volatile Ts... t); // { dg-message "sorry, unimplemented" }
+
+template<typename... Ts>
+void foo3(const volatile Ts... t); // { dg-message "sorry, unimplemented" }
+
+template<typename... Ts>
+void bar0()
+{
+ foo0<int, Ts..., int>(0, 0, 0);
+}
+
+template<typename... Ts>
+void bar1()
+{
+ foo1<int, Ts..., int>(0, 0, 0);
+}
+
+template<typename... Ts>
+void bar2()
+{
+ foo2<int, Ts..., int>(0, 0, 0);
+}
+
+template<typename... Ts>
+void bar3()
+{
+ foo3<int, Ts..., int>(0, 0, 0);
+}
+
+int main()
+{
+ bar0<char[]>(); // { dg-bogus "" }
+ bar0<const char[]>(); // { dg-bogus "" }
+ bar0<volatile char[]>(); // { dg-bogus "" }
+ bar0<const volatile char[]>(); // { dg-bogus "" }
+
+ bar1<char[]>(); // { dg-message "required from here" }
+ bar1<const char[]>(); // { dg-bogus "" }
+ bar1<volatile char[]>(); // { dg-message "required from here" }
+ bar1<const volatile char[]>(); // { dg-bogus "" }
+
+ bar2<char[]>(); // { dg-message "required from here" }
+ bar2<const char[]>(); // { dg-message "required from
here" }
+ bar2<volatile char[]>(); // { dg-bogus "" }
+ bar2<const volatile char[]>(); // { dg-bogus "" }
+
+ bar3<char[]>(); // { dg-message "required from here" }
+ bar3<const char[]>(); // { dg-message "required from
here" }
+ bar3<volatile char[]>(); // { dg-message "required from here" }
+ bar3<const volatile char[]>(); // { dg-bogus "" }
+}
--
2.26.0.106.g9fadedd637
