Re: [PATCH] c++: check arity before deduction w/ explicit targs [PR12672]

[Jason Merrill <jason@redhat.com>]

On 8/30/21 9:26 PM, Patrick Palka wrote:
> During overload resolution, when the arity of a function template
> clearly disagrees with the arity of the call, no specialization of the
> function template could yield a viable candidate.  The deduction routine
> type_unification_real already notices this situation, but not before
> it substitutes explicit template arguments into the template, a step
> which could induce a hard error.  Although it's necessary to perform
> this substitution first in order to check arity perfectly (since the
> substitution can e.g. expand a non-trailing parameter pack), in most
> cases we can determine ahead of time whether there's an arity
> disagreement without needing to perform deduction.
> 
> To that end, this patch implements an (approximate) arity check in
> add_template_candidate_real that guards actual deduction.  It's enabled
> only when there are explicit template arguments since that's when
> deduction can force otherwise avoidable template instantiations.  (I
> experimented with enabling it unconditionally as an optimization and
> saw some compile-time improvements of about 5% but also some regressions
> by about the same magnitude, so kept it conditional.)
> 
> In passing, this adds a least_p parameter to arity_rejection for sake
> of consistent diagnostics with unify_arity.
> 
> A couple of testcases needed to be adjusted so that deduction continues
> to occur as intended after this change.  Except in unify6.C, where we
> were expecting foo<void ()> to be ill-formed due to substitution
> yielding a function type with an added 'const', but I think this is
> permitted by [dcl.fct]/7, so I changed the test accordingly.

Agreed.

> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?

OK, thanks.

> 	PR c++/12672
> 
> gcc/cp/ChangeLog:
> 
> 	* call.c (rejection_reason::call_varargs_p): Rename this
> 	previously unused member to ...
> 	(rejection_reason::least_p): ... this.
> 	(arity_rejection): Add least_p parameter.
> 	(add_template_candidate_real): When there are explicit
> 	template arguments, check that the arity of the call agrees with
> 	the arity of the function before attempting deduction.
> 	(print_arity_information): Add least_p parameter.
> 	(print_z_candidate): Adjust call to print_arity_information.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/decltype29.C: Adjust.
> 	* g++.dg/template/error56.C: Adjust.
> 	* g++.old-deja/g++.pt/unify6.C: Adjust.
> 	* g++.dg/template/explicit-args6.C: New test.
> ---
>   gcc/cp/call.c                                 | 67 ++++++++++++++++---
>   gcc/testsuite/g++.dg/cpp0x/decltype29.C       |  4 +-
>   gcc/testsuite/g++.dg/template/error56.C       |  4 +-
>   .../g++.dg/template/explicit-args6.C          | 33 +++++++++
>   gcc/testsuite/g++.old-deja/g++.pt/unify6.C    |  4 +-
>   5 files changed, 96 insertions(+), 16 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/explicit-args6.C
> 
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index e4df72ec1a3..80e6121ce44 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -455,8 +455,8 @@ struct rejection_reason {
>         int expected;
>         /* The actual number of arguments in the call.  */
>         int actual;
> -      /* Whether the call was a varargs call.  */
> -      bool call_varargs_p;
> +      /* Whether EXPECTED should be treated as a lower bound.  */
> +      bool least_p;
>       } arity;
>       /* Information about an argument conversion mismatch.  */
>       struct conversion_info conversion;
> @@ -628,12 +628,13 @@ alloc_rejection (enum rejection_reason_code code)
>   }
>   
>   static struct rejection_reason *
> -arity_rejection (tree first_arg, int expected, int actual)
> +arity_rejection (tree first_arg, int expected, int actual, bool least_p = false)
>   {
>     struct rejection_reason *r = alloc_rejection (rr_arity);
>     int adjust = first_arg != NULL_TREE;
>     r->u.arity.expected = expected - adjust;
>     r->u.arity.actual = actual - adjust;
> +  r->u.arity.least_p = least_p;
>     return r;
>   }
>   
> @@ -3452,6 +3453,44 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl,
>       }
>     gcc_assert (ia == nargs_without_in_chrg);
>   
> +  if (!obj && explicit_targs)
> +    {
> +      /* Check that there's no obvious arity mismatch before proceeding with
> +	 deduction.  This avoids substituting explicit template arguments
> +	 into the template (which could result in an error outside the
> +	 immediate context) when the resulting candidate would be unviable
> +	 anyway.  */
> +      int min_arity = 0, max_arity = 0;
> +      tree parms = TYPE_ARG_TYPES (TREE_TYPE (tmpl));
> +      parms = skip_artificial_parms_for (tmpl, parms);
> +      for (; parms != void_list_node; parms = TREE_CHAIN (parms))
> +	{
> +	  if (!parms || PACK_EXPANSION_P (TREE_VALUE (parms)))
> +	    {
> +	      max_arity = -1;
> +	      break;
> +	    }
> +	  if (TREE_PURPOSE (parms))
> +	    /* A parameter with a default argument.  */
> +	    ++max_arity;
> +	  else
> +	    ++min_arity, ++max_arity;
> +	}
> +      if (ia < (unsigned)min_arity)
> +	{
> +	  /* Too few arguments.  */
> +	  reason = arity_rejection (NULL_TREE, min_arity, ia,
> +				    /*least_p=*/(max_arity == -1));
> +	  goto fail;
> +	}
> +      else if (max_arity != -1 && ia > (unsigned)max_arity)
> +	{
> +	  /* Too many arguments.  */
> +	  reason = arity_rejection (NULL_TREE, max_arity, ia);
> +	  goto fail;
> +	}
> +    }
> +
>     errs = errorcount+sorrycount;
>     if (!obj)
>       convs = alloc_conversions (nargs);
> @@ -3725,12 +3764,19 @@ print_conversion_rejection (location_t loc, struct conversion_info *info,
>      HAVE.  */
>   
>   static void
> -print_arity_information (location_t loc, unsigned int have, unsigned int want)
> -{
> -  inform_n (loc, want,
> -	    "  candidate expects %d argument, %d provided",
> -	    "  candidate expects %d arguments, %d provided",
> -	    want, have);
> +print_arity_information (location_t loc, unsigned int have, unsigned int want,
> +			 bool least_p)
> +{
> +  if (least_p)
> +    inform_n (loc, want,
> +	      "  candidate expects at least %d argument, %d provided",
> +	      "  candidate expects at least %d arguments, %d provided",
> +	      want, have);
> +  else
> +    inform_n (loc, want,
> +	      "  candidate expects %d argument, %d provided",
> +	      "  candidate expects %d arguments, %d provided",
> +	      want, have);
>   }
>   
>   /* Print information about one overload candidate CANDIDATE.  MSGSTR
> @@ -3794,7 +3840,8 @@ print_z_candidate (location_t loc, const char *msgstr,
>   	{
>   	case rr_arity:
>   	  print_arity_information (cloc, r->u.arity.actual,
> -				   r->u.arity.expected);
> +				   r->u.arity.expected,
> +				   r->u.arity.least_p);
>   	  break;
>   	case rr_arg_conversion:
>   	  print_conversion_rejection (cloc, &r->u.conversion, fn);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/decltype29.C b/gcc/testsuite/g++.dg/cpp0x/decltype29.C
> index 51da8ddd0de..ea97b033569 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/decltype29.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/decltype29.C
> @@ -10,10 +10,10 @@ ft() {}
>   
>   template<class F, int N>
>   decltype (ft<F> (F()))		// { dg-error "depth" }
> -ft() {}
> +ft(F) {}
>   
>   int main() {
> -    ft<struct a*, 0>();		// { dg-message "from here" }
> +    ft<struct a*, 0>(0);		// { dg-message "from here" }
>   }
>   
>   // { dg-prune-output "compilation terminated" }
> diff --git a/gcc/testsuite/g++.dg/template/error56.C b/gcc/testsuite/g++.dg/template/error56.C
> index e85471a50b0..71206a1ae5c 100644
> --- a/gcc/testsuite/g++.dg/template/error56.C
> +++ b/gcc/testsuite/g++.dg/template/error56.C
> @@ -3,12 +3,12 @@
>   struct A
>   {
>     template <class T> void f(T);
> -  void f();
> +  void f(int);
>   };
>   
>   int main()
>   {
> -  A().f<1>();			// { dg-error "f<1>" }
> +  A().f<1>(0);			// { dg-error "f<1>" }
>     // { dg-error "type/value mismatch at argument 1" "" { target *-*-* } .-1 }
>     // { dg-message "expected a type, got .1." "" { target *-*-* } .-2 }
>   }
> diff --git a/gcc/testsuite/g++.dg/template/explicit-args6.C b/gcc/testsuite/g++.dg/template/explicit-args6.C
> new file mode 100644
> index 00000000000..fb5e89e4cf6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/explicit-args6.C
> @@ -0,0 +1,33 @@
> +// PR c++/12672
> +// Verify we don't substitute explicit template arguments into
> +// candidate function templates when the arity of the function
> +// template disagrees with the arity of the call.
> +
> +template<class T>
> +struct A { typedef typename T::type type; };
> +
> +template<class T> void f(T); // arity 1
> +template<class T> void f(T, T, T); // arity 3
> +
> +template<class T> typename A<T>::type f(T, T); // arity 2
> +template<class T, class U> typename A<T>::type f(U, U); // arity 2
> +
> +struct B {
> +  template<class T> void f(T); // arity 1
> +  template<class T> void f(T, T, T); // arity 3
> +
> +  template<class T> typename A<T>::type f(T, T); // arity 2
> +  template<class T, class U> typename A<T>::type f(U, U); // arity 2
> +};
> +
> +int main() {
> +  // If overload resolution attempts deduction for any of the arity-2 function
> +  // templates, the substitution of explicit arguments into the template would
> +  // cause a hard error.
> +  f<int>(1);
> +  f<int>(1, 1, 1);
> +
> +  B b;
> +  b.f<int>(1);
> +  b.f<int>(1, 1, 1);
> +}
> diff --git a/gcc/testsuite/g++.old-deja/g++.pt/unify6.C b/gcc/testsuite/g++.old-deja/g++.pt/unify6.C
> index d122ec2dcb9..ee14ceadd91 100644
> --- a/gcc/testsuite/g++.old-deja/g++.pt/unify6.C
> +++ b/gcc/testsuite/g++.old-deja/g++.pt/unify6.C
> @@ -23,8 +23,8 @@ template<class T> void foo(T const *){} // { dg-error "pointer to reference" }
>   
>   void f()
>   {
> -  foo<int &>(); // { dg-error "" } attempt to build int & const *
> -  foo<void ()>(); // { dg-error "" } attempt to build void (const *)()
> +  foo<int &>(0); // { dg-error "" } attempt to build int & const *
> +  foo<void ()>(0); // OK by [dcl.fct]/7, the const is silently dropped
>   }
>   
>   typedef void (*Fptr)();
>