On Wed, 1 Apr 2020, Jason Merrill wrote:
> On 4/1/20 6:29 PM, Jason Merrill wrote:
> > On 3/31/20 3:50 PM, Patrick Palka wrote:
> > > On Tue, 31 Mar 2020, Jason Merrill wrote:
> > >
> > > > On 3/30/20 6:46 PM, Patrick Palka wrote:
> > > > > On Mon, 30 Mar 2020, Jason Merrill wrote:
> > > > > > On 3/30/20 3:58 PM, 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.
> > > > > >
> > > > > > Hmm, this seems like writing more complicated code for a very
> > > > > > marginal
> > > > > > optimization; how many function templates have so many parameters
> > > > > > that
> > > > > > walking
> > > > > > over them once to compare types will have any effect on compile
> > > > > > time?
> > > > >
> > > > > Good point... though I just tried implementing this check in
> > > > > tsubst_function_decl, and it seems it might be just as complicated to
> > > > > implement it there instead, at least if we want to handle function
> > > > > parameter packs correctly.
> > > > >
> > > > > If we were to implement this check in tsubst_function_decl, then since
> > > > > we have access to the instantiated function, it would presumably
> > > > > suffice
> > > > > to compare its substituted DECL_ARGUMENTS with its substituted
> > > > > TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
> > > > > catch the original testcase, i.e.
> > > > >
> > > > > template<typename T>
> > > > > void foo(const T);
> > > > > int main() { foo<int[]>(0); }
> > > > >
> > > > > because the DECL_ARGUMENTS of foo<int[]> would be {const int*} and its
> > > > > TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
> > > > > corresponding testcase that uses a function parameter pack, i.e.
> > > > >
> > > > > template<typename... Ts>
> > > > > void foo(const Ts...);
> > > > > int main() { foo<int[]>(0); }
> > > > >
> > > > > because it turns out we don't strip top-level cv-qualifiers from
> > > > > function parameter packs from TYPE_ARG_TYPES at declaration time, as
> > > > > we
> > > > > do with regular function parameters. So in this second testcase both
> > > > > DECL_ARGUMENTS and TYPE_ARG_TYPES of foo<int[]> would be {const int*},
> > > > > and yet we would (presumably) want to reject this instantiation too.
> > > > >
> > > > > So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from
> > > > > tsubst_function_decl would not suffice, and we would still need to do
> > > > > a
> > > > > variant of the trick that's done in this patch, i.e. substitute into
> > > > > each dependent parameter type stripped of its top-level cv-qualifiers,
> > > > > to see if these cv-qualifiers make a material difference in the
> > > > > resulting function type. Or maybe there's yet another way to detect
> > > > > this?
> > > >
> > > > I think let's go ahead with comparing TYPE_ARG_TYPES and DECL_ARGUMENTS;
> > > > the
> > > > problem comes when they disagree. If we're handling pack expansions
> > > > wrong,
> > > > that's a separate issue.
> > >
> > > Hm, comparing TYPE_ARG_TYPES and DECL_ARGUMENTS for compatibility seems
> > > to be exposing a latent bug with how we handle lambdas that appear in
> > > function parameter types. Take g++.dg/cpp2a/lambda-uneval3.C for
> > > example:
> > >
> > > template <class T> void spam(decltype([]{}) (*s)[sizeof(T)]) {}
> > > int main() { spam<char>(nullptr); }
> > >
> > > According to tsubst_function_decl in current trunk, the type of the
> > > function paremeter 's' of spam<char> according to its TYPE_ARG_TYPES is
> > > struct ._anon_4[1] *
> > > and according to its DECL_ARGUMENTS the type of 's' is
> > > struct ._anon_5[1] *
> > >
> > > The disagreement happens because we call tsubst_lambda_expr twice during
> > > substitution and thereby generate two distinct lambda types, one when
> > > substituting into the TYPE_ARG_TYPES and another when substituting into
> > > the DECL_ARGUMENTS. I'm not sure how to work around this
> > > bug/false-positive..
> >
> > Oof.
> >
> > I think probably the right answer is to rebuild TYPE_ARG_TYPES from
> > DECL_ARGUMENTS if they don't match.
>
> ...and treat that as a resolution of 1001/1322, so not giving an error.
Is something like this what you have in mind? Bootstrap and testing in
progress.
-- >8 --
Subject: [PATCH] c++: Rebuild function type when it disagrees with formal
parameter types [PR92010]
gcc/cp/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* pt.c (maybe_rebuild_function_type): New function.
(tsubst_function_decl): Use it.
gcc/testsuite/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* g++.dg/cpp2a/lambda-uneval11.c: New test.
* g++.dg/template/array33.C: New test.
* g++.dg/template/array34.C: New test.
* g++.dg/template/defarg22.C: New test.
---
gcc/cp/pt.c | 55 +++++++++++++++++
gcc/testsuite/g++.dg/cpp2a/lambda-uneval11.C | 10 ++++
gcc/testsuite/g++.dg/template/array33.C | 63 ++++++++++++++++++++
gcc/testsuite/g++.dg/template/array34.C | 63 ++++++++++++++++++++
gcc/testsuite/g++.dg/template/defarg22.C | 13 ++++
5 files changed, 204 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/lambda-uneval11.C
create mode 100644 gcc/testsuite/g++.dg/template/array33.C
create mode 100644 gcc/testsuite/g++.dg/template/array34.C
create mode 100644 gcc/testsuite/g++.dg/template/defarg22.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 041ce35a31c..fc0df790c0f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13475,6 +13475,59 @@ lookup_explicit_specifier (tree v)
return *explicit_specifier_map->get (v);
}
+/* Check if the function type of DECL, a FUNCTION_DECL, agrees with the type of
+ each of its formal parameters. If there is a disagreement then rebuild
+ DECL's function type according to its formal parameter types, as part of a
+ resolution for Core issues 1001/1322. */
+
+static void
+maybe_rebuild_function_decl_type (tree decl)
+{
+ bool function_type_needs_rebuilding = false;
+ if (tree parm_list = FUNCTION_FIRST_USER_PARM (decl))
+ {
+ tree parm_type_list = FUNCTION_FIRST_USER_PARMTYPE (decl);
+ while (parm_type_list && parm_type_list != void_list_node)
+ {
+ tree parm_type = TREE_VALUE (parm_type_list);
+ tree formal_parm_type_unqual = strip_top_quals (TREE_TYPE
(parm_list));
+ if (!same_type_p (parm_type, formal_parm_type_unqual))
+ {
+ function_type_needs_rebuilding = true;
+ break;
+ }
+
+ parm_list = DECL_CHAIN (parm_list);
+ parm_type_list = TREE_CHAIN (parm_type_list);
+ }
+ }
+
+ if (!function_type_needs_rebuilding)
+ return;
+
+ const tree new_arg_types = copy_list (TYPE_ARG_TYPES (TREE_TYPE (decl)));
+
+ tree parm_list = FUNCTION_FIRST_USER_PARM (decl);
+ tree old_parm_type_list = FUNCTION_FIRST_USER_PARMTYPE (decl);
+ tree new_parm_type_list = skip_artificial_parms_for (decl, new_arg_types);
+ while (old_parm_type_list && old_parm_type_list != void_list_node)
+ {
+ tree *new_parm_type = &TREE_VALUE (new_parm_type_list);
+ tree formal_parm_type_unqual = strip_top_quals (TREE_TYPE (parm_list));
+ if (!same_type_p (*new_parm_type, formal_parm_type_unqual))
+ *new_parm_type = formal_parm_type_unqual;
+
+ if (TREE_CHAIN (old_parm_type_list) == void_list_node)
+ TREE_CHAIN (new_parm_type_list) = void_list_node;
+ parm_list = DECL_CHAIN (parm_list);
+ old_parm_type_list = TREE_CHAIN (old_parm_type_list);
+ new_parm_type_list = TREE_CHAIN (new_parm_type_list);
+ }
+
+ TREE_TYPE (decl)
+ = build_function_type (TREE_TYPE (TREE_TYPE (decl)), new_arg_types);
+}
+
/* Subroutine of tsubst_decl for the case when T is a FUNCTION_DECL. */
static tree
@@ -13665,6 +13718,8 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t
complain,
DECL_ARGUMENTS (r) = parms;
DECL_RESULT (r) = NULL_TREE;
+ maybe_rebuild_function_decl_type (r);
+
TREE_STATIC (r) = 0;
TREE_PUBLIC (r) = TREE_PUBLIC (t);
DECL_EXTERNAL (r) = 1;
diff --git a/gcc/testsuite/g++.dg/cpp2a/lambda-uneval11.C
b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval11.C
new file mode 100644
index 00000000000..a04262494c7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/lambda-uneval11.C
@@ -0,0 +1,10 @@
+// PR c++/92010
+// { dg-do compile { target c++2a } }
+
+template <class T> void spam(decltype([]{}) (*s)[sizeof(T)] = nullptr)
+{ }
+
+void foo()
+{
+ spam<int>();
+}
diff --git a/gcc/testsuite/g++.dg/template/array33.C
b/gcc/testsuite/g++.dg/template/array33.C
new file mode 100644
index 00000000000..0aa587351b4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/array33.C
@@ -0,0 +1,63 @@
+// Verify that top-level cv-qualifiers on parameter types are considered
+// when determining the function type of an instantiated function template.
+// This resolves a part of Core issues 1001/1322.
+// { dg-do compile }
+// { dg-additional-options "-Wno-volatile" }
+
+template<typename T>
+void foo0(T t = 0);
+
+template<typename T>
+void foo1(const T = 0);
+
+template<typename T>
+void foo2(volatile T t = 0);
+
+template<typename T>
+void foo3(const volatile T t = 0);
+
+#if __cplusplus >= 201103L
+#define SA(X) static_assert(X,#X)
+SA(__is_same(decltype(foo0<char[]>), void(char*)));
+SA(__is_same(decltype(foo0<const char[]>), void(const char*)));
+SA(__is_same(decltype(foo0<volatile char[]>), void(volatile char*)));
+SA(__is_same(decltype(foo0<const volatile char[]>), void(const volatile
char*)));
+
+SA(__is_same(decltype(foo1<char[]>), void(const char*)));
+SA(__is_same(decltype(foo1<const char[]>), void(const char*)));
+SA(__is_same(decltype(foo1<volatile char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo1<const volatile char[]>), void(const volatile
char*)));
+
+SA(__is_same(decltype(foo2<char[]>), void(volatile char*)));
+SA(__is_same(decltype(foo2<const char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo2<volatile char[]>), void(volatile char*)));
+SA(__is_same(decltype(foo2<const volatile char[]>), void(const volatile
char*)));
+
+SA(__is_same(decltype(foo3<char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo3<const char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo3<volatile char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo3<const volatile char[]>), void(const volatile
char*)));
+#endif
+
+int main()
+{
+ foo0<char[]>();
+ foo0<const char[]>();
+ foo0<volatile char[]>();
+ foo0<const volatile char[]>();
+
+ foo1<char[]>();
+ foo1<const char[]>();
+ foo1<volatile char[]>();
+ foo1<const volatile char[]>();
+
+ foo2<char[]>();
+ foo2<const char[]>();
+ foo2<volatile char[]>();
+ foo2<const volatile char[]>();
+
+ foo3<char[]>();
+ foo3<const char[]>();
+ foo3<volatile char[]>();
+ foo3<const volatile char[]>();
+}
diff --git a/gcc/testsuite/g++.dg/template/array34.C
b/gcc/testsuite/g++.dg/template/array34.C
new file mode 100644
index 00000000000..38c06401974
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/array34.C
@@ -0,0 +1,63 @@
+// Verify that top-level cv-qualifiers on parameter types are considered
+// when determining the function type of an instantiated function template.
+// This resolves a part of Core issues 1001/1322.
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-Wno-volatile" }
+
+template<typename... Ts>
+void foo0(Ts... t);
+
+template<typename... Ts>
+void foo1(const Ts... t);
+
+template<typename... Ts>
+void foo2(volatile Ts... t);
+
+template<typename... Ts>
+void foo3(const volatile Ts... t);
+
+#if __cplusplus >= 201103L
+#define SA(X) static_assert(X,#X)
+SA(__is_same(decltype(foo0<char[]>), void(char*)));
+SA(__is_same(decltype(foo0<const char[]>), void(const char*)));
+SA(__is_same(decltype(foo0<volatile char[]>), void(volatile char*)));
+SA(__is_same(decltype(foo0<const volatile char[]>), void(const volatile
char*)));
+
+SA(__is_same(decltype(foo1<char[]>), void(const char*)));
+SA(__is_same(decltype(foo1<const char[]>), void(const char*)));
+SA(__is_same(decltype(foo1<volatile char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo1<const volatile char[]>), void(const volatile
char*)));
+
+SA(__is_same(decltype(foo2<char[]>), void(volatile char*)));
+SA(__is_same(decltype(foo2<const char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo2<volatile char[]>), void(volatile char*)));
+SA(__is_same(decltype(foo2<const volatile char[]>), void(const volatile
char*)));
+
+SA(__is_same(decltype(foo3<char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo3<const char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo3<volatile char[]>), void(const volatile char*)));
+SA(__is_same(decltype(foo3<const volatile char[]>), void(const volatile
char*)));
+#endif
+
+int main()
+{
+ foo0<char[]>(0);
+ foo0<const char[]>(0);
+ foo0<volatile char[]>(0);
+ foo0<const volatile char[]>(0);
+
+ foo1<char[]>(0);
+ foo1<const char[]>(0);
+ foo1<volatile char[]>(0);
+ foo1<const volatile char[]>(0);
+
+ foo2<char[]>(0);
+ foo2<const char[]>(0);
+ foo2<volatile char[]>(0);
+ foo2<const volatile char[]>(0);
+
+ foo3<char[]>(0);
+ foo3<const char[]>(0);
+ foo3<volatile char[]>(0);
+ foo3<const volatile char[]>(0);
+}
diff --git a/gcc/testsuite/g++.dg/template/defarg22.C
b/gcc/testsuite/g++.dg/template/defarg22.C
new file mode 100644
index 00000000000..599061cedb0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/defarg22.C
@@ -0,0 +1,13 @@
+// PR c++/92010
+// { dg-do compile { target c++11 } }
+
+template <typename T = char[3]>
+void foo(const T t = "; ")
+{
+}
+
+int main()
+{
+ foo ();
+}
+
--
2.26.0.106.g9fadedd637
