[PATCH] c++: Fix ICE with bad conversion shortcutting [PR104622]

[PATCH] c++: Fix ICE with bad conversion shortcutting [PR104622]

[Patrick Palka]

When shortcutting bad conversions during overload resolution, we assume
argument conversions get computed in sequential order and that therefore
we just need to inspect the last conversion in order to determine if _any_
conversion is missing.  But this assumption turns out to be false for
templates, because during deduction check_non_deducible_conversion can
compute an argument conversion out of order.

So in the testcase below, at the end of add_template_candidate the convs
array looks like {bad, missing, good} where the last conversion was
computed during deduction and the first was computed later from
add_function_candidate.  We need to add this candidate to BAD_FNS since
not all of its argument conversions were computed, but we don't do so
because we only checked if the last argument conversion was missing.

This patch fixes this by checking for a missing conversion exhaustively.
In passing, this cleans up check_non_deducible_conversion a bit since
AFAICT the only values of strict we expect to see here are the three
enumerators of unification_kind_t.

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

	PR c++/104622

gcc/cp/ChangeLog:

	* call.cc (missing_conversion_p): Define.
	(add_candidates): Use it.
	* pt.cc (check_non_deducible_conversion): Change type of strict
	parameter to unification_kind_t and directly test for DEDUCE_CALL.

gcc/testsuite/ChangeLog:

	* g++.dg/template/conv18.C: New test.
---
 gcc/cp/call.cc                         | 13 ++++++++++++-
 gcc/cp/pt.cc                           |  6 +++---
 gcc/testsuite/g++.dg/template/conv18.C | 14 ++++++++++++++
 3 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/conv18.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index d6eed5ed835..8fe8ef306ea 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -6023,6 +6023,17 @@ perfect_candidate_p (z_candidate *cand)
   return true;
 }
 
+/* True iff one of CAND's argument conversions is NULL.  */
+
+static bool
+missing_conversion_p (const z_candidate *cand)
+{
+  for (unsigned i = 0; i < cand->num_convs; ++i)
+    if (!cand->convs[i])
+      return true;
+  return false;
+}
+
 /* Add each of the viable functions in FNS (a FUNCTION_DECL or
    OVERLOAD) to the CANDIDATES, returning an updated list of
    CANDIDATES.  The ARGS are the arguments provided to the call;
@@ -6200,7 +6211,7 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
 
       if (cand->viable == -1
 	  && shortcut_bad_convs
-	  && !cand->convs[cand->reversed () ? 0 : cand->num_convs - 1])
+	  && missing_conversion_p (cand))
 	{
 	  /* This candidate has been tentatively marked non-strictly viable,
 	     and we didn't compute all argument conversions for it (having
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index f890d92d715..715eea27577 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -152,7 +152,7 @@ static tree coerce_innermost_template_parms (tree, tree, tree, tsubst_flags_t,
 					      bool, bool);
 static void tsubst_enum	(tree, tree, tree);
 static bool check_instantiated_args (tree, tree, tsubst_flags_t);
-static int check_non_deducible_conversion (tree, tree, int, int,
+static int check_non_deducible_conversion (tree, tree, unification_kind_t, int,
 					   struct conversion **, bool);
 static int maybe_adjust_types_for_deduction (tree, unification_kind_t,
 					     tree*, tree*, tree);
@@ -22304,7 +22304,7 @@ maybe_adjust_types_for_deduction (tree tparms,
    unify_one_argument.  */
 
 static int
-check_non_deducible_conversion (tree parm, tree arg, int strict,
+check_non_deducible_conversion (tree parm, tree arg, unification_kind_t strict,
 				int flags, struct conversion **conv_p,
 				bool explain_p)
 {
@@ -22324,7 +22324,7 @@ check_non_deducible_conversion (tree parm, tree arg, int strict,
       if (can_convert_arg (type, parm, NULL_TREE, flags, complain))
 	return unify_success (explain_p);
     }
-  else if (strict != DEDUCE_EXACT)
+  else if (strict == DEDUCE_CALL)
     {
       bool ok = false;
       tree conv_arg = TYPE_P (arg) ? NULL_TREE : arg;
diff --git a/gcc/testsuite/g++.dg/template/conv18.C b/gcc/testsuite/g++.dg/template/conv18.C
new file mode 100644
index 00000000000..f59f6fda77c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/conv18.C
@@ -0,0 +1,14 @@
+// PR c++/104622
+// { dg-additional-options "-fpermissive" }
+
+template<class T>
+struct type_identity {
+  typedef T type;
+};
+
+template<class T> void f(typename type_identity<T>::type*, T, int*);
+
+int main() {
+  const int p = 0;
+  f(&p, 0, 0); // { dg-warning "invalid conversion" }
+}
-- 
2.35.1.455.g1a4874565f

Re: [PATCH] c++: Fix ICE with bad conversion shortcutting [PR104622]

[Jason Merrill]

On 3/10/22 15:30, Patrick Palka wrote:
> When shortcutting bad conversions during overload resolution, we assume
> argument conversions get computed in sequential order and that therefore
> we just need to inspect the last conversion in order to determine if _any_
> conversion is missing.  But this assumption turns out to be false for
> templates, because during deduction check_non_deducible_conversion can
> compute an argument conversion out of order.
> 
> So in the testcase below, at the end of add_template_candidate the convs
> array looks like {bad, missing, good} where the last conversion was
> computed during deduction and the first was computed later from
> add_function_candidate.  We need to add this candidate to BAD_FNS since
> not all of its argument conversions were computed, but we don't do so
> because we only checked if the last argument conversion was missing.
> 
> This patch fixes this by checking for a missing conversion exhaustively.
> In passing, this cleans up check_non_deducible_conversion a bit since
> AFAICT the only values of strict we expect to see here are the three
> enumerators of unification_kind_t.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?

OK.

> 	PR c++/104622
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (missing_conversion_p): Define.
> 	(add_candidates): Use it.
> 	* pt.cc (check_non_deducible_conversion): Change type of strict
> 	parameter to unification_kind_t and directly test for DEDUCE_CALL.
>
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/conv18.C: New test.
> ---
>   gcc/cp/call.cc                         | 13 ++++++++++++-
>   gcc/cp/pt.cc                           |  6 +++---
>   gcc/testsuite/g++.dg/template/conv18.C | 14 ++++++++++++++
>   3 files changed, 29 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/conv18.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index d6eed5ed835..8fe8ef306ea 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -6023,6 +6023,17 @@ perfect_candidate_p (z_candidate *cand)
>     return true;
>   }
>   
> +/* True iff one of CAND's argument conversions is NULL.  */
> +
> +static bool
> +missing_conversion_p (const z_candidate *cand)
> +{
> +  for (unsigned i = 0; i < cand->num_convs; ++i)
> +    if (!cand->convs[i])
> +      return true;
> +  return false;
> +}
> +
>   /* Add each of the viable functions in FNS (a FUNCTION_DECL or
>      OVERLOAD) to the CANDIDATES, returning an updated list of
>      CANDIDATES.  The ARGS are the arguments provided to the call;
> @@ -6200,7 +6211,7 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
>   
>         if (cand->viable == -1
>   	  && shortcut_bad_convs
> -	  && !cand->convs[cand->reversed () ? 0 : cand->num_convs - 1])
> +	  && missing_conversion_p (cand))
>   	{
>   	  /* This candidate has been tentatively marked non-strictly viable,
>   	     and we didn't compute all argument conversions for it (having
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index f890d92d715..715eea27577 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -152,7 +152,7 @@ static tree coerce_innermost_template_parms (tree, tree, tree, tsubst_flags_t,
>   					      bool, bool);
>   static void tsubst_enum	(tree, tree, tree);
>   static bool check_instantiated_args (tree, tree, tsubst_flags_t);
> -static int check_non_deducible_conversion (tree, tree, int, int,
> +static int check_non_deducible_conversion (tree, tree, unification_kind_t, int,
>   					   struct conversion **, bool);
>   static int maybe_adjust_types_for_deduction (tree, unification_kind_t,
>   					     tree*, tree*, tree);
> @@ -22304,7 +22304,7 @@ maybe_adjust_types_for_deduction (tree tparms,
>      unify_one_argument.  */
>   
>   static int
> -check_non_deducible_conversion (tree parm, tree arg, int strict,
> +check_non_deducible_conversion (tree parm, tree arg, unification_kind_t strict,
>   				int flags, struct conversion **conv_p,
>   				bool explain_p)
>   {
> @@ -22324,7 +22324,7 @@ check_non_deducible_conversion (tree parm, tree arg, int strict,
>         if (can_convert_arg (type, parm, NULL_TREE, flags, complain))
>   	return unify_success (explain_p);
>       }
> -  else if (strict != DEDUCE_EXACT)
> +  else if (strict == DEDUCE_CALL)
>       {
>         bool ok = false;
>         tree conv_arg = TYPE_P (arg) ? NULL_TREE : arg;
> diff --git a/gcc/testsuite/g++.dg/template/conv18.C b/gcc/testsuite/g++.dg/template/conv18.C
> new file mode 100644
> index 00000000000..f59f6fda77c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/conv18.C
> @@ -0,0 +1,14 @@
> +// PR c++/104622
> +// { dg-additional-options "-fpermissive" }
> +
> +template<class T>
> +struct type_identity {
> +  typedef T type;
> +};
> +
> +template<class T> void f(typename type_identity<T>::type*, T, int*);
> +
> +int main() {
> +  const int p = 0;
> +  f(&p, 0, 0); // { dg-warning "invalid conversion" }
> +}