diff mbox series

c++: Fix ICE in tsubst_default_argument [PR92010]

Message ID 20200323012105.3692086-1-ppalka@redhat.com
State New
Headers show
Series c++: Fix ICE in tsubst_default_argument [PR92010] | expand

Commit Message

Jeff Law via Gcc-patches March 23, 2020, 1:21 a.m. UTC
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?

gcc/cp/ChangeLog:

	PR c++/92010
	* pt.c (tsubst_default_argument): Relax assertion to permit
	disagreements between the function type and the parameter type
	about the cv-qualification of the pointed-to type.

gcc/testsuite/ChangeLog:

	PR c++/92010
	* g++.dg/template/defarg22.C: New test.
---
 gcc/cp/pt.c                              | 17 ++++++++++++++++-
 gcc/testsuite/g++.dg/template/defarg22.C | 11 +++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/template/defarg22.C

Comments

Jeff Law via Gcc-patches March 23, 2020, 3:11 a.m. UTC | #1
On Sun, 22 Mar 2020, 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?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/92010
> 	* pt.c (tsubst_default_argument): Relax assertion to permit
> 	disagreements between the function type and the parameter type
> 	about the cv-qualification of the pointed-to type.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/92010
> 	* g++.dg/template/defarg22.C: New test.
> ---

Here's a slightly simpler version of the patch that moves the
POINTER_TYPE_P checks into the gcc_assert:

-- >8 --
---
 gcc/cp/pt.c                              | 16 +++++++++++++++-
 gcc/testsuite/g++.dg/template/defarg22.C | 11 +++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/template/defarg22.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 03a8dfbd37c..849628840d6 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13337,7 +13337,21 @@ tsubst_default_argument (tree fn, int parmnum, tree type, tree arg,
   if (parmtype == error_mark_node)
     return error_mark_node;
 
-  gcc_assert (same_type_ignoring_top_level_qualifiers_p (type, parmtype));
+  if (same_type_ignoring_top_level_qualifiers_p (type, parmtype))
+    ;
+  else
+    {
+      /* The function type and the parameter type can disagree about the
+	 cv-qualification of the pointed-to type; see PR92010.  */
+      gcc_assert (POINTER_TYPE_P (type) && POINTER_TYPE_P (parmtype)
+		  && same_type_p (TREE_TYPE (type),
+				  strip_top_quals (TREE_TYPE (parmtype))));
+      /* Verify that this happens only when the dependent parameter type is a
+	 cv-qualified wildcard type.  */
+      tree pattern_parm = get_pattern_parm (parm, DECL_TI_TEMPLATE (fn));
+      gcc_assert (WILDCARD_TYPE_P (TREE_TYPE (pattern_parm))
+		  && cv_qualified_p (TREE_TYPE (pattern_parm)));
+    }
 
   tree *slot;
   if (defarg_inst && (slot = defarg_inst->get (parm)))
diff --git a/gcc/testsuite/g++.dg/template/defarg22.C b/gcc/testsuite/g++.dg/template/defarg22.C
new file mode 100644
index 00000000000..cf6261916d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/defarg22.C
@@ -0,0 +1,11 @@
+// PR c++/92010
+// { dg-do compile }
+
+template <typename T>
+void foo(const T t = 0)
+{ }
+
+int main()
+{
+  foo<char[]>();
+}
Jeff Law via Gcc-patches March 26, 2020, 7:29 p.m. UTC | #2
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.

> gcc/cp/ChangeLog:
> 
> 	PR c++/92010
> 	* pt.c (tsubst_default_argument): Relax assertion to permit
> 	disagreements between the function type and the parameter type
> 	about the cv-qualification of the pointed-to type.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/92010
> 	* g++.dg/template/defarg22.C: New test.
> ---
>   gcc/cp/pt.c                              | 17 ++++++++++++++++-
>   gcc/testsuite/g++.dg/template/defarg22.C | 11 +++++++++++
>   2 files changed, 27 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/defarg22.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 9e1eb9416c9..923166276b8 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -13337,7 +13337,22 @@ tsubst_default_argument (tree fn, int parmnum, tree type, tree arg,
>     if (parmtype == error_mark_node)
>       return error_mark_node;
>   
> -  gcc_assert (same_type_ignoring_top_level_qualifiers_p (type, parmtype));
> +  if (same_type_ignoring_top_level_qualifiers_p (type, parmtype))
> +    ;
> +  else if (POINTER_TYPE_P (type) && POINTER_TYPE_P (parmtype))
> +    {
> +      /* The function type and the parameter type can disagree about the
> +	 cv-qualification of the pointed-to type; see PR92010.  */
> +      gcc_assert (same_type_p (TREE_TYPE (type),
> +			       strip_top_quals (TREE_TYPE (parmtype))));
> +      /* Verify that this happens only when the dependent parameter type is a
> +	 cv-qualified wildcard type.  */
> +      tree pattern_parm = get_pattern_parm (parm, DECL_TI_TEMPLATE (fn));
> +      gcc_assert (WILDCARD_TYPE_P (TREE_TYPE (pattern_parm))
> +		  && cv_qualified_p (TREE_TYPE (pattern_parm)));
> +    }
> +  else
> +    gcc_unreachable ();
>   
>     tree *slot;
>     if (defarg_inst && (slot = defarg_inst->get (parm)))
> diff --git a/gcc/testsuite/g++.dg/template/defarg22.C b/gcc/testsuite/g++.dg/template/defarg22.C
> new file mode 100644
> index 00000000000..cf6261916d8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/defarg22.C
> @@ -0,0 +1,11 @@
> +// PR c++/92010
> +// { dg-do compile }
> +
> +template <typename T>
> +void foo(const T t = 0)
> +{ }
> +
> +int main()
> +{
> +  foo<char[]>();
> +}
>
Jeff Law via Gcc-patches March 30, 2020, 7:58 p.m. UTC | #3
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?

-- >8 --

Subject: [PATCH] c++: Reject some instantiations that depend on resolution of
 Core issues 1001/1322

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                             | 70 ++++++++++++++++++++++++-
 gcc/testsuite/g++.dg/template/array33.C | 39 ++++++++++++++
 gcc/testsuite/g++.dg/template/array34.C | 63 ++++++++++++++++++++++
 3 files changed, 171 insertions(+), 1 deletion(-)
 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..fd99053df36 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14072,9 +14072,77 @@ 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);
 
+	    const int type_quals = cp_type_quals (type);
+
+	    /* 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 the case where 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.  */
+	    tree unqual_expanded_types = NULL_TREE;
+	    if (TREE_CODE (type) == ARRAY_TYPE
+		&& (type_quals & (TYPE_QUAL_CONST|TYPE_QUAL_VOLATILE))
+		&& 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 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);
+	    cp_apply_type_quals_to_decl (type_quals, r);
 
             if (DECL_INITIAL (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 "" }
+}
Jeff Law via Gcc-patches March 30, 2020, 8:15 p.m. UTC | #4
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 "" }
+}
Jeff Law via Gcc-patches March 30, 2020, 8:42 p.m. UTC | #5
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?

> Is something like this what you have in mind?
> 
> -- >8 --
> 
> Subject: [PATCH] c++: Reject some instantiations that depend on resolution of
>   Core issues 1001/1322
> 
> 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                             | 70 ++++++++++++++++++++++++-
>   gcc/testsuite/g++.dg/template/array33.C | 39 ++++++++++++++
>   gcc/testsuite/g++.dg/template/array34.C | 63 ++++++++++++++++++++++
>   3 files changed, 171 insertions(+), 1 deletion(-)
>   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..fd99053df36 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -14072,9 +14072,77 @@ 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);
>   
> +	    const int type_quals = cp_type_quals (type);
> +
> +	    /* 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 the case where 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.  */
> +	    tree unqual_expanded_types = NULL_TREE;
> +	    if (TREE_CODE (type) == ARRAY_TYPE
> +		&& (type_quals & (TYPE_QUAL_CONST|TYPE_QUAL_VOLATILE))
> +		&& 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 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);
> +	    cp_apply_type_quals_to_decl (type_quals, r);
>   
>               if (DECL_INITIAL (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 "" }
> +}
>
Jeff Law via Gcc-patches March 30, 2020, 10:46 p.m. UTC | #6
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?

> 
> > Is something like this what you have in mind?
> > 
> > -- >8 --
> > 
> > Subject: [PATCH] c++: Reject some instantiations that depend on resolution
> > of
> >   Core issues 1001/1322
> > 
> > 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                             | 70 ++++++++++++++++++++++++-
> >   gcc/testsuite/g++.dg/template/array33.C | 39 ++++++++++++++
> >   gcc/testsuite/g++.dg/template/array34.C | 63 ++++++++++++++++++++++
> >   3 files changed, 171 insertions(+), 1 deletion(-)
> >   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..fd99053df36 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -14072,9 +14072,77 @@ 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);
> >   +	    const int type_quals = cp_type_quals (type);
> > +
> > +	    /* 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 the case where 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.
> > */
> > +	    tree unqual_expanded_types = NULL_TREE;
> > +	    if (TREE_CODE (type) == ARRAY_TYPE
> > +		&& (type_quals & (TYPE_QUAL_CONST|TYPE_QUAL_VOLATILE))
> > +		&& 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 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);
> > +	    cp_apply_type_quals_to_decl (type_quals, r);
> >                 if (DECL_INITIAL (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 "" }
> > +}
> > 
> 
>
Jeff Law via Gcc-patches March 31, 2020, 5:13 p.m. UTC | #7
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.

Jason
Jeff Law via Gcc-patches March 31, 2020, 7:50 p.m. UTC | #8
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..
Jeff Law via Gcc-patches April 1, 2020, 10:29 p.m. UTC | #9
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.

Jason
Jeff Law via Gcc-patches April 1, 2020, 10:37 p.m. UTC | #10
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.

Jason
Jeff Law via Gcc-patches April 6, 2020, 3:45 p.m. UTC | #11
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 ();
+}
+
Jeff Law via Gcc-patches April 6, 2020, 9:33 p.m. UTC | #12
On 4/6/20 11:45 AM, Patrick Palka wrote:
> 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.

Yes, thanks.

> -- >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);
> +    }

The usual pattern for this sort of thing is to use a tree* to track the 
end of the new list, which should also avoid making a garbage copy of 
void_list_node.  e.g. from tsubst_attribute:

>       tree list = NULL_TREE;
>       tree *q = &list;
>       for (int i = 0; i < len; ++i)
>         {
>           tree elt = TREE_VEC_ELT (pack, i);
>           *q = build_tree_list (purp, elt);
>           q = &TREE_CHAIN (*q);
>         }

Jason
Jeff Law via Gcc-patches April 7, 2020, 5:40 p.m. UTC | #13
On Mon, 6 Apr 2020, Jason Merrill wrote:
> On 4/6/20 11:45 AM, Patrick Palka wrote:
> > 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.
> 
> Yes, thanks.
> 
> > -- >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);
> > +    }
> 
> The usual pattern for this sort of thing is to use a tree* to track the end of
> the new list, which should also avoid making a garbage copy of void_list_node.
> e.g. from tsubst_attribute:
> 
> >       tree list = NULL_TREE;
> >       tree *q = &list;
> >       for (int i = 0; i < len; ++i)
> >         {
> >           tree elt = TREE_VEC_ELT (pack, i);
> >           *q = build_tree_list (purp, elt);
> >           q = &TREE_CHAIN (*q);
> >         }

Ah so that's the right way do it :) Patch updated to make use of this
pattern.

This version of the patch is more complete.  It builds the new
FUNCTION_TYPE and METHOD_TYPE the same way that tsubst_function_type
does, by splitting out and reusing the relevant parts of
tsubst_function_type into a separate subroutine that is responsible for
propagating TYPE_ATTRIBUTES, TYPE_RAISES_EXCEPTION, ref-qualifiers, etc.

I wonder if for consistency and correctness we might have to update
other callers of tsubst_function_type/tsubst to make sure this
function-type-rebuilding based on parameter types is done in these
callers too.  For example, there is is_specialization_of_friend which
calls tsubst_function_type on the type of a function decl, and
fn_type_unification and determine_specialization which also call tsubst
on the type of a function decl (and pass the tf_fndecl_type flag).

If so, maybe we could instead leverage the tf_fndecl_type flag and the
'in_decl' tsubst parameter to change tsubst_arg_types to immediately
build the function type according to the parameter types of in_decl
(which would presumably be the FUNCTION_DECL)?  That way, we would just
have to update the above potentially problematic callers to pass
tf_fndecl_type and set in_decl appropriately when calling tsubst and
would only have to build the function type once.

Patch partially tested on unbootstrapped x86_64-pc-linux-gnu, and
bootstrap/regtest is 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 (rebuild_function_or_method_type): Split function out from ...
	(tsubst_function_type): ... here.
	(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                                  | 151 ++++++++++++++-----
 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, 263 insertions(+), 37 deletions(-)
 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 6122227c22f..256a937eace 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13475,6 +13475,116 @@ lookup_explicit_specifier (tree v)
   return *explicit_specifier_map->get (v);
 }
 
+/* Given T, a FUNCTION_TYPE or METHOD_TYPE, construct and return a corresponding
+   FUNCTION_TYPE or METHOD_TYPE whose return type is RETURN_TYPE, argument types
+   are ARG_TYPES, and exception specification is RAISES, and otherwise is
+   identical to T.  */
+
+static tree
+rebuild_function_or_method_type (tree t, tree return_type, tree arg_types,
+				 tree raises, tsubst_flags_t complain)
+{
+  gcc_assert (FUNC_OR_METHOD_TYPE_P (t));
+
+  tree new_type;
+  if (TREE_CODE (t) == FUNCTION_TYPE)
+    {
+      new_type = build_function_type (return_type, arg_types);
+      new_type = apply_memfn_quals (new_type, type_memfn_quals (t));
+    }
+  else
+    {
+      tree r = TREE_TYPE (TREE_VALUE (arg_types));
+      /* Don't pick up extra function qualifiers from the basetype.  */
+      r = cp_build_qualified_type_real (r, type_memfn_quals (t), complain);
+      if (! MAYBE_CLASS_TYPE_P (r))
+	{
+	  /* [temp.deduct]
+
+	     Type deduction may fail for any of the following
+	     reasons:
+
+	     -- Attempting to create "pointer to member of T" when T
+	     is not a class type.  */
+	  if (complain & tf_error)
+	    error ("creating pointer to member function of non-class type %qT",
+		   r);
+	  return error_mark_node;
+	}
+
+      new_type = build_method_type_directly (r, return_type,
+					     TREE_CHAIN (arg_types));
+    }
+  new_type = cp_build_type_attribute_variant (new_type, TYPE_ATTRIBUTES (t));
+
+  cp_ref_qualifier rqual = type_memfn_rqual (t);
+  bool late_return_type_p = TYPE_HAS_LATE_RETURN_TYPE (t);
+  return build_cp_fntype_variant (new_type, rqual, raises, late_return_type_p);
+}
+
+/* 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 fntype = TREE_TYPE (decl);
+  tree parm_list = DECL_ARGUMENTS (decl);
+  tree old_parm_type_list = TYPE_ARG_TYPES (fntype);
+  tree new_parm_type_list = NULL_TREE;
+  tree *q = &new_parm_type_list;
+  for (int skip = num_artificial_parms_for (decl); skip > 0; skip--)
+    {
+      *q = copy_node (old_parm_type_list);
+      parm_list = DECL_CHAIN (parm_list);
+      old_parm_type_list = TREE_CHAIN (old_parm_type_list);
+      q = &TREE_CHAIN (*q);
+    }
+  while (old_parm_type_list && old_parm_type_list != void_list_node)
+    {
+      *q = copy_node (old_parm_type_list);
+      tree *new_parm_type = &TREE_VALUE (*q);
+      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;
+
+      parm_list = DECL_CHAIN (parm_list);
+      old_parm_type_list = TREE_CHAIN (old_parm_type_list);
+      q = &TREE_CHAIN (*q);
+    }
+  if (old_parm_type_list == void_list_node)
+    *q = void_list_node;
+
+  TREE_TYPE (decl)
+    = rebuild_function_or_method_type (fntype,
+				       TREE_TYPE (fntype), new_parm_type_list,
+				       TYPE_RAISES_EXCEPTIONS (fntype), tf_none);
+}
+
 /* Subroutine of tsubst_decl for the case when T is a FUNCTION_DECL.  */
 
 static tree
@@ -13665,6 +13775,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;
@@ -14694,7 +14806,6 @@ tsubst_function_type (tree t,
 {
   tree return_type;
   tree arg_types = NULL_TREE;
-  tree fntype;
 
   /* The TYPE_CONTEXT is not used for function/method types.  */
   gcc_assert (TYPE_CONTEXT (t) == NULL_TREE);
@@ -14765,42 +14876,8 @@ tsubst_function_type (tree t,
     }
 
   /* Construct a new type node and return it.  */
-  if (TREE_CODE (t) == FUNCTION_TYPE)
-    {
-      fntype = build_function_type (return_type, arg_types);
-      fntype = apply_memfn_quals (fntype, type_memfn_quals (t));
-    }
-  else
-    {
-      tree r = TREE_TYPE (TREE_VALUE (arg_types));
-      /* Don't pick up extra function qualifiers from the basetype.  */
-      r = cp_build_qualified_type_real (r, type_memfn_quals (t), complain);
-      if (! MAYBE_CLASS_TYPE_P (r))
-	{
-	  /* [temp.deduct]
-
-	     Type deduction may fail for any of the following
-	     reasons:
-
-	     -- Attempting to create "pointer to member of T" when T
-	     is not a class type.  */
-	  if (complain & tf_error)
-	    error ("creating pointer to member function of non-class type %qT",
-		      r);
-	  return error_mark_node;
-	}
-
-      fntype = build_method_type_directly (r, return_type,
-					   TREE_CHAIN (arg_types));
-    }
-  fntype = cp_build_type_attribute_variant (fntype, TYPE_ATTRIBUTES (t));
-
-  /* See comment above.  */
-  tree raises = NULL_TREE;
-  cp_ref_qualifier rqual = type_memfn_rqual (t);
-  fntype = build_cp_fntype_variant (fntype, rqual, raises, late_return_type_p);
-
-  return fntype;
+  return rebuild_function_or_method_type (t, return_type, arg_types,
+					  /*raises=*/NULL_TREE, complain);
 }
 
 /* FNTYPE is a FUNCTION_TYPE or METHOD_TYPE.  Substitute the template
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 ();
+}
+
Jeff Law via Gcc-patches April 7, 2020, 8:26 p.m. UTC | #14
On Tue, 7 Apr 2020, Patrick Palka wrote:
> On Mon, 6 Apr 2020, Jason Merrill wrote:
> > On 4/6/20 11:45 AM, Patrick Palka wrote:
> > > 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.
> > 
> > Yes, thanks.
> > 
> > > -- >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);
> > > +    }
> > 
> > The usual pattern for this sort of thing is to use a tree* to track the end of
> > the new list, which should also avoid making a garbage copy of void_list_node.
> > e.g. from tsubst_attribute:
> > 
> > >       tree list = NULL_TREE;
> > >       tree *q = &list;
> > >       for (int i = 0; i < len; ++i)
> > >         {
> > >           tree elt = TREE_VEC_ELT (pack, i);
> > >           *q = build_tree_list (purp, elt);
> > >           q = &TREE_CHAIN (*q);
> > >         }
> 
> Ah so that's the right way do it :) Patch updated to make use of this
> pattern.
> 
> This version of the patch is more complete.  It builds the new
> FUNCTION_TYPE and METHOD_TYPE the same way that tsubst_function_type
> does, by splitting out and reusing the relevant parts of
> tsubst_function_type into a separate subroutine that is responsible for
> propagating TYPE_ATTRIBUTES, TYPE_RAISES_EXCEPTION, ref-qualifiers, etc.
> 
> I wonder if for consistency and correctness we might have to update
> other callers of tsubst_function_type/tsubst to make sure this
> function-type-rebuilding based on parameter types is done in these
> callers too.  For example, there is is_specialization_of_friend which
> calls tsubst_function_type on the type of a function decl, and
> fn_type_unification and determine_specialization which also call tsubst
> on the type of a function decl (and pass the tf_fndecl_type flag).
> 
> If so, maybe we could instead leverage the tf_fndecl_type flag and the
> 'in_decl' tsubst parameter to change tsubst_arg_types to immediately
> build the function type according to the parameter types of in_decl
> (which would presumably be the FUNCTION_DECL)?  That way, we would just
> have to update the above potentially problematic callers to pass
> tf_fndecl_type and set in_decl appropriately when calling tsubst and
> would only have to build the function type once.

Never mind about the above alternative suggestion, it wouldn't work for
parameter types that involve lambdas in unevaluated contexts since we
would still end up substituting into the lambda expression twice, and
would still end up with TYPE_ARG_TYPES disagreeing with the
DECL_ARGUMENTS.

> 
> Patch partially tested on unbootstrapped x86_64-pc-linux-gnu, and
> bootstrap/regtest is in progress.

Bootstrap and regtest finished on x86_64-pc-linux-gnu with no new
regressions.

> 
> -- >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 (rebuild_function_or_method_type): Split function out from ...
> 	(tsubst_function_type): ... here.
> 	(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                                  | 151 ++++++++++++++-----
>  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, 263 insertions(+), 37 deletions(-)
>  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 6122227c22f..256a937eace 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -13475,6 +13475,116 @@ lookup_explicit_specifier (tree v)
>    return *explicit_specifier_map->get (v);
>  }
>  
> +/* Given T, a FUNCTION_TYPE or METHOD_TYPE, construct and return a corresponding
> +   FUNCTION_TYPE or METHOD_TYPE whose return type is RETURN_TYPE, argument types
> +   are ARG_TYPES, and exception specification is RAISES, and otherwise is
> +   identical to T.  */
> +
> +static tree
> +rebuild_function_or_method_type (tree t, tree return_type, tree arg_types,
> +				 tree raises, tsubst_flags_t complain)
> +{
> +  gcc_assert (FUNC_OR_METHOD_TYPE_P (t));
> +
> +  tree new_type;
> +  if (TREE_CODE (t) == FUNCTION_TYPE)
> +    {
> +      new_type = build_function_type (return_type, arg_types);
> +      new_type = apply_memfn_quals (new_type, type_memfn_quals (t));
> +    }
> +  else
> +    {
> +      tree r = TREE_TYPE (TREE_VALUE (arg_types));
> +      /* Don't pick up extra function qualifiers from the basetype.  */
> +      r = cp_build_qualified_type_real (r, type_memfn_quals (t), complain);
> +      if (! MAYBE_CLASS_TYPE_P (r))
> +	{
> +	  /* [temp.deduct]
> +
> +	     Type deduction may fail for any of the following
> +	     reasons:
> +
> +	     -- Attempting to create "pointer to member of T" when T
> +	     is not a class type.  */
> +	  if (complain & tf_error)
> +	    error ("creating pointer to member function of non-class type %qT",
> +		   r);
> +	  return error_mark_node;
> +	}
> +
> +      new_type = build_method_type_directly (r, return_type,
> +					     TREE_CHAIN (arg_types));
> +    }
> +  new_type = cp_build_type_attribute_variant (new_type, TYPE_ATTRIBUTES (t));
> +
> +  cp_ref_qualifier rqual = type_memfn_rqual (t);
> +  bool late_return_type_p = TYPE_HAS_LATE_RETURN_TYPE (t);
> +  return build_cp_fntype_variant (new_type, rqual, raises, late_return_type_p);
> +}
> +
> +/* 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 fntype = TREE_TYPE (decl);
> +  tree parm_list = DECL_ARGUMENTS (decl);
> +  tree old_parm_type_list = TYPE_ARG_TYPES (fntype);
> +  tree new_parm_type_list = NULL_TREE;
> +  tree *q = &new_parm_type_list;
> +  for (int skip = num_artificial_parms_for (decl); skip > 0; skip--)
> +    {
> +      *q = copy_node (old_parm_type_list);
> +      parm_list = DECL_CHAIN (parm_list);
> +      old_parm_type_list = TREE_CHAIN (old_parm_type_list);
> +      q = &TREE_CHAIN (*q);
> +    }
> +  while (old_parm_type_list && old_parm_type_list != void_list_node)
> +    {
> +      *q = copy_node (old_parm_type_list);
> +      tree *new_parm_type = &TREE_VALUE (*q);
> +      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;
> +
> +      parm_list = DECL_CHAIN (parm_list);
> +      old_parm_type_list = TREE_CHAIN (old_parm_type_list);
> +      q = &TREE_CHAIN (*q);
> +    }
> +  if (old_parm_type_list == void_list_node)
> +    *q = void_list_node;
> +
> +  TREE_TYPE (decl)
> +    = rebuild_function_or_method_type (fntype,
> +				       TREE_TYPE (fntype), new_parm_type_list,
> +				       TYPE_RAISES_EXCEPTIONS (fntype), tf_none);
> +}
> +
>  /* Subroutine of tsubst_decl for the case when T is a FUNCTION_DECL.  */
>  
>  static tree
> @@ -13665,6 +13775,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;
> @@ -14694,7 +14806,6 @@ tsubst_function_type (tree t,
>  {
>    tree return_type;
>    tree arg_types = NULL_TREE;
> -  tree fntype;
>  
>    /* The TYPE_CONTEXT is not used for function/method types.  */
>    gcc_assert (TYPE_CONTEXT (t) == NULL_TREE);
> @@ -14765,42 +14876,8 @@ tsubst_function_type (tree t,
>      }
>  
>    /* Construct a new type node and return it.  */
> -  if (TREE_CODE (t) == FUNCTION_TYPE)
> -    {
> -      fntype = build_function_type (return_type, arg_types);
> -      fntype = apply_memfn_quals (fntype, type_memfn_quals (t));
> -    }
> -  else
> -    {
> -      tree r = TREE_TYPE (TREE_VALUE (arg_types));
> -      /* Don't pick up extra function qualifiers from the basetype.  */
> -      r = cp_build_qualified_type_real (r, type_memfn_quals (t), complain);
> -      if (! MAYBE_CLASS_TYPE_P (r))
> -	{
> -	  /* [temp.deduct]
> -
> -	     Type deduction may fail for any of the following
> -	     reasons:
> -
> -	     -- Attempting to create "pointer to member of T" when T
> -	     is not a class type.  */
> -	  if (complain & tf_error)
> -	    error ("creating pointer to member function of non-class type %qT",
> -		      r);
> -	  return error_mark_node;
> -	}
> -
> -      fntype = build_method_type_directly (r, return_type,
> -					   TREE_CHAIN (arg_types));
> -    }
> -  fntype = cp_build_type_attribute_variant (fntype, TYPE_ATTRIBUTES (t));
> -
> -  /* See comment above.  */
> -  tree raises = NULL_TREE;
> -  cp_ref_qualifier rqual = type_memfn_rqual (t);
> -  fntype = build_cp_fntype_variant (fntype, rqual, raises, late_return_type_p);
> -
> -  return fntype;
> +  return rebuild_function_or_method_type (t, return_type, arg_types,
> +					  /*raises=*/NULL_TREE, complain);
>  }
>  
>  /* FNTYPE is a FUNCTION_TYPE or METHOD_TYPE.  Substitute the template
> 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
>
Jeff Law via Gcc-patches April 7, 2020, 9:21 p.m. UTC | #15
On 4/7/20 1:40 PM, Patrick Palka wrote:
> On Mon, 6 Apr 2020, Jason Merrill wrote:
>> On 4/6/20 11:45 AM, Patrick Palka wrote:
>>> 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.
>>
>> Yes, thanks.
>>
>>> -- >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);
>>> +    }
>>
>> The usual pattern for this sort of thing is to use a tree* to track the end of
>> the new list, which should also avoid making a garbage copy of void_list_node.
>> e.g. from tsubst_attribute:
>>
>>>        tree list = NULL_TREE;
>>>        tree *q = &list;
>>>        for (int i = 0; i < len; ++i)
>>>          {
>>>            tree elt = TREE_VEC_ELT (pack, i);
>>>            *q = build_tree_list (purp, elt);
>>>            q = &TREE_CHAIN (*q);
>>>          }
> 
> Ah so that's the right way do it :) Patch updated to make use of this
> pattern.
> 
> This version of the patch is more complete.  It builds the new
> FUNCTION_TYPE and METHOD_TYPE the same way that tsubst_function_type
> does, by splitting out and reusing the relevant parts of
> tsubst_function_type into a separate subroutine that is responsible for
> propagating TYPE_ATTRIBUTES, TYPE_RAISES_EXCEPTION, ref-qualifiers, etc.
> 
> I wonder if for consistency and correctness we might have to update
> other callers of tsubst_function_type/tsubst to make sure this
> function-type-rebuilding based on parameter types is done in these
> callers too.  For example, there is is_specialization_of_friend which
> calls tsubst_function_type on the type of a function decl, and
> fn_type_unification and determine_specialization which also call tsubst
> on the type of a function decl (and pass the tf_fndecl_type flag).
> 
> If so, maybe we could instead leverage the tf_fndecl_type flag and the
> 'in_decl' tsubst parameter to change tsubst_arg_types to immediately
> build the function type according to the parameter types of in_decl
> (which would presumably be the FUNCTION_DECL)?  That way, we would just
> have to update the above potentially problematic callers to pass
> tf_fndecl_type and set in_decl appropriately when calling tsubst and
> would only have to build the function type once.
> 
> Patch partially tested on unbootstrapped x86_64-pc-linux-gnu, and
> bootstrap/regtest is in progress.
> 
> -- >8 --
> 
> Subject: [PATCH] c++: Rebuild function type when it disagrees with formal
>   parameter types [PR92010]

OK, thanks.  Note that we're trying to keep the length of the git 
subject line (not the email subject line that adds [PATCH] and such) 
under 50 chars for the sake of things like git log --oneline.  Going 
over isn't a terrible thing, but please keep that in mind.

> gcc/cp/ChangeLog:
> 
> 	Core issues 1001 and 1322
> 	PR c++/92010
> 	* pt.c (rebuild_function_or_method_type): Split function out from ...
> 	(tsubst_function_type): ... here.
> 	(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                                  | 151 ++++++++++++++-----
>   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, 263 insertions(+), 37 deletions(-)
>   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 6122227c22f..256a937eace 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -13475,6 +13475,116 @@ lookup_explicit_specifier (tree v)
>     return *explicit_specifier_map->get (v);
>   }
>   
> +/* Given T, a FUNCTION_TYPE or METHOD_TYPE, construct and return a corresponding
> +   FUNCTION_TYPE or METHOD_TYPE whose return type is RETURN_TYPE, argument types
> +   are ARG_TYPES, and exception specification is RAISES, and otherwise is
> +   identical to T.  */
> +
> +static tree
> +rebuild_function_or_method_type (tree t, tree return_type, tree arg_types,
> +				 tree raises, tsubst_flags_t complain)
> +{
> +  gcc_assert (FUNC_OR_METHOD_TYPE_P (t));
> +
> +  tree new_type;
> +  if (TREE_CODE (t) == FUNCTION_TYPE)
> +    {
> +      new_type = build_function_type (return_type, arg_types);
> +      new_type = apply_memfn_quals (new_type, type_memfn_quals (t));
> +    }
> +  else
> +    {
> +      tree r = TREE_TYPE (TREE_VALUE (arg_types));
> +      /* Don't pick up extra function qualifiers from the basetype.  */
> +      r = cp_build_qualified_type_real (r, type_memfn_quals (t), complain);
> +      if (! MAYBE_CLASS_TYPE_P (r))
> +	{
> +	  /* [temp.deduct]
> +
> +	     Type deduction may fail for any of the following
> +	     reasons:
> +
> +	     -- Attempting to create "pointer to member of T" when T
> +	     is not a class type.  */
> +	  if (complain & tf_error)
> +	    error ("creating pointer to member function of non-class type %qT",
> +		   r);
> +	  return error_mark_node;
> +	}
> +
> +      new_type = build_method_type_directly (r, return_type,
> +					     TREE_CHAIN (arg_types));
> +    }
> +  new_type = cp_build_type_attribute_variant (new_type, TYPE_ATTRIBUTES (t));
> +
> +  cp_ref_qualifier rqual = type_memfn_rqual (t);
> +  bool late_return_type_p = TYPE_HAS_LATE_RETURN_TYPE (t);
> +  return build_cp_fntype_variant (new_type, rqual, raises, late_return_type_p);
> +}
> +
> +/* 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 fntype = TREE_TYPE (decl);
> +  tree parm_list = DECL_ARGUMENTS (decl);
> +  tree old_parm_type_list = TYPE_ARG_TYPES (fntype);
> +  tree new_parm_type_list = NULL_TREE;
> +  tree *q = &new_parm_type_list;
> +  for (int skip = num_artificial_parms_for (decl); skip > 0; skip--)
> +    {
> +      *q = copy_node (old_parm_type_list);
> +      parm_list = DECL_CHAIN (parm_list);
> +      old_parm_type_list = TREE_CHAIN (old_parm_type_list);
> +      q = &TREE_CHAIN (*q);
> +    }
> +  while (old_parm_type_list && old_parm_type_list != void_list_node)
> +    {
> +      *q = copy_node (old_parm_type_list);
> +      tree *new_parm_type = &TREE_VALUE (*q);
> +      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;
> +
> +      parm_list = DECL_CHAIN (parm_list);
> +      old_parm_type_list = TREE_CHAIN (old_parm_type_list);
> +      q = &TREE_CHAIN (*q);
> +    }
> +  if (old_parm_type_list == void_list_node)
> +    *q = void_list_node;
> +
> +  TREE_TYPE (decl)
> +    = rebuild_function_or_method_type (fntype,
> +				       TREE_TYPE (fntype), new_parm_type_list,
> +				       TYPE_RAISES_EXCEPTIONS (fntype), tf_none);
> +}
> +
>   /* Subroutine of tsubst_decl for the case when T is a FUNCTION_DECL.  */
>   
>   static tree
> @@ -13665,6 +13775,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;
> @@ -14694,7 +14806,6 @@ tsubst_function_type (tree t,
>   {
>     tree return_type;
>     tree arg_types = NULL_TREE;
> -  tree fntype;
>   
>     /* The TYPE_CONTEXT is not used for function/method types.  */
>     gcc_assert (TYPE_CONTEXT (t) == NULL_TREE);
> @@ -14765,42 +14876,8 @@ tsubst_function_type (tree t,
>       }
>   
>     /* Construct a new type node and return it.  */
> -  if (TREE_CODE (t) == FUNCTION_TYPE)
> -    {
> -      fntype = build_function_type (return_type, arg_types);
> -      fntype = apply_memfn_quals (fntype, type_memfn_quals (t));
> -    }
> -  else
> -    {
> -      tree r = TREE_TYPE (TREE_VALUE (arg_types));
> -      /* Don't pick up extra function qualifiers from the basetype.  */
> -      r = cp_build_qualified_type_real (r, type_memfn_quals (t), complain);
> -      if (! MAYBE_CLASS_TYPE_P (r))
> -	{
> -	  /* [temp.deduct]
> -
> -	     Type deduction may fail for any of the following
> -	     reasons:
> -
> -	     -- Attempting to create "pointer to member of T" when T
> -	     is not a class type.  */
> -	  if (complain & tf_error)
> -	    error ("creating pointer to member function of non-class type %qT",
> -		      r);
> -	  return error_mark_node;
> -	}
> -
> -      fntype = build_method_type_directly (r, return_type,
> -					   TREE_CHAIN (arg_types));
> -    }
> -  fntype = cp_build_type_attribute_variant (fntype, TYPE_ATTRIBUTES (t));
> -
> -  /* See comment above.  */
> -  tree raises = NULL_TREE;
> -  cp_ref_qualifier rqual = type_memfn_rqual (t);
> -  fntype = build_cp_fntype_variant (fntype, rqual, raises, late_return_type_p);
> -
> -  return fntype;
> +  return rebuild_function_or_method_type (t, return_type, arg_types,
> +					  /*raises=*/NULL_TREE, complain);
>   }
>   
>   /* FNTYPE is a FUNCTION_TYPE or METHOD_TYPE.  Substitute the template
> 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 ();
> +}
> +
>
Jeff Law via Gcc-patches April 8, 2020, 2:18 p.m. UTC | #16
On Tue, 7 Apr 2020, Jason Merrill wrote:
> On 4/7/20 1:40 PM, Patrick Palka wrote:
> > On Mon, 6 Apr 2020, Jason Merrill wrote:
> > > On 4/6/20 11:45 AM, Patrick Palka wrote:
> > > > 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.
> > > 
> > > Yes, thanks.
> > > 
> > > > -- >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);
> > > > +    }
> > > 
> > > The usual pattern for this sort of thing is to use a tree* to track the
> > > end of
> > > the new list, which should also avoid making a garbage copy of
> > > void_list_node.
> > > e.g. from tsubst_attribute:
> > > 
> > > >        tree list = NULL_TREE;
> > > >        tree *q = &list;
> > > >        for (int i = 0; i < len; ++i)
> > > >          {
> > > >            tree elt = TREE_VEC_ELT (pack, i);
> > > >            *q = build_tree_list (purp, elt);
> > > >            q = &TREE_CHAIN (*q);
> > > >          }
> > 
> > Ah so that's the right way do it :) Patch updated to make use of this
> > pattern.
> > 
> > This version of the patch is more complete.  It builds the new
> > FUNCTION_TYPE and METHOD_TYPE the same way that tsubst_function_type
> > does, by splitting out and reusing the relevant parts of
> > tsubst_function_type into a separate subroutine that is responsible for
> > propagating TYPE_ATTRIBUTES, TYPE_RAISES_EXCEPTION, ref-qualifiers, etc.
> > 
> > I wonder if for consistency and correctness we might have to update
> > other callers of tsubst_function_type/tsubst to make sure this
> > function-type-rebuilding based on parameter types is done in these
> > callers too.  For example, there is is_specialization_of_friend which
> > calls tsubst_function_type on the type of a function decl, and
> > fn_type_unification and determine_specialization which also call tsubst
> > on the type of a function decl (and pass the tf_fndecl_type flag).
> > 
> > If so, maybe we could instead leverage the tf_fndecl_type flag and the
> > 'in_decl' tsubst parameter to change tsubst_arg_types to immediately
> > build the function type according to the parameter types of in_decl
> > (which would presumably be the FUNCTION_DECL)?  That way, we would just
> > have to update the above potentially problematic callers to pass
> > tf_fndecl_type and set in_decl appropriately when calling tsubst and
> > would only have to build the function type once.
> > 
> > Patch partially tested on unbootstrapped x86_64-pc-linux-gnu, and
> > bootstrap/regtest is in progress.
> > 
> > -- >8 --
> > 
> > Subject: [PATCH] c++: Rebuild function type when it disagrees with formal
> >   parameter types [PR92010]
> 
> OK, thanks.  Note that we're trying to keep the length of the git subject line
> (not the email subject line that adds [PATCH] and such) under 50 chars for the
> sake of things like git log --oneline.  Going over isn't a terrible thing, but
> please keep that in mind.

Ah okay, noted.  To that end, I committed the patch with the subject
line changed to the shorter
    c++: Function type and parameter type disagreements [PR92010]

> 
> > gcc/cp/ChangeLog:
> > 
> > 	Core issues 1001 and 1322
> > 	PR c++/92010
> > 	* pt.c (rebuild_function_or_method_type): Split function out from ...
> > 	(tsubst_function_type): ... here.
> > 	(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                                  | 151 ++++++++++++++-----
> >   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, 263 insertions(+), 37 deletions(-)
> >   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 6122227c22f..256a937eace 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -13475,6 +13475,116 @@ lookup_explicit_specifier (tree v)
> >     return *explicit_specifier_map->get (v);
> >   }
> >   +/* Given T, a FUNCTION_TYPE or METHOD_TYPE, construct and return a
> > corresponding
> > +   FUNCTION_TYPE or METHOD_TYPE whose return type is RETURN_TYPE, argument
> > types
> > +   are ARG_TYPES, and exception specification is RAISES, and otherwise is
> > +   identical to T.  */
> > +
> > +static tree
> > +rebuild_function_or_method_type (tree t, tree return_type, tree arg_types,
> > +				 tree raises, tsubst_flags_t complain)
> > +{
> > +  gcc_assert (FUNC_OR_METHOD_TYPE_P (t));
> > +
> > +  tree new_type;
> > +  if (TREE_CODE (t) == FUNCTION_TYPE)
> > +    {
> > +      new_type = build_function_type (return_type, arg_types);
> > +      new_type = apply_memfn_quals (new_type, type_memfn_quals (t));
> > +    }
> > +  else
> > +    {
> > +      tree r = TREE_TYPE (TREE_VALUE (arg_types));
> > +      /* Don't pick up extra function qualifiers from the basetype.  */
> > +      r = cp_build_qualified_type_real (r, type_memfn_quals (t), complain);
> > +      if (! MAYBE_CLASS_TYPE_P (r))
> > +	{
> > +	  /* [temp.deduct]
> > +
> > +	     Type deduction may fail for any of the following
> > +	     reasons:
> > +
> > +	     -- Attempting to create "pointer to member of T" when T
> > +	     is not a class type.  */
> > +	  if (complain & tf_error)
> > +	    error ("creating pointer to member function of non-class type
> > %qT",
> > +		   r);
> > +	  return error_mark_node;
> > +	}
> > +
> > +      new_type = build_method_type_directly (r, return_type,
> > +					     TREE_CHAIN (arg_types));
> > +    }
> > +  new_type = cp_build_type_attribute_variant (new_type, TYPE_ATTRIBUTES
> > (t));
> > +
> > +  cp_ref_qualifier rqual = type_memfn_rqual (t);
> > +  bool late_return_type_p = TYPE_HAS_LATE_RETURN_TYPE (t);
> > +  return build_cp_fntype_variant (new_type, rqual, raises,
> > late_return_type_p);
> > +}
> > +
> > +/* 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 fntype = TREE_TYPE (decl);
> > +  tree parm_list = DECL_ARGUMENTS (decl);
> > +  tree old_parm_type_list = TYPE_ARG_TYPES (fntype);
> > +  tree new_parm_type_list = NULL_TREE;
> > +  tree *q = &new_parm_type_list;
> > +  for (int skip = num_artificial_parms_for (decl); skip > 0; skip--)
> > +    {
> > +      *q = copy_node (old_parm_type_list);
> > +      parm_list = DECL_CHAIN (parm_list);
> > +      old_parm_type_list = TREE_CHAIN (old_parm_type_list);
> > +      q = &TREE_CHAIN (*q);
> > +    }
> > +  while (old_parm_type_list && old_parm_type_list != void_list_node)
> > +    {
> > +      *q = copy_node (old_parm_type_list);
> > +      tree *new_parm_type = &TREE_VALUE (*q);
> > +      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;
> > +
> > +      parm_list = DECL_CHAIN (parm_list);
> > +      old_parm_type_list = TREE_CHAIN (old_parm_type_list);
> > +      q = &TREE_CHAIN (*q);
> > +    }
> > +  if (old_parm_type_list == void_list_node)
> > +    *q = void_list_node;
> > +
> > +  TREE_TYPE (decl)
> > +    = rebuild_function_or_method_type (fntype,
> > +				       TREE_TYPE (fntype), new_parm_type_list,
> > +				       TYPE_RAISES_EXCEPTIONS (fntype),
> > tf_none);
> > +}
> > +
> >   /* Subroutine of tsubst_decl for the case when T is a FUNCTION_DECL.  */
> >     static tree
> > @@ -13665,6 +13775,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;
> > @@ -14694,7 +14806,6 @@ tsubst_function_type (tree t,
> >   {
> >     tree return_type;
> >     tree arg_types = NULL_TREE;
> > -  tree fntype;
> >       /* The TYPE_CONTEXT is not used for function/method types.  */
> >     gcc_assert (TYPE_CONTEXT (t) == NULL_TREE);
> > @@ -14765,42 +14876,8 @@ tsubst_function_type (tree t,
> >       }
> >       /* Construct a new type node and return it.  */
> > -  if (TREE_CODE (t) == FUNCTION_TYPE)
> > -    {
> > -      fntype = build_function_type (return_type, arg_types);
> > -      fntype = apply_memfn_quals (fntype, type_memfn_quals (t));
> > -    }
> > -  else
> > -    {
> > -      tree r = TREE_TYPE (TREE_VALUE (arg_types));
> > -      /* Don't pick up extra function qualifiers from the basetype.  */
> > -      r = cp_build_qualified_type_real (r, type_memfn_quals (t), complain);
> > -      if (! MAYBE_CLASS_TYPE_P (r))
> > -	{
> > -	  /* [temp.deduct]
> > -
> > -	     Type deduction may fail for any of the following
> > -	     reasons:
> > -
> > -	     -- Attempting to create "pointer to member of T" when T
> > -	     is not a class type.  */
> > -	  if (complain & tf_error)
> > -	    error ("creating pointer to member function of non-class type
> > %qT",
> > -		      r);
> > -	  return error_mark_node;
> > -	}
> > -
> > -      fntype = build_method_type_directly (r, return_type,
> > -					   TREE_CHAIN (arg_types));
> > -    }
> > -  fntype = cp_build_type_attribute_variant (fntype, TYPE_ATTRIBUTES (t));
> > -
> > -  /* See comment above.  */
> > -  tree raises = NULL_TREE;
> > -  cp_ref_qualifier rqual = type_memfn_rqual (t);
> > -  fntype = build_cp_fntype_variant (fntype, rqual, raises,
> > late_return_type_p);
> > -
> > -  return fntype;
> > +  return rebuild_function_or_method_type (t, return_type, arg_types,
> > +					  /*raises=*/NULL_TREE, complain);
> >   }
> >     /* FNTYPE is a FUNCTION_TYPE or METHOD_TYPE.  Substitute the template
> > 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 ();
> > +}
> > +
> > 
> 
>
diff mbox series

Patch

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 9e1eb9416c9..923166276b8 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13337,7 +13337,22 @@  tsubst_default_argument (tree fn, int parmnum, tree type, tree arg,
   if (parmtype == error_mark_node)
     return error_mark_node;
 
-  gcc_assert (same_type_ignoring_top_level_qualifiers_p (type, parmtype));
+  if (same_type_ignoring_top_level_qualifiers_p (type, parmtype))
+    ;
+  else if (POINTER_TYPE_P (type) && POINTER_TYPE_P (parmtype))
+    {
+      /* The function type and the parameter type can disagree about the
+	 cv-qualification of the pointed-to type; see PR92010.  */
+      gcc_assert (same_type_p (TREE_TYPE (type),
+			       strip_top_quals (TREE_TYPE (parmtype))));
+      /* Verify that this happens only when the dependent parameter type is a
+	 cv-qualified wildcard type.  */
+      tree pattern_parm = get_pattern_parm (parm, DECL_TI_TEMPLATE (fn));
+      gcc_assert (WILDCARD_TYPE_P (TREE_TYPE (pattern_parm))
+		  && cv_qualified_p (TREE_TYPE (pattern_parm)));
+    }
+  else
+    gcc_unreachable ();
 
   tree *slot;
   if (defarg_inst && (slot = defarg_inst->get (parm)))
diff --git a/gcc/testsuite/g++.dg/template/defarg22.C b/gcc/testsuite/g++.dg/template/defarg22.C
new file mode 100644
index 00000000000..cf6261916d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/defarg22.C
@@ -0,0 +1,11 @@ 
+// PR c++/92010
+// { dg-do compile }
+
+template <typename T>
+void foo(const T t = 0)
+{ }
+
+int main()
+{
+  foo<char[]>();
+}