public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
@ 2015-12-24 17:58 Patrick Palka
  2015-12-25  2:41 ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2015-12-24 17:58 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason, Patrick Palka

This patch fixes the issue where we do not fail when, during
substitution, we end up creating a single-dimension array parameter of
type void, or an array parameter that has a zero or negative major
bound.  This issue occurs because we decay an array parameter type into
a pointer type as soon as we process the parameter (in grokdeclarator),
so we lose the array type's possibly dependent bounds and so on.

The obvious fix for this issue is to withhold decaying dependent array
parameter types until after substitution is done on them (and to
temporarily decay array parameter types for unification).  That is the
approach that this patch takes.

So first off, in grokdeclarator we just have to retain the array
parameter type if it's dependent (and avoid messing with the element
type's cv qualifiers in grokparms).

On the substitution side of things, nothing extra needs to be done as
tsubst_arg_types already takes care to decay the array parameter type
after substitution.  Things get slightly more tricky on the unification
side.

For unification, we need to add a special case to unify a dependent T[N]
parameter type with a T * argument type so that type deduction, template
function overload resolution, specializations, etc still work properly
in such cases.  After a couple of failed approaches that attempted to
localize the requisite changes to a single common function, e.g. in
unify() or in unify_one_arg() or in maybe_adjust_types_for_deduction() I
realized that the interface for unification is not used consistently
enough to trivially achieve this. For example, a change to unify() will
not work well because one of its callers, more_specialized_fn(), strips
REFERENCE_TYPEs before calling it so we may be inadvertently unifying a
T (&)[N] with a T *& which seems wrong.  So instead, this patch
takes the easier route and just adds preparatory logic to decay these
dependent array parameter types where necessary so that by the time
unify() is called it will be looking at two decayed T * types.  There
only seem to be three places where this needs to be done.

Aside from the two tests derived from the relevant PRs, the new tests
unify12.C - unify16.C are all examples for which my earlier iterations
of this patch introduced regressions.  They run cleanly with or without
this patch.

Tested via bootstrap + regtest on x86_64-pc-linux-gnu, and also tested by
compiling boost and running its testsuite.  Although boost's testsuite
is somewhat spotty, from what I can tell no new regressions were
introduced.

gcc/cp/ChangeLog:

	PR c++/11858
	PR c++/24663
	PR c++/24664
	* decl.c (grokdeclarator): Don't decay array parameter type to
	a pointer type if it's dependent.
	(grokparms): Invoke strip_top_quals instead of directly invoking
	cp_build_qualified_type.
	* pt.c (decay_dependent_array_parm_type): New static function.
	(type_unification_real): Call decay_dependent_array_parm_type
	to decay a dependent array parameter type to its corresponding
	pointer type before unification.
	(more_specialized_fn): Likewise.
	(get_bindings): Likewise.
	* tree.c (cp_build_qualified_type): Trivial typofix in
	documentation.

gcc/testsuite/ChangeLog:

	PR c++/11858
	PR c++/24663
	PR c++/24664
	* g++.dg/template/pr11858.C: New test.
	* g++.dg/template/pr24663.C: New test.
	* g++.dg/template/unify12.C: New test.
	* g++.dg/template/unify13.C: New test.
	* g++.dg/template/unify14.C: New test.
	* g++.dg/template/unify15.C: New test.
	* g++.dg/template/unify16.C: New test.
---
 gcc/cp/decl.c                           | 12 +++++--
 gcc/cp/pt.c                             | 27 +++++++++++++++-
 gcc/cp/tree.c                           |  2 +-
 gcc/testsuite/g++.dg/template/pr11858.C |  5 +++
 gcc/testsuite/g++.dg/template/pr24663.C | 22 +++++++++++++
 gcc/testsuite/g++.dg/template/unify12.C | 46 +++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/template/unify13.C | 26 +++++++++++++++
 gcc/testsuite/g++.dg/template/unify14.C |  5 +++
 gcc/testsuite/g++.dg/template/unify15.C | 15 +++++++++
 gcc/testsuite/g++.dg/template/unify16.C | 56 +++++++++++++++++++++++++++++++++
 10 files changed, 211 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/pr11858.C
 create mode 100644 gcc/testsuite/g++.dg/template/pr24663.C
 create mode 100644 gcc/testsuite/g++.dg/template/unify12.C
 create mode 100644 gcc/testsuite/g++.dg/template/unify13.C
 create mode 100644 gcc/testsuite/g++.dg/template/unify14.C
 create mode 100644 gcc/testsuite/g++.dg/template/unify15.C
 create mode 100644 gcc/testsuite/g++.dg/template/unify16.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index af5f265..1c7dfe6 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -10898,8 +10898,13 @@ grokdeclarator (const cp_declarator *declarator,
 
       if (TREE_CODE (type) == ARRAY_TYPE)
 	{
-	  /* Transfer const-ness of array into that of type pointed to.  */
-	  type = build_pointer_type (TREE_TYPE (type));
+	  /* Withhold decaying a dependent array type so that that during
+	     instantiation we can detect type deduction failure cases such as
+	     creating an array of void, creating a zero-size array, etc.  */
+	  if (dependent_type_p (type))
+	    ;
+	  else
+	    type = build_pointer_type (TREE_TYPE (type));
 	  type_quals = TYPE_UNQUALIFIED;
 	  array_parameter_p = true;
 	}
@@ -11696,7 +11701,8 @@ grokparms (tree parmlist, tree *parms)
 
 	  /* Top-level qualifiers on the parameters are
 	     ignored for function types.  */
-	  type = cp_build_qualified_type (type, 0);
+	  type = strip_top_quals (type);
+
 	  if (TREE_CODE (type) == METHOD_TYPE)
 	    {
 	      error ("parameter %qD invalidly declared method type", decl);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index dab15bd..35b017e 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -17726,6 +17726,23 @@ fn_type_unification (tree fn,
   return r;
 }
 
+/* TYPE is the type of a function parameter.  If TYPE is a (dependent)
+   ARRAY_TYPE, return the corresponding POINTER_TYPE to which it decays.
+   Otherwise return TYPE.  (We shouldn't see non-dependent ARRAY_TYPE
+   parameters because they get decayed as soon as they are declared.)  */
+
+static tree
+decay_dependent_array_parm_type (tree type)
+{
+  if (TREE_CODE (type) == ARRAY_TYPE)
+    {
+      gcc_assert (uses_template_parms (type));
+      return type_decays_to (type);
+    }
+
+  return type;
+}
+
 /* Adjust types before performing type deduction, as described in
    [temp.deduct.call] and [temp.deduct.conv].  The rules in these two
    sections are symmetric.  PARM is the type of a function parameter
@@ -18164,6 +18181,8 @@ type_unification_real (tree tparms,
       arg = args[ia];
       ++ia;
 
+      parm = decay_dependent_array_parm_type (parm);
+
       if (unify_one_argument (tparms, targs, parm, arg, subr, strict,
 			      explain_p))
 	return 1;
@@ -20166,6 +20185,9 @@ more_specialized_fn (tree pat1, tree pat2, int len)
           len = 0;
         }
 
+      arg1 = decay_dependent_array_parm_type (arg1);
+      arg2 = decay_dependent_array_parm_type (arg2);
+
       if (TREE_CODE (arg1) == REFERENCE_TYPE)
 	{
 	  ref1 = TYPE_REF_IS_RVALUE (arg1) + 1;
@@ -20451,7 +20473,10 @@ get_bindings (tree fn, tree decl, tree explicit_args, bool check_rettype)
   for (arg = decl_arg_types, ix = 0;
        arg != NULL_TREE && arg != void_list_node;
        arg = TREE_CHAIN (arg), ++ix)
-    args[ix] = TREE_VALUE (arg);
+    {
+      args[ix] = TREE_VALUE (arg);
+      args[ix] = decay_dependent_array_parm_type (args[ix]);
+    }
 
   if (fn_type_unification (fn, explicit_args, targs,
 			   args, ix,
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 250fe27..8e3dbce 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -1012,7 +1012,7 @@ c_build_qualified_type (tree type, int type_quals, tree /* orig_qual_type */,
    arrays correctly.  In particular, if TYPE is an array of T's, and
    TYPE_QUALS is non-empty, returns an array of qualified T's.
 
-   FLAGS determines how to deal with ill-formed qualifications. If
+   COMPLAIN determines how to deal with ill-formed qualifications. If
    tf_ignore_bad_quals is set, then bad qualifications are dropped
    (this is permitted if TYPE was introduced via a typedef or template
    type parameter). If bad qualifications are dropped and tf_warning
diff --git a/gcc/testsuite/g++.dg/template/pr11858.C b/gcc/testsuite/g++.dg/template/pr11858.C
new file mode 100644
index 0000000..dc0d688
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr11858.C
@@ -0,0 +1,5 @@
+// PR c++/11858
+
+template <typename T> struct S { static typename T::x f (); }; // { dg-error "" }
+template <class T> int f (int [sizeof(T::f())]);
+int const i = f<S<int> >(0); // { dg-error "no matching function" }
diff --git a/gcc/testsuite/g++.dg/template/pr24663.C b/gcc/testsuite/g++.dg/template/pr24663.C
new file mode 100644
index 0000000..2dc68c2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr24663.C
@@ -0,0 +1,22 @@
+// PR c++/24663
+
+template<int I> int f1 (char[I]);
+template<int I> int f1 (char p1 = I);
+int i = f1<0>(0);
+
+template<typename T, int I> int f2 (T[I]); // { dg-error "" }
+int j = f2<int, 0>(0); // { dg-error "no matching function" }
+int k = f2<void, 1>(0); // { dg-error "no matching function" }
+
+int o[5];
+int l = f2<int[5], 1>(&o);
+
+template<int I> int f3 (char [][I]);
+template<int I> int f3 (char p1 = I);
+int x1 = f3<1>(0); // { dg-error "is ambiguous" }
+int x2 = f3<1>();
+
+template<typename T, int I> int f4 (T [][I]); // { dg-error "" }
+int y1 = f4<void, 1>(0); // { dg-error "no matching function" }
+int y2 = f4<int (void), 1>(0); // { dg-error "no matching function" }
+int y3 = f4<int&, 1>(0); // { dg-error "no matching function" }
diff --git a/gcc/testsuite/g++.dg/template/unify12.C b/gcc/testsuite/g++.dg/template/unify12.C
new file mode 100644
index 0000000..6e624e4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/unify12.C
@@ -0,0 +1,46 @@
+// { dg-do run }
+#include <cassert>
+
+template<typename T, int I> int foo (T [I][I]) { return 0; }
+
+template int foo (char [][6]);
+
+template <typename T>
+int foo (T *)
+{
+  return -1;
+}
+
+template <typename T>
+int foo (T [3][3])
+{
+  return 1;
+}
+
+template <int I>
+int foo (bool [I][I])
+{
+  return 2;
+}
+
+template <>
+int foo (bool [3][2])
+{
+  return 3;
+}
+
+char x[3];
+bool y[4];
+bool z[3][2];
+
+int a = foo (&x);
+int b = foo (&y);
+int c = foo (z);
+
+int
+main ()
+{
+  assert (a == 1);
+  assert (b == 2);
+  assert (c == 3);
+}
diff --git a/gcc/testsuite/g++.dg/template/unify13.C b/gcc/testsuite/g++.dg/template/unify13.C
new file mode 100644
index 0000000..56a46df
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/unify13.C
@@ -0,0 +1,26 @@
+// { dg-do run }
+#include <cassert>
+
+template<typename T, int I> int foo (T [I][I]) { return 0; }
+
+template<typename T>
+int foo (T [3][2])
+{
+  return 1;
+}
+
+template <>
+int foo (bool [3][2])
+{
+  return 2;
+}
+
+bool z[3][2];
+
+int a = foo (z);
+
+int
+main ()
+{
+  assert (a == 2);
+}
diff --git a/gcc/testsuite/g++.dg/template/unify14.C b/gcc/testsuite/g++.dg/template/unify14.C
new file mode 100644
index 0000000..7fda8fd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/unify14.C
@@ -0,0 +1,5 @@
+template <typename T, int X>
+void bar (T [X]) { }
+
+template <typename T, int X>
+void bar (const T [X]) { }
diff --git a/gcc/testsuite/g++.dg/template/unify15.C b/gcc/testsuite/g++.dg/template/unify15.C
new file mode 100644
index 0000000..fe4848b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/unify15.C
@@ -0,0 +1,15 @@
+// { dg-do run }
+#include <cassert>
+
+template <typename T, int N>
+int bar (T (&) [N]) { return 0; }
+
+template <typename T, int N>
+int bar (const T (&) [N]) { return 1; }
+
+int
+main ()
+{
+  const int s[2] = { 0 };
+  assert (bar (s) == 1);
+}
diff --git a/gcc/testsuite/g++.dg/template/unify16.C b/gcc/testsuite/g++.dg/template/unify16.C
new file mode 100644
index 0000000..7b5a2aa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/unify16.C
@@ -0,0 +1,56 @@
+// { dg-do run }
+#include <cassert>
+
+template <typename T>
+struct Foo
+{
+  static int foo (T) { return 0; }
+};
+
+template <typename T, int I>
+struct Foo<T[I]>
+{
+  static int foo (T[I]) { return 1; }
+};
+
+template <int I>
+struct Foo<char[I]>
+{
+  static int foo (char[I]) { return 2; }
+};
+
+template <typename T>
+struct Foo<T[5]>
+{
+  static int foo (T[5]) { return 3; }
+};
+
+template <>
+struct Foo<char[5]>
+{
+  static int foo (char[5]) { return 4; }
+};
+
+template <>
+struct Foo<const char[5]>
+{
+  static int foo (const char[5]) { return 5; }
+};
+
+int a = Foo<const char[5]>::foo (0);
+int b = Foo<char[5]>::foo (0);
+int c = Foo<bool[5]>::foo (0);
+int d = Foo<char[4]>::foo (0);
+int e = Foo<bool[4]>::foo (0);
+int f = Foo<char[]>::foo (0);
+
+int
+main (void)
+{
+  assert (a == 5);
+  assert (b == 4);
+  assert (c == 3);
+  assert (d == 2);
+  assert (e == 1);
+  assert (f == 0);
+}
-- 
2.7.0.rc1.98.gacf58d0.dirty

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2015-12-24 17:58 [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early) Patrick Palka
@ 2015-12-25  2:41 ` Jason Merrill
  2015-12-25 17:37   ` Patrick Palka
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2015-12-25  2:41 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

On 12/24/2015 12:57 PM, Patrick Palka wrote:
> So instead, this patch
> takes the easier route and just adds preparatory logic to decay these
> dependent array parameter types where necessary so that by the time
> unify() is called it will be looking at two decayed T * types.  There
> only seem to be three places where this needs to be done.

Does it not make sense to do this in maybe_adjust_types_for_deduction?

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2015-12-25  2:41 ` Jason Merrill
@ 2015-12-25 17:37   ` Patrick Palka
  2016-01-18 15:34     ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2015-12-25 17:37 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Thu, Dec 24, 2015 at 9:41 PM, Jason Merrill <jason@redhat.com> wrote:
> On 12/24/2015 12:57 PM, Patrick Palka wrote:
>>
>> So instead, this patch
>> takes the easier route and just adds preparatory logic to decay these
>> dependent array parameter types where necessary so that by the time
>> unify() is called it will be looking at two decayed T * types.  There
>> only seem to be three places where this needs to be done.
>
>
> Does it not make sense to do this in maybe_adjust_types_for_deduction?

That alone would not be sufficient because more_specialized_fn()
doesn't call maybe_adjust_types_for_deduction() beforehand, yet we
have to do the decaying there too (and on both types, not just one of
them).

And maybe_adjust_types_for_deduction() seems to operate on the
presumption that one type is the parameter type and one is the
argument type. But in more_specialized_fn() and in get_bindings() we
are really working with two parameter types and have to decay them
both. So sometimes we have to decay one of the types that are
eventually going to get passed to unify(), and other times we want to
decay both types that are going to get passed to unify().
maybe_adjust_types_for_deduction() seems to only expect the former
case.

Finally, maybe_adjust_types_for_deduction() is not called when
unifying a nested function declarator (because it is guarded by the
subr flag in unify_one_argument), so doing it there we would also
regress in the following test case:

void foo (int *);

template <typename T>
void bar (void (T[5]));

void
baz (void)
{
  bar (foo); // type of function foo will no longer unify with void
(T[5]) so deduction of type T now fails
}

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2015-12-25 17:37   ` Patrick Palka
@ 2016-01-18 15:34     ` Jason Merrill
  2016-01-18 19:13       ` Patrick Palka
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2016-01-18 15:34 UTC (permalink / raw)
  To: Patrick Palka; +Cc: GCC Patches

On 12/25/2015 12:37 PM, Patrick Palka wrote:
> That alone would not be sufficient because more_specialized_fn()
> doesn't call maybe_adjust_types_for_deduction() beforehand, yet we
> have to do the decaying there too (and on both types, not just one of
> them).
>
> And maybe_adjust_types_for_deduction() seems to operate on the
> presumption that one type is the parameter type and one is the
> argument type. But in more_specialized_fn() and in get_bindings() we
> are really working with two parameter types and have to decay them
> both. So sometimes we have to decay one of the types that are
> eventually going to get passed to unify(), and other times we want to
> decay both types that are going to get passed to unify().
> maybe_adjust_types_for_deduction() seems to only expect the former
> case.
>
> Finally, maybe_adjust_types_for_deduction() is not called when
> unifying a nested function declarator (because it is guarded by the
> subr flag in unify_one_argument), so doing it there we would also
> regress in the following test case:

Ah, that makes sense.

How about keeping the un-decayed type in the PARM_DECLs, so that we get 
the substitution failure in instantiate_template, but having the decayed 
type in the TYPE_ARG_TYPES, probably by doing the decay in grokparms, so 
it's already decayed when we're doing unification?

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-18 15:34     ` Jason Merrill
@ 2016-01-18 19:13       ` Patrick Palka
  2016-01-18 21:04         ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2016-01-18 19:13 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Mon, Jan 18, 2016 at 10:34 AM, Jason Merrill <jason@redhat.com> wrote:
> On 12/25/2015 12:37 PM, Patrick Palka wrote:
>>
>> That alone would not be sufficient because more_specialized_fn()
>> doesn't call maybe_adjust_types_for_deduction() beforehand, yet we
>> have to do the decaying there too (and on both types, not just one of
>> them).
>>
>> And maybe_adjust_types_for_deduction() seems to operate on the
>> presumption that one type is the parameter type and one is the
>> argument type. But in more_specialized_fn() and in get_bindings() we
>> are really working with two parameter types and have to decay them
>> both. So sometimes we have to decay one of the types that are
>> eventually going to get passed to unify(), and other times we want to
>> decay both types that are going to get passed to unify().
>> maybe_adjust_types_for_deduction() seems to only expect the former
>> case.
>>
>> Finally, maybe_adjust_types_for_deduction() is not called when
>> unifying a nested function declarator (because it is guarded by the
>> subr flag in unify_one_argument), so doing it there we would also
>> regress in the following test case:
>
>
> Ah, that makes sense.
>
> How about keeping the un-decayed type in the PARM_DECLs, so that we get the
> substitution failure in instantiate_template, but having the decayed type in
> the TYPE_ARG_TYPES, probably by doing the decay in grokparms, so it's
> already decayed when we're doing unification?

I just tried this, and it works well! With this approach, all but one
of the test cases pass.  The failing test case is unify17.C:

-- 8< --

void foo (int *);

template <typename T>
void bar (void (T[5])); // { dg-error "array of 'void'" }

void
baz (void)
{
  bar<void> (0); // { dg-error "no matching function" }
}

-- 8< --

Here, we don't get a substitution failure because we don't have a
corresponding FUNCTION_DECL for the nested function specifier, only a
FUNCTION_TYPE. So there is no PARM_DECL to recurse into during
substitution, that retains the un-decayed argument type "T[5]" of the
nested function specifier.

I'm not yet sure how to work around this...

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-18 19:13       ` Patrick Palka
@ 2016-01-18 21:04         ` Jason Merrill
  2016-01-19 21:10           ` Patrick Palka
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2016-01-18 21:04 UTC (permalink / raw)
  To: Patrick Palka; +Cc: GCC Patches

On 01/18/2016 02:12 PM, Patrick Palka wrote:
> On Mon, Jan 18, 2016 at 10:34 AM, Jason Merrill <jason@redhat.com> wrote:
>> On 12/25/2015 12:37 PM, Patrick Palka wrote:
>>>
>>> That alone would not be sufficient because more_specialized_fn()
>>> doesn't call maybe_adjust_types_for_deduction() beforehand, yet we
>>> have to do the decaying there too (and on both types, not just one of
>>> them).
>>>
>>> And maybe_adjust_types_for_deduction() seems to operate on the
>>> presumption that one type is the parameter type and one is the
>>> argument type. But in more_specialized_fn() and in get_bindings() we
>>> are really working with two parameter types and have to decay them
>>> both. So sometimes we have to decay one of the types that are
>>> eventually going to get passed to unify(), and other times we want to
>>> decay both types that are going to get passed to unify().
>>> maybe_adjust_types_for_deduction() seems to only expect the former
>>> case.
>>>
>>> Finally, maybe_adjust_types_for_deduction() is not called when
>>> unifying a nested function declarator (because it is guarded by the
>>> subr flag in unify_one_argument), so doing it there we would also
>>> regress in the following test case:
>>
>>
>> Ah, that makes sense.
>>
>> How about keeping the un-decayed type in the PARM_DECLs, so that we get the
>> substitution failure in instantiate_template, but having the decayed type in
>> the TYPE_ARG_TYPES, probably by doing the decay in grokparms, so it's
>> already decayed when we're doing unification?
>
> I just tried this, and it works well! With this approach, all but one
> of the test cases pass.  The failing test case is unify17.C:
>
> -- 8< --
>
> void foo (int *);
>
> template <typename T>
> void bar (void (T[5])); // { dg-error "array of 'void'" }
>
> void
> baz (void)
> {
>    bar<void> (0); // { dg-error "no matching function" }
> }
>
> -- 8< --
>
> Here, we don't get a substitution failure because we don't have a
> corresponding FUNCTION_DECL for the nested function specifier, only a
> FUNCTION_TYPE. So there is no PARM_DECL to recurse into during
> substitution, that retains the un-decayed argument type "T[5]" of the
> nested function specifier.

Then your original patch is OK.  Thanks for indulging me.

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-18 21:04         ` Jason Merrill
@ 2016-01-19 21:10           ` Patrick Palka
  2016-01-20  3:30             ` Patrick Palka
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2016-01-19 21:10 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Mon, Jan 18, 2016 at 4:04 PM, Jason Merrill <jason@redhat.com> wrote:
> On 01/18/2016 02:12 PM, Patrick Palka wrote:
>>
>> On Mon, Jan 18, 2016 at 10:34 AM, Jason Merrill <jason@redhat.com> wrote:
>>>
>>> On 12/25/2015 12:37 PM, Patrick Palka wrote:
>>>>
>>>>
>>>> That alone would not be sufficient because more_specialized_fn()
>>>> doesn't call maybe_adjust_types_for_deduction() beforehand, yet we
>>>> have to do the decaying there too (and on both types, not just one of
>>>> them).
>>>>
>>>> And maybe_adjust_types_for_deduction() seems to operate on the
>>>> presumption that one type is the parameter type and one is the
>>>> argument type. But in more_specialized_fn() and in get_bindings() we
>>>> are really working with two parameter types and have to decay them
>>>> both. So sometimes we have to decay one of the types that are
>>>> eventually going to get passed to unify(), and other times we want to
>>>> decay both types that are going to get passed to unify().
>>>> maybe_adjust_types_for_deduction() seems to only expect the former
>>>> case.
>>>>
>>>> Finally, maybe_adjust_types_for_deduction() is not called when
>>>> unifying a nested function declarator (because it is guarded by the
>>>> subr flag in unify_one_argument), so doing it there we would also
>>>> regress in the following test case:
>>>
>>>
>>>
>>> Ah, that makes sense.
>>>
>>> How about keeping the un-decayed type in the PARM_DECLs, so that we get
>>> the
>>> substitution failure in instantiate_template, but having the decayed type
>>> in
>>> the TYPE_ARG_TYPES, probably by doing the decay in grokparms, so it's
>>> already decayed when we're doing unification?
>>
>>
>> I just tried this, and it works well! With this approach, all but one
>> of the test cases pass.  The failing test case is unify17.C:
>>
>> -- 8< --
>>
>> void foo (int *);
>>
>> template <typename T>
>> void bar (void (T[5])); // { dg-error "array of 'void'" }
>>
>> void
>> baz (void)
>> {
>>    bar<void> (0); // { dg-error "no matching function" }
>> }
>>
>> -- 8< --
>>
>> Here, we don't get a substitution failure because we don't have a
>> corresponding FUNCTION_DECL for the nested function specifier, only a
>> FUNCTION_TYPE. So there is no PARM_DECL to recurse into during
>> substitution, that retains the un-decayed argument type "T[5]" of the
>> nested function specifier.
>
>
> Then your original patch is OK.  Thanks for indulging me.

I have committed the original patch, but I just noticed that it has
the unintended effect of changing whether certain template
declarations are considered duplicates or not.  For instance, in the
following test case we no longer consider the three declarations of
"foo" to be duplicates and we instead register three overloads for
foo, so that when we go to use foo later on we now get an ambiguity
error:

$ cat hmmmmm.cc
template <typename T>
int foo (T [4]);

template <typename T>
int foo (T [3]);

template <typename T>
int foo (T *);

int x = foo<int> (0);
$ g++ hmmmmm.cc
hmmmmm.cc:10:20: error: call of overloaded ‘foo(int)’ is ambiguous
 int x = foo<int> (0);
                    ^
hmmmmm.cc:2:5: note: candidate: int foo(T [4]) [with T = int]
 int foo (T [4]);
     ^~~
hmmmmm.cc:5:5: note: candidate: int foo(T [3]) [with T = int]
 int foo (T [3]);
     ^~~
hmmmmm.cc:8:5: note: candidate: int foo(T*) [with T = int]
 int foo (T *);
     ^~~


Before the patch, this test case would have compiled cleanly because
each subsequent declaration of foo would have been considered a
duplicate of the first one (since we would have decayed the array
parameter types early on).

I am not sure whether the previous or current behavior is correct, or
if maybe the first two decls ought to be considered duplicates and the
last one not.  What do you think?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-19 21:10           ` Patrick Palka
@ 2016-01-20  3:30             ` Patrick Palka
  2016-01-21 18:43               ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2016-01-20  3:30 UTC (permalink / raw)
  To: Patrick Palka; +Cc: Jason Merrill, GCC Patches

[-- Attachment #1: Type: text/plain, Size: 9979 bytes --]

On Tue, 19 Jan 2016, Patrick Palka wrote:

> On Mon, Jan 18, 2016 at 4:04 PM, Jason Merrill <jason@redhat.com> wrote:
>> On 01/18/2016 02:12 PM, Patrick Palka wrote:
>>>
>>> On Mon, Jan 18, 2016 at 10:34 AM, Jason Merrill <jason@redhat.com> wrote:
>>>>
>>>> On 12/25/2015 12:37 PM, Patrick Palka wrote:
>>>>>
>>>>>
>>>>> That alone would not be sufficient because more_specialized_fn()
>>>>> doesn't call maybe_adjust_types_for_deduction() beforehand, yet we
>>>>> have to do the decaying there too (and on both types, not just one of
>>>>> them).
>>>>>
>>>>> And maybe_adjust_types_for_deduction() seems to operate on the
>>>>> presumption that one type is the parameter type and one is the
>>>>> argument type. But in more_specialized_fn() and in get_bindings() we
>>>>> are really working with two parameter types and have to decay them
>>>>> both. So sometimes we have to decay one of the types that are
>>>>> eventually going to get passed to unify(), and other times we want to
>>>>> decay both types that are going to get passed to unify().
>>>>> maybe_adjust_types_for_deduction() seems to only expect the former
>>>>> case.
>>>>>
>>>>> Finally, maybe_adjust_types_for_deduction() is not called when
>>>>> unifying a nested function declarator (because it is guarded by the
>>>>> subr flag in unify_one_argument), so doing it there we would also
>>>>> regress in the following test case:
>>>>
>>>>
>>>>
>>>> Ah, that makes sense.
>>>>
>>>> How about keeping the un-decayed type in the PARM_DECLs, so that we get
>>>> the
>>>> substitution failure in instantiate_template, but having the decayed type
>>>> in
>>>> the TYPE_ARG_TYPES, probably by doing the decay in grokparms, so it's
>>>> already decayed when we're doing unification?
>>>
>>>
>>> I just tried this, and it works well! With this approach, all but one
>>> of the test cases pass.  The failing test case is unify17.C:
>>>
>>> -- 8< --
>>>
>>> void foo (int *);
>>>
>>> template <typename T>
>>> void bar (void (T[5])); // { dg-error "array of 'void'" }
>>>
>>> void
>>> baz (void)
>>> {
>>>    bar<void> (0); // { dg-error "no matching function" }
>>> }
>>>
>>> -- 8< --
>>>
>>> Here, we don't get a substitution failure because we don't have a
>>> corresponding FUNCTION_DECL for the nested function specifier, only a
>>> FUNCTION_TYPE. So there is no PARM_DECL to recurse into during
>>> substitution, that retains the un-decayed argument type "T[5]" of the
>>> nested function specifier.
>>
>>
>> Then your original patch is OK.  Thanks for indulging me.
>
> I have committed the original patch, but I just noticed that it has
> the unintended effect of changing whether certain template
> declarations are considered duplicates or not.  For instance, in the
> following test case we no longer consider the three declarations of
> "foo" to be duplicates and we instead register three overloads for
> foo, so that when we go to use foo later on we now get an ambiguity
> error:
>
> $ cat hmmmmm.cc
> template <typename T>
> int foo (T [4]);
>
> template <typename T>
> int foo (T [3]);
>
> template <typename T>
> int foo (T *);
>
> int x = foo<int> (0);
> $ g++ hmmmmm.cc
> hmmmmm.cc:10:20: error: call of overloaded ‘foo(int)’ is ambiguous
> int x = foo<int> (0);
>                    ^
> hmmmmm.cc:2:5: note: candidate: int foo(T [4]) [with T = int]
> int foo (T [4]);
>     ^~~
> hmmmmm.cc:5:5: note: candidate: int foo(T [3]) [with T = int]
> int foo (T [3]);
>     ^~~
> hmmmmm.cc:8:5: note: candidate: int foo(T*) [with T = int]
> int foo (T *);
>     ^~~
>
>
> Before the patch, this test case would have compiled cleanly because
> each subsequent declaration of foo would have been considered a
> duplicate of the first one (since we would have decayed the array
> parameter types early on).
>
> I am not sure whether the previous or current behavior is correct, or
> if maybe the first two decls ought to be considered duplicates and the
> last one not.  What do you think?
>

A much more significant side effect of this patch is that it changes the
mangling of function templates that have dependent array parameters.
For the following test case:

     template <typename T, int I>
     int foo (T [I]);

     int x = foo<int, 5> (0);

We now output

   _Z3fooIiLi5EEiAT0__T_ (aka "int foo<int, 5>(int [5])")

instead of

   _Z3fooIiLi5EEiPT_ (aka "int foo<int, 5>(int*)")

because when mangling a specialized FUNCTION_DECL the mangler uses the
undecayed TYPE_ARG_TYPES (TREE_TYPE (...)) of the corresponding
TEMPLATE_DECL instead of using the decayed
TYPE_ARG_TYPES (TREE_TYPE (...)) of that FUNCTION_DECL.

In light of this, I am inclined to instead use your suggested approach
(to decay the parameter types in grokparms, which become the
FUNCTION_TYPE's TYPE_ARG_TYPES, while retaining the un-decayed types in
the PARM_DECLs) since that approach does not exhibit this mangling issue
or the above mentioned overloading issue.  Here is a patch for that.

-- 8< --

Subject: [PATCH] Use a different approach for avoiding to decay array
  parameters too early

gcc/cp/ChangeLog:

 	PR c++/11858
 	PR c++/24663
 	PR c++/24664
 	* decl.c (grokparms): Decay each parameter type after stripping
 	its qualifiers.
 	* pt.c (tsubst_decl) [FUNCTION_DECL]: Return error_mark_node if
 	substition of the DECL_ARGUMENTS fails.
 	(decay_dependent_array_parm_type): Delete.
 	(type_unification_real): Remove calls to
 	decay_dependent_array_parm_type.
 	(more_specialized_fn): Likewise.
 	(get_bindings): Likewise.

gcc/testsuite/ChangeLog:

 	PR c++/11858
 	PR c++/24663
 	PR c++/24664
 	* g++.dg/template/mangle2.C: New test.
 	* g++.dg/template/unify17.C: XFAIL.
---
  gcc/cp/decl.c                           |  1 +
  gcc/cp/pt.c                             | 31 +++++--------------------------
  gcc/testsuite/g++.dg/template/mangle2.C | 22 ++++++++++++++++++++++
  gcc/testsuite/g++.dg/template/unify17.C |  4 ++--
  4 files changed, 30 insertions(+), 28 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/template/mangle2.C

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index ceeef60..d681281 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -11702,6 +11702,7 @@ grokparms (tree parmlist, tree *parms)
  	  /* Top-level qualifiers on the parameters are
  	     ignored for function types.  */
  	  type = strip_top_quals (type);
+	  type = type_decays_to (type);

  	  if (TREE_CODE (type) == METHOD_TYPE)
  	    {
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index fdef601..a7084a1 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11740,6 +11740,10 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain)
  				     complain, t);
  	DECL_RESULT (r) = NULL_TREE;

+	for (tree parm = DECL_ARGUMENTS (r); parm; parm = DECL_CHAIN (parm))
+	  if (TREE_TYPE (parm) == error_mark_node)
+	    RETURN (error_mark_node);
+
  	TREE_STATIC (r) = 0;
  	TREE_PUBLIC (r) = TREE_PUBLIC (t);
  	DECL_EXTERNAL (r) = 1;
@@ -17742,23 +17746,6 @@ fn_type_unification (tree fn,
    return r;
  }

-/* TYPE is the type of a function parameter.  If TYPE is a (dependent)
-   ARRAY_TYPE, return the corresponding POINTER_TYPE to which it decays.
-   Otherwise return TYPE.  (We shouldn't see non-dependent ARRAY_TYPE
-   parameters because they get decayed as soon as they are declared.)  */
-
-static tree
-decay_dependent_array_parm_type (tree type)
-{
-  if (TREE_CODE (type) == ARRAY_TYPE)
-    {
-      gcc_assert (uses_template_parms (type));
-      return type_decays_to (type);
-    }
-
-  return type;
-}
-
  /* Adjust types before performing type deduction, as described in
     [temp.deduct.call] and [temp.deduct.conv].  The rules in these two
     sections are symmetric.  PARM is the type of a function parameter
@@ -18197,8 +18184,6 @@ type_unification_real (tree tparms,
        arg = args[ia];
        ++ia;

-      parm = decay_dependent_array_parm_type (parm);
-
        if (unify_one_argument (tparms, targs, parm, arg, subr, strict,
  			      explain_p))
  	return 1;
@@ -20232,9 +20217,6 @@ more_specialized_fn (tree pat1, tree pat2, int len)
            len = 0;
          }

-      arg1 = decay_dependent_array_parm_type (arg1);
-      arg2 = decay_dependent_array_parm_type (arg2);
-
        if (TREE_CODE (arg1) == REFERENCE_TYPE)
  	{
  	  ref1 = TYPE_REF_IS_RVALUE (arg1) + 1;
@@ -20520,10 +20502,7 @@ get_bindings (tree fn, tree decl, tree explicit_args, bool check_rettype)
    for (arg = decl_arg_types, ix = 0;
         arg != NULL_TREE && arg != void_list_node;
         arg = TREE_CHAIN (arg), ++ix)
-    {
-      args[ix] = TREE_VALUE (arg);
-      args[ix] = decay_dependent_array_parm_type (args[ix]);
-    }
+    args[ix] = TREE_VALUE (arg);

    if (fn_type_unification (fn, explicit_args, targs,
  			   args, ix,
diff --git a/gcc/testsuite/g++.dg/template/mangle2.C b/gcc/testsuite/g++.dg/template/mangle2.C
new file mode 100644
index 0000000..f9ea040
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/mangle2.C
@@ -0,0 +1,22 @@
+template <typename T, int I>
+int foo (T [I]);
+
+template <typename T>
+int bar (T [7]);
+
+template <int I>
+int baz (int [I]);
+
+extern int x, y, z;
+
+int
+main ()
+{
+  x = foo<int, 5> (0);
+  y = bar<char> (0);
+  z = baz<9> (0);
+}
+
+// { dg-final { scan-assembler "_Z3fooIiLi5EEiPT_" } }
+// { dg-final { scan-assembler "_Z3barIcEiPT_" } }
+// { dg-final { scan-assembler "_Z3bazILi9EEiPi" } }
diff --git a/gcc/testsuite/g++.dg/template/unify17.C b/gcc/testsuite/g++.dg/template/unify17.C
index 2da8553..3ae7699 100644
--- a/gcc/testsuite/g++.dg/template/unify17.C
+++ b/gcc/testsuite/g++.dg/template/unify17.C
@@ -1,11 +1,11 @@
  void foo (int *);

  template <typename T>
-void bar (void (T[5])); // { dg-error "array of 'void'" }
+void bar (void (T[5])); // { dg-error "array of 'void'" "" { xfail *-*-* } }

  void
  baz (void)
  {
    bar (foo); // { dg-bogus "" }
-  bar<void> (0); // { dg-error "no matching function" }
+  bar<void> (0); // { dg-error "no matching function" "" { xfail *-*-* } }
  }
-- 
2.7.0.83.gdfccd77.dirty

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-20  3:30             ` Patrick Palka
@ 2016-01-21 18:43               ` Jason Merrill
  2016-01-21 21:45                 ` Patrick Palka
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2016-01-21 18:43 UTC (permalink / raw)
  To: Patrick Palka; +Cc: GCC Patches

On 01/19/2016 10:30 PM, Patrick Palka wrote:
>      * g++.dg/template/unify17.C: XFAIL.

Hmm, I'm not comfortable with deliberately regressing this testcase.

>   template <typename T>
> -void bar (void (T[5])); // { dg-error "array of 'void'" }
> +void bar (void (T[5])); // { dg-error "array of 'void'" "" { xfail
> *-*-* } }

Can we work it so that T[5] also is un-decayed in the DECL_ARGUMENTS of 
bar, but decayed in the TYPE_ARG_TYPES?

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-21 18:43               ` Jason Merrill
@ 2016-01-21 21:45                 ` Patrick Palka
  2016-01-22 16:17                   ` Patrick Palka
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2016-01-21 21:45 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Patrick Palka, GCC Patches

On Thu, 21 Jan 2016, Jason Merrill wrote:

> On 01/19/2016 10:30 PM, Patrick Palka wrote:
>>      * g++.dg/template/unify17.C: XFAIL.
>
> Hmm, I'm not comfortable with deliberately regressing this testcase.
>
>>   template <typename T>
>> -void bar (void (T[5])); // { dg-error "array of 'void'" }
>> +void bar (void (T[5])); // { dg-error "array of 'void'" "" { xfail
>> *-*-* } }
>
> Can we work it so that T[5] also is un-decayed in the DECL_ARGUMENTS of bar, 
> but decayed in the TYPE_ARG_TYPES?

I think so, I'll try it.

>
> Jason
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-21 21:45                 ` Patrick Palka
@ 2016-01-22 16:17                   ` Patrick Palka
  2016-01-22 21:05                     ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2016-01-22 16:17 UTC (permalink / raw)
  To: Patrick Palka; +Cc: Jason Merrill, GCC Patches

On Thu, 21 Jan 2016, Patrick Palka wrote:

> On Thu, 21 Jan 2016, Jason Merrill wrote:
>
>> On 01/19/2016 10:30 PM, Patrick Palka wrote:
>>>      * g++.dg/template/unify17.C: XFAIL.
>> 
>> Hmm, I'm not comfortable with deliberately regressing this testcase.
>>
>>>   template <typename T>
>>> -void bar (void (T[5])); // { dg-error "array of 'void'" }
>>> +void bar (void (T[5])); // { dg-error "array of 'void'" "" { xfail
>>> *-*-* } }
>> 
>> Can we work it so that T[5] also is un-decayed in the DECL_ARGUMENTS of 
>> bar, but decayed in the TYPE_ARG_TYPES?
>
> I think so, I'll try it.

Well, I tried it and the result is really ugly and it only "somewhat"
works.  (Maybe I'm just missing something obvious though.)  The ugliness
comes from the fact that decaying an array parameter type of a function
type embedded deep within some tree structure requires rebuilding all
the tree structures in between to avoid issues due to tree sharing.
This approach only "somewhat" works because it currently looks through
function, pointer, reference and array types.  And I just noticed that
this approach does not work at all for USING_DECLs because no PARM_DECL
is ever retained anyway in that case.

I think a better, complete fix for this issue would be to, one way or
another, be able to get at the PARM_DECLs that correspond to a given
FUNCTION_TYPE.  Say, if, the TREE_CHAIN of a FUNCTION_TYPE optionally
pointed to its PARM_DECLs, or something.  What do you think?

In the meantime, at this stage, I am personally most comfortable with
the previous patch (the one that XFAILs unify17.C).


diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index f4604b6..c70eb12 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -9082,6 +9082,92 @@ check_var_type (tree identifier, tree type)
    return type;
  }

+/* Given a type T, return a copy of T within which each array parameter type of
+   each function type embedded in T has been decayed to the corresponding
+   pointer type.  If no decaying was done, return T.
+
+   For example, if T corresponds to the type
+
+       void (** (char, void (&) (int[5]), char[7])) (int (*) (long[10]));
+                                 ~~~~~~   ~~~~~~~             ~~~~~~~
+
+   then the type returned by this function corresponds to
+
+       void (** (char, void (&) (int *), char *)) (int (*) (long *))
+                                 ~~~~~   ~~~~~~             ~~~~~~
+*/
+
+static tree
+decay_array_parms_r (tree t)
+{
+  if (t == NULL_TREE)
+    return t;
+
+  if (FUNC_OR_METHOD_TYPE_P (t))
+    {
+      auto_vec<tree> new_types;
+      new_types.reserve (8);
+      bool any_changed_p = false;
+
+      for (tree arg = TYPE_ARG_TYPES (t);
+	   arg != NULL_TREE && arg != void_list_node;
+	   arg = TREE_CHAIN (arg))
+	{
+	  tree old_type = TREE_VALUE (arg);
+	  tree new_type;
+
+	  if (TREE_CODE (old_type) == ARRAY_TYPE)
+	    new_type = decay_array_parms_r (type_decays_to (old_type));
+	  else
+	    new_type = decay_array_parms_r (old_type);
+
+	  if (old_type != new_type)
+	    any_changed_p = true;
+
+	  new_types.safe_push (new_type);
+	}
+
+      tree old_ret_type = TREE_TYPE (t);
+      tree new_ret_type = decay_array_parms_r (old_ret_type);
+      if (old_ret_type != new_ret_type)
+	any_changed_p = true;
+
+      if (!any_changed_p)
+	return t;
+
+      tree new_type_arg_types = NULL_TREE;
+      tree arg;
+      int i = 0;
+      for (arg = TYPE_ARG_TYPES (t);
+	   arg != NULL_TREE && arg != void_list_node;
+	   arg = TREE_CHAIN (arg), i++)
+	new_type_arg_types = tree_cons (TREE_PURPOSE (arg),
+					new_types[i],
+					new_type_arg_types);
+      new_type_arg_types = nreverse (new_type_arg_types);
+      if (arg == void_list_node)
+	new_type_arg_types = chainon (new_type_arg_types, void_list_node);
+
+      return build_function_type (new_ret_type, new_type_arg_types);
+    }
+  else if (TREE_CODE (t) == POINTER_TYPE)
+    {
+      tree old_type_type = TREE_TYPE (t);
+      tree new_type_type = decay_array_parms_r (old_type_type);
+      if (old_type_type != new_type_type)
+	return build_pointer_type (new_type_type);
+    }
+  else if (TREE_CODE (t) == REFERENCE_TYPE)
+    {
+      tree old_type_type = TREE_TYPE (t);
+      tree new_type_type = decay_array_parms_r (old_type_type);
+      if (old_type_type != new_type_type)
+	return cp_build_reference_type (new_type_type, TYPE_REF_IS_RVALUE (t));
+    }
+
+  return t;
+}
+
  /* Given declspecs and a declarator (abstract or otherwise), determine
     the name and type of the object declared and construct a DECL node
     for it.
@@ -10213,6 +10299,15 @@ grokdeclarator (const cp_declarator *declarator,

  	    type = build_function_type (type, arg_types);

+	    /* Decay all the (dependent) array parameter types embedded in this
+	       function type into their corresponding pointer types.  This is
+	       only done once, recursively, at the top-level declarator,
+	       not when processing a nested function declarator, designated by
+	       DECL_CONTEXT == PARM, so that the PARM_DECLS built will
+	       contain the un-decayed types.  */
+	    if (decl_context != PARM && processing_template_decl)
+	      type = decay_array_parms_r (type);
+
  	    tree attrs = declarator->std_attributes;
  	    if (tx_qual)
  	      {
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 3733ee0..596b129 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11795,6 +11795,10 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain)
  				     complain, t);
  	DECL_RESULT (r) = NULL_TREE;

+	for (tree parm = DECL_ARGUMENTS (r); parm; parm = DECL_CHAIN (parm))
+	  if (TREE_TYPE (parm) == error_mark_node)
+	    RETURN (error_mark_node);
+
  	TREE_STATIC (r) = 0;
  	TREE_PUBLIC (r) = TREE_PUBLIC (t);
  	DECL_EXTERNAL (r) = 1;
@@ -17797,23 +17801,6 @@ fn_type_unification (tree fn,
    return r;
  }

-/* TYPE is the type of a function parameter.  If TYPE is a (dependent)
-   ARRAY_TYPE, return the corresponding POINTER_TYPE to which it decays.
-   Otherwise return TYPE.  (We shouldn't see non-dependent ARRAY_TYPE
-   parameters because they get decayed as soon as they are declared.)  */
-
-static tree
-decay_dependent_array_parm_type (tree type)
-{
-  if (TREE_CODE (type) == ARRAY_TYPE)
-    {
-      gcc_assert (uses_template_parms (type));
-      return type_decays_to (type);
-    }
-
-  return type;
-}
-
  /* Adjust types before performing type deduction, as described in
     [temp.deduct.call] and [temp.deduct.conv].  The rules in these two
     sections are symmetric.  PARM is the type of a function parameter
@@ -18252,8 +18239,6 @@ type_unification_real (tree tparms,
        arg = args[ia];
        ++ia;

-      parm = decay_dependent_array_parm_type (parm);
-
        if (unify_one_argument (tparms, targs, parm, arg, subr, strict,
  			      explain_p))
  	return 1;
@@ -20287,9 +20272,6 @@ more_specialized_fn (tree pat1, tree pat2, int len)
            len = 0;
          }

-      arg1 = decay_dependent_array_parm_type (arg1);
-      arg2 = decay_dependent_array_parm_type (arg2);
-
        if (TREE_CODE (arg1) == REFERENCE_TYPE)
  	{
  	  ref1 = TYPE_REF_IS_RVALUE (arg1) + 1;
@@ -20575,10 +20557,7 @@ get_bindings (tree fn, tree decl, tree explicit_args, bool check_rettype)
    for (arg = decl_arg_types, ix = 0;
         arg != NULL_TREE && arg != void_list_node;
         arg = TREE_CHAIN (arg), ++ix)
-    {
-      args[ix] = TREE_VALUE (arg);
-      args[ix] = decay_dependent_array_parm_type (args[ix]);
-    }
+    args[ix] = TREE_VALUE (arg);

    if (fn_type_unification (fn, explicit_args, targs,
  			   args, ix,
diff --git a/gcc/testsuite/g++.dg/template/mangle2.C b/gcc/testsuite/g++.dg/template/mangle2.C
new file mode 100644
index 0000000..f9ea040
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/mangle2.C
@@ -0,0 +1,22 @@
+template <typename T, int I>
+int foo (T [I]);
+
+template <typename T>
+int bar (T [7]);
+
+template <int I>
+int baz (int [I]);
+
+extern int x, y, z;
+
+int
+main ()
+{
+  x = foo<int, 5> (0);
+  y = bar<char> (0);
+  z = baz<9> (0);
+}
+
+// { dg-final { scan-assembler "_Z3fooIiLi5EEiPT_" } }
+// { dg-final { scan-assembler "_Z3barIcEiPT_" } }
+// { dg-final { scan-assembler "_Z3bazILi9EEiPi" } }
-- 
2.7.0.134.gf5046bd.dirty

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-22 16:17                   ` Patrick Palka
@ 2016-01-22 21:05                     ` Jason Merrill
  2016-01-22 22:30                       ` Patrick Palka
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2016-01-22 21:05 UTC (permalink / raw)
  To: Patrick Palka; +Cc: GCC Patches

On 01/22/2016 11:17 AM, Patrick Palka wrote:
> On Thu, 21 Jan 2016, Patrick Palka wrote:
>> On Thu, 21 Jan 2016, Jason Merrill wrote:
>>
>>> On 01/19/2016 10:30 PM, Patrick Palka wrote:
>>>>      * g++.dg/template/unify17.C: XFAIL.
>>>
>>> Hmm, I'm not comfortable with deliberately regressing this testcase.
>>>
>>>>   template <typename T>
>>>> -void bar (void (T[5])); // { dg-error "array of 'void'" }
>>>> +void bar (void (T[5])); // { dg-error "array of 'void'" "" { xfail
>>>> *-*-* } }
>>>
>>> Can we work it so that T[5] also is un-decayed in the DECL_ARGUMENTS
>>> of bar, but decayed in the TYPE_ARG_TYPES?
>>
>> I think so, I'll try it.
>
> Well, I tried it and the result is really ugly and it only "somewhat"
> works.  (Maybe I'm just missing something obvious though.)  The ugliness
> comes from the fact that decaying an array parameter type of a function
> type embedded deep within some tree structure requires rebuilding all
> the tree structures in between to avoid issues due to tree sharing.

Yes, that does complicate things.

> This approach only "somewhat" works because it currently looks through
> function, pointer, reference and array types.

Right, you would need to handle template arguments as well.

> And I just noticed that
> this approach does not work at all for USING_DECLs because no PARM_DECL
> is ever retained anyway in that case.

I don't understand what you mean about USING_DECLs.

> I think a better, complete fix for this issue would be to, one way or
> another, be able to get at the PARM_DECLs that correspond to a given
> FUNCTION_TYPE.  Say, if, the TREE_CHAIN of a FUNCTION_TYPE optionally
> pointed to its PARM_DECLs, or something.  What do you think?

Hmm.  So void(int[5]) and void(int*) would be distinct types, but they 
would share TYPE_CANONICAL, as though one is a typedef of the other? 
Interesting, but I'm not sure how that would interact with template 
argument canonicalization.  Well, that can probably be made to work by 
treating dependent template arguments as distinct more frequently.

Another thought: What if we keep a list of arrays we need to substitute 
into for a particular function template?

> In the meantime, at this stage, I am personally most comfortable with
> the previous patch (the one that XFAILs unify17.C).

I don't think that's a good tradeoff, sorry.  For the moment, let's 
revert your earlier patch.

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-22 21:05                     ` Jason Merrill
@ 2016-01-22 22:30                       ` Patrick Palka
  2016-01-25 15:11                         ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2016-01-22 22:30 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Patrick Palka, GCC Patches

On Fri, 22 Jan 2016, Jason Merrill wrote:

> On 01/22/2016 11:17 AM, Patrick Palka wrote:
>> On Thu, 21 Jan 2016, Patrick Palka wrote:
>>> On Thu, 21 Jan 2016, Jason Merrill wrote:
>>> 
>>>> On 01/19/2016 10:30 PM, Patrick Palka wrote:
>>>>>      * g++.dg/template/unify17.C: XFAIL.
>>>> 
>>>> Hmm, I'm not comfortable with deliberately regressing this testcase.
>>>>
>>>>>   template <typename T>
>>>>> -void bar (void (T[5])); // { dg-error "array of 'void'" }
>>>>> +void bar (void (T[5])); // { dg-error "array of 'void'" "" { xfail
>>>>> *-*-* } }
>>>> 
>>>> Can we work it so that T[5] also is un-decayed in the DECL_ARGUMENTS
>>>> of bar, but decayed in the TYPE_ARG_TYPES?
>>> 
>>> I think so, I'll try it.
>> 
>> Well, I tried it and the result is really ugly and it only "somewhat"
>> works.  (Maybe I'm just missing something obvious though.)  The ugliness
>> comes from the fact that decaying an array parameter type of a function
>> type embedded deep within some tree structure requires rebuilding all
>> the tree structures in between to avoid issues due to tree sharing.
>
> Yes, that does complicate things.
>
>> This approach only "somewhat" works because it currently looks through
>> function, pointer, reference and array types.
>
> Right, you would need to handle template arguments as well.
>
>> And I just noticed that
>> this approach does not work at all for USING_DECLs because no PARM_DECL
>> is ever retained anyway in that case.
>
> I don't understand what you mean about USING_DECLs.

I just meant that we fail and would continue to fail to diagnose an
"array of void" error in the following test case:

    template <typename T>
    using X = void (T[5]);

    void foo (X<void>);

>
>> I think a better, complete fix for this issue would be to, one way or
>> another, be able to get at the PARM_DECLs that correspond to a given
>> FUNCTION_TYPE.  Say, if, the TREE_CHAIN of a FUNCTION_TYPE optionally
>> pointed to its PARM_DECLs, or something.  What do you think?
>
> Hmm.  So void(int[5]) and void(int*) would be distinct types, but they would 
> share TYPE_CANONICAL, as though one is a typedef of the other? Interesting, 
> but I'm not sure how that would interact with template argument 
> canonicalization.  Well, that can probably be made to work by treating 
> dependent template arguments as distinct more frequently.
>
> Another thought: What if we keep a list of arrays we need to substitute into 
> for a particular function template?

That approach definitely seems easier to reason about.  And it could
properly handle "using" templates as well as variable templates -- any
TEMPLATE_DECL, I think.

>
>> In the meantime, at this stage, I am personally most comfortable with
>> the previous patch (the one that XFAILs unify17.C).
>
> I don't think that's a good tradeoff, sorry.  For the moment, let's revert 
> your earlier patch.

Okay, will do.

>
> Jason
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early)
  2016-01-22 22:30                       ` Patrick Palka
@ 2016-01-25 15:11                         ` Jason Merrill
  0 siblings, 0 replies; 14+ messages in thread
From: Jason Merrill @ 2016-01-25 15:11 UTC (permalink / raw)
  To: Patrick Palka; +Cc: GCC Patches

On 01/22/2016 05:30 PM, Patrick Palka wrote:
> On Fri, 22 Jan 2016, Jason Merrill wrote:
>> On 01/22/2016 11:17 AM, Patrick Palka wrote:
>>> On Thu, 21 Jan 2016, Patrick Palka wrote:
>>>> On Thu, 21 Jan 2016, Jason Merrill wrote:
>>>>
>>>>> On 01/19/2016 10:30 PM, Patrick Palka wrote:
>>>>>>      * g++.dg/template/unify17.C: XFAIL.
>>>>>
>>>>> Hmm, I'm not comfortable with deliberately regressing this testcase.
>>>>>
>>>>>>   template <typename T>
>>>>>> -void bar (void (T[5])); // { dg-error "array of 'void'" }
>>>>>> +void bar (void (T[5])); // { dg-error "array of 'void'" "" { xfail
>>>>>> *-*-* } }
>>>>>
>>>>> Can we work it so that T[5] also is un-decayed in the DECL_ARGUMENTS
>>>>> of bar, but decayed in the TYPE_ARG_TYPES?
>>>>
>>>> I think so, I'll try it.
>>>
>>> Well, I tried it and the result is really ugly and it only "somewhat"
>>> works.  (Maybe I'm just missing something obvious though.)  The ugliness
>>> comes from the fact that decaying an array parameter type of a function
>>> type embedded deep within some tree structure requires rebuilding all
>>> the tree structures in between to avoid issues due to tree sharing.
>>
>> Yes, that does complicate things.
>>
>>> This approach only "somewhat" works because it currently looks through
>>> function, pointer, reference and array types.
>>
>> Right, you would need to handle template arguments as well.
>>
>>> And I just noticed that
>>> this approach does not work at all for USING_DECLs because no PARM_DECL
>>> is ever retained anyway in that case.
>>
>> I don't understand what you mean about USING_DECLs.
>
> I just meant that we fail and would continue to fail to diagnose an
> "array of void" error in the following test case:
>
>     template <typename T>
>     using X = void (T[5]);
>
>     void foo (X<void>);

True.  I think here we want to get the error when instantiating X<void>.

>>> I think a better, complete fix for this issue would be to, one way or
>>> another, be able to get at the PARM_DECLs that correspond to a given
>>> FUNCTION_TYPE.  Say, if, the TREE_CHAIN of a FUNCTION_TYPE optionally
>>> pointed to its PARM_DECLs, or something.  What do you think?
>>
>> Hmm.  So void(int[5]) and void(int*) would be distinct types, but they
>> would share TYPE_CANONICAL, as though one is a typedef of the other?
>> Interesting, but I'm not sure how that would interact with template
>> argument canonicalization.  Well, that can probably be made to work by
>> treating dependent template arguments as distinct more frequently.
>>
>> Another thought: What if we keep a list of arrays we need to
>> substitute into for a particular function template?
>
> That approach definitely seems easier to reason about.  And it could
> properly handle "using" templates as well as variable templates -- any
> TEMPLATE_DECL, I think.

Agreed.

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-01-25 15:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-24 17:58 [PATCH] Fix the remaining PR c++/24666 blockers (arrays decay to pointers too early) Patrick Palka
2015-12-25  2:41 ` Jason Merrill
2015-12-25 17:37   ` Patrick Palka
2016-01-18 15:34     ` Jason Merrill
2016-01-18 19:13       ` Patrick Palka
2016-01-18 21:04         ` Jason Merrill
2016-01-19 21:10           ` Patrick Palka
2016-01-20  3:30             ` Patrick Palka
2016-01-21 18:43               ` Jason Merrill
2016-01-21 21:45                 ` Patrick Palka
2016-01-22 16:17                   ` Patrick Palka
2016-01-22 21:05                     ` Jason Merrill
2016-01-22 22:30                       ` Patrick Palka
2016-01-25 15:11                         ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).