Re: [PATCH] c++: fix cases of core1001/1322 by not dropping cv-qualifier of function parameter of type of typename or decltype[PR101402,PR102033,PR102034,PR102039,PR102

[Jason Merrill <jason@redhat.com>]

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

On 10/3/21 07:14, Nick Huang wrote:
> I made a little improvement of my fix by including template
> type parameter to not dropping cv-const because they are similar to dependent
> type which you cannot determine whether they are top-level cv-qualifier or not
> until instantiation.
> 
> +         if (processing_template_decl
> +               && (TREE_CODE (type) == TYPENAME_TYPE
> +                  || TREE_CODE (type) == DECLTYPE_TYPE
> ++                  || TREE_CODE (type) == TEMPLATE_TYPE_PARM  // this is new

I think WILDCARD_TYPE_P is what you want, except...

> 1. It fix your test case of
> template <class T>
> struct A{
>     void f(T);
> };
> template <class T>
> void A<T>::f(const T){ }
> template<>
> void A<int[]>::f(const int*){}
> 
> current GCC mistakenly accepts without considering the gap of missing "const"
> between declaration and out of line definition. clang correctly rejects it.
> (https://www.godbolt.org/z/qb9Tf99eK) and explains the cause nicely.
> My fix also rejects it.

Your patch rejects that testcase even without the specialization on 
int[], which no other compiler I'm aware of does; this is why I think 
this approach is wrong.  I think your approach would make sense for the 
standard to specify, but it doesn't (currently).

You seem to have missed my September 28 mail that argued for fixing the 
bug in determine_specialization that was preventing the 92010 fix from 
handling these cases.  I tried removing the redundant code in 
determine_specialization, but then it turned out that I needed to fix 
the corresponding code in fn_type_unification, as in the patch below. 
This approach does not reject the testcase above, which I think is OK; 
whether the specialization is well-formed depends on which declaration 
we are trying to specialize, and GCC chooses the definition.

Thoughts?

[-- Attachment #2: 0001-c-array-cv-quals-and-template-specialization.patch --]
[-- Type: text/x-patch, Size: 4738 bytes --]

From 8671432aac3444603e00c07d0d5d6cebda7d5da8 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Tue, 28 Sep 2021 10:02:04 -0400
Subject: [PATCH] c++: array cv-quals and template specialization
To: gcc-patches@gcc.gnu.org

gcc/cp/ChangeLog:

	* pt.c (determine_specialization): Remove redundant code.
	(fn_type_unification): Check for mismatched length.
	(type_unification_real): Ignore terminal void.
	(get_bindings): Don't stop at void_list_node.
	* class.c (resolve_address_of_overloaded_function): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/parse/pr101402.C: No errors.
	* g++.dg/template/fnspec2.C: New test.
---
 gcc/cp/class.c                          |  2 +-
 gcc/cp/pt.c                             | 30 ++++++++++++-------------
 gcc/testsuite/g++.dg/parse/pr101402.C   |  4 ++--
 gcc/testsuite/g++.dg/template/fnspec2.C |  9 ++++++++
 4 files changed, 26 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/fnspec2.C

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 59611627d18..f16e50b9de9 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -8382,7 +8382,7 @@ resolve_address_of_overloaded_function (tree target_type,
       nargs = list_length (target_arg_types);
       args = XALLOCAVEC (tree, nargs);
       for (arg = target_arg_types, ia = 0;
-	   arg != NULL_TREE && arg != void_list_node;
+	   arg != NULL_TREE;
 	   arg = TREE_CHAIN (arg), ++ia)
 	args[ia] = TREE_VALUE (arg);
       nargs = ia;
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 19e03369ffa..2eb641e0304 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -2230,7 +2230,6 @@ determine_specialization (tree template_id,
 	{
 	  tree decl_arg_types;
 	  tree fn_arg_types;
-	  tree insttype;
 
 	  /* In case of explicit specialization, we need to check if
 	     the number of template headers appearing in the specialization
@@ -2356,20 +2355,6 @@ determine_specialization (tree template_id,
 	       template argument.  */
 	    continue;
 
-          /* Remove, from the set of candidates, all those functions
-             whose constraints are not satisfied. */
-          if (flag_concepts && !constraints_satisfied_p (fn, targs))
-            continue;
-
-          // Then, try to form the new function type.
-	  insttype = tsubst (TREE_TYPE (fn), targs, tf_fndecl_type, NULL_TREE);
-	  if (insttype == error_mark_node)
-	    continue;
-	  fn_arg_types
-	    = skip_artificial_parms_for (fn, TYPE_ARG_TYPES (insttype));
-	  if (!compparms (fn_arg_types, decl_arg_types))
-	    continue;
-
 	  /* Save this template, and the arguments deduced.  */
 	  templates = tree_cons (targs, fn, templates);
 	}
@@ -21856,6 +21841,15 @@ fn_type_unification (tree fn,
 				 TREE_VALUE (sarg));
 	    goto fail;
 	  }
+      if ((i < nargs || sarg)
+	  /* add_candidates uses DEDUCE_EXACT for x.operator foo(), but args
+	     doesn't contain the trailing void, and conv fns are always ().  */
+	  && !DECL_CONV_FN_P (decl))
+	{
+	  unsigned nsargs = i + list_length (sarg);
+	  unify_arity (explain_p, nargs, nsargs);
+	  goto fail;
+	}
     }
 
   /* After doing deduction with the inherited constructor, actually return an
@@ -22379,6 +22373,10 @@ type_unification_real (tree tparms,
   args = xargs;
   nargs = xnargs;
 
+  /* Only fn_type_unification cares about terminal void.  */
+  if (nargs && args[nargs-1] == void_type_node)
+    --nargs;
+
   ia = 0;
   while (parms && parms != void_list_node
 	 && ia < nargs)
@@ -24880,7 +24878,7 @@ get_bindings (tree fn, tree decl, tree explicit_args, bool check_rettype)
   nargs = list_length (decl_arg_types);
   args = XALLOCAVEC (tree, nargs);
   for (arg = decl_arg_types, ix = 0;
-       arg != NULL_TREE && arg != void_list_node;
+       arg != NULL_TREE;
        arg = TREE_CHAIN (arg), ++ix)
     args[ix] = TREE_VALUE (arg);
 
diff --git a/gcc/testsuite/g++.dg/parse/pr101402.C b/gcc/testsuite/g++.dg/parse/pr101402.C
index 42a9ce3ab78..7cf02da5401 100644
--- a/gcc/testsuite/g++.dg/parse/pr101402.C
+++ b/gcc/testsuite/g++.dg/parse/pr101402.C
@@ -22,8 +22,8 @@ void f(T);
 };
 
 template<class T>
-void A<T>::f(const T){} /* { dg-error "no declaration matches" } */
+void A<T>::f(const T){}
 
 template<>
-void A<int[3]>::f(const int*){} /* { dg-error "does not match any template declaration" } */
+void A<int[3]>::f(const int*){}
 }
diff --git a/gcc/testsuite/g++.dg/template/fnspec2.C b/gcc/testsuite/g++.dg/template/fnspec2.C
new file mode 100644
index 00000000000..7a4b1012d89
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/fnspec2.C
@@ -0,0 +1,9 @@
+template <class T>
+void f(T);
+
+template<> void f(int, ...);	// { dg-error "match" }
+
+template <class T>
+void g(T, ...);
+
+template<> void g(int);		// { dg-error "match" }
-- 
2.27.0