[PATCH] c++: shortcut bad convs during overload resolution [PR101904]

Patrick Palka ppalka@redhat.com
Tue Aug 31 01:26:54 GMT 2021


In the context of overload resolution we have the notion of a "bad"
argument conversion, which is a conversion that "would be a permitted
with a bending of the language standards", and we handle such bad
conversions specially.  In particular, we rank a bad conversion as
better than no conversion but worse than a good conversion, and a bad
conversion doesn't necessarily make a candidate unviable.  With the
flag -fpermissive, we permit the situation where overload resolution
selects a candidate that contains a bad conversion (which we call a
non-strictly viable candidate).  And without the flag we issue a
distinct permerror in this situation instead.

One consequence of this defacto behavior is that in order to distinguish
a non-strictly viable candidate from an unviable candidate, if we
encounter a bad argument conversion during overload resolution we must
keep converting subsequent arguments because a subsequent conversion
could render the candidate unviable instead of just non-strictly viable.
But checking subsequent arguments can force template instantiations and
result in otherwise avoidable hard errors.  And in particular, all
'this' conversions are at worst bad, so this means the const/ref-qualifiers
of a member function can't be used to prune a candidate quickly, which
is the subject of the mentioned PR.

This patch tries to improve the situation without changing the defacto
output of add_candidates.  Specifically, when considering a candidate
during overload resolution this patch makes us shortcut argument
conversion checking upon encountering the first bad conversion
(tentatively marking the candidate as non-strictly viable, though it
could ultimately be unviable) under the assumption that we'll eventually
find a strictly viable candidate anyway (rendering the distinction
between non-strictly viable and unviable moot, since both are worse
than a strictly viable candidate).  If this assumption turns out to be
false, we'll fully reconsider the candidate under the defacto behavior
(without the shortcutting).

So in the best case (there's a strictly viable candidate), we avoid
some argument conversions and/or template argument deduction that may
cause a hard error.  In the worst case (there's no such candidate), we
have to redundantly consider some candidates twice.  (In a previous
version of the patch, to avoid this redundant checking I created a new
"deferred" conversion type that represents a conversion that is yet to
be performed, and instead of reconsidering a candidate I just realized
its deferred conversions.  But it doesn't seem this redundancy is a
significant performance issue to justify the added complexity of this
other approach.)

Lots of care was taken to preserve the defacto behavior w.r.t.
non-strictly viable candidates, but I wonder how important this behavior
is nowadays?  Can the notion of a non-strictly viable candidate be done
away with, or is it here to stay?

Bootstrapped and regtested on x86_64-pc-linux-gnu.

	PR c++/101904

gcc/cp/ChangeLog:

	* call.c (build_this_conversion): New function, split out from
	add_function_candidate.
	(add_function_candidate): New parameter shortcut_bad_convs.
	Document it.  Use build_this_conversion.  Stop at the first bad
	argument conversion when shortcut_bad_convs is true.
	(add_template_candidate_real): New parameter shortcut_bad_convs.
	Use build_this_conversion to check the 'this' conversion before
	attempting deduction.  When the rejection reason code is
	rr_bad_arg_conversion, pass -1 instead of 0 as the viable
	parameter to add_candidate.  Pass 'convs' to add_candidate.
	(add_template_candidate): New parameter shortcut_bad_convs.
	(add_template_conv_candidate): Pass false as shortcut_bad_convs
	to add_template_candidate_real.
	(add_candidates): Prefer to shortcut bad conversions during
	overload resolution under the assumption that we'll eventually
	see a strictly viable candidate.  If this assumption turns out
	to be false, re-process the non-strictly viable candidates
	without shortcutting those bad conversions.

gcc/testsuite/ChangeLog:

	* g++.dg/template/conv17.C: New test.
---
 gcc/cp/call.c                          | 250 +++++++++++++++++--------
 gcc/testsuite/g++.dg/template/conv17.C |  56 ++++++
 2 files changed, 233 insertions(+), 73 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/conv17.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index e4df72ec1a3..2a0bf933a89 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -175,17 +175,17 @@ static struct z_candidate *splice_viable (struct z_candidate *, bool, bool *);
 static bool any_strictly_viable (struct z_candidate *);
 static struct z_candidate *add_template_candidate
 	(struct z_candidate **, tree, tree, tree, tree, const vec<tree, va_gc> *,
-	 tree, tree, tree, int, unification_kind_t, tsubst_flags_t);
+	 tree, tree, tree, int, unification_kind_t, bool, tsubst_flags_t);
 static struct z_candidate *add_template_candidate_real
 	(struct z_candidate **, tree, tree, tree, tree, const vec<tree, va_gc> *,
-	 tree, tree, tree, int, tree, unification_kind_t, tsubst_flags_t);
+	 tree, tree, tree, int, tree, unification_kind_t, bool, tsubst_flags_t);
 static bool is_complete (tree);
 static struct z_candidate *add_conv_candidate
 	(struct z_candidate **, tree, tree, const vec<tree, va_gc> *, tree,
 	 tree, tsubst_flags_t);
 static struct z_candidate *add_function_candidate
 	(struct z_candidate **, tree, tree, tree, const vec<tree, va_gc> *, tree,
-	 tree, int, conversion**, tsubst_flags_t);
+	 tree, int, conversion**, bool, tsubst_flags_t);
 static conversion *implicit_conversion (tree, tree, tree, bool, int,
 					tsubst_flags_t);
 static conversion *reference_binding (tree, tree, tree, bool, int,
@@ -2241,6 +2241,56 @@ conv_flags (int i, int nargs, tree fn, tree arg, int flags)
   return lflags;
 }
 
+/* Build an appropriate 'this' conversion for the method FN and class
+   type CTYPE from the value ARG (having type ARGTYPE) to the type PARMTYPE.
+   This function modifies PARMTYPE, ARGTYPE and ARG.  */
+
+static conversion *
+build_this_conversion (tree fn, tree ctype,
+		       tree& parmtype, tree& argtype, tree& arg,
+		       int flags, tsubst_flags_t complain)
+{
+  gcc_assert (DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
+	      && !DECL_CONSTRUCTOR_P (fn));
+
+  /* The type of the implicit object parameter ('this') for
+     overload resolution is not always the same as for the
+     function itself; conversion functions are considered to
+     be members of the class being converted, and functions
+     introduced by a using-declaration are considered to be
+     members of the class that uses them.
+
+     Since build_over_call ignores the ICS for the `this'
+     parameter, we can just change the parm type.  */
+  parmtype = cp_build_qualified_type (ctype,
+				      cp_type_quals (TREE_TYPE (parmtype)));
+  bool this_p = true;
+  if (FUNCTION_REF_QUALIFIED (TREE_TYPE (fn)))
+    {
+      /* If the function has a ref-qualifier, the implicit
+	 object parameter has reference type.  */
+      bool rv = FUNCTION_RVALUE_QUALIFIED (TREE_TYPE (fn));
+      parmtype = cp_build_reference_type (parmtype, rv);
+      /* The special handling of 'this' conversions in compare_ics
+	 does not apply if there is a ref-qualifier.  */
+      this_p = false;
+    }
+  else
+    {
+      parmtype = build_pointer_type (parmtype);
+      /* We don't use build_this here because we don't want to
+	 capture the object argument until we've chosen a
+	 non-static member function.  */
+      arg = build_address (arg);
+      argtype = lvalue_type (arg);
+    }
+  flags |= LOOKUP_ONLYCONVERTING;
+  conversion *t = implicit_conversion (parmtype, argtype, arg,
+				       /*c_cast_p=*/false, flags, complain);
+  t->this_p = this_p;
+  return t;
+}
+
 /* Create an overload candidate for the function or method FN called
    with the argument list FIRST_ARG/ARGS and add it to CANDIDATES.
    FLAGS is passed on to implicit_conversion.
@@ -2248,7 +2298,14 @@ conv_flags (int i, int nargs, tree fn, tree arg, int flags)
    This does not change ARGS.
 
    CTYPE, if non-NULL, is the type we want to pretend this function
-   comes from for purposes of overload resolution.  */
+   comes from for purposes of overload resolution.
+
+   SHORTCUT_BAD_CONVS controls how we handle "bad" argument conversions.
+   If true, we stop computing conversions upon seeing the first bad
+   conversion.  This is used by add_candidates to avoid computing
+   more conversions than necessary in the presence of a strictly viable
+   candidate, while preserving the defacto behavior of overload resolution
+   when it turns out there are only non-strictly viable candidates.  */
 
 static struct z_candidate *
 add_function_candidate (struct z_candidate **candidates,
@@ -2256,6 +2313,7 @@ add_function_candidate (struct z_candidate **candidates,
 			const vec<tree, va_gc> *args, tree access_path,
 			tree conversion_path, int flags,
 			conversion **convs,
+			bool shortcut_bad_convs,
 			tsubst_flags_t complain)
 {
   tree parmlist = TYPE_ARG_TYPES (TREE_TYPE (fn));
@@ -2377,8 +2435,6 @@ add_function_candidate (struct z_candidate **candidates,
     {
       tree argtype, to_type;
       tree arg;
-      conversion *t;
-      int is_this;
 
       if (parmnode == void_list_node)
 	break;
@@ -2397,54 +2453,23 @@ add_function_candidate (struct z_candidate **candidates,
 		(*args)[i + skip - (first_arg != NULL_TREE ? 1 : 0)]);
       argtype = lvalue_type (arg);
 
-      is_this = (i == 0 && DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
-		 && ! DECL_CONSTRUCTOR_P (fn));
-
+      conversion *t;
       if (parmnode)
 	{
 	  tree parmtype = TREE_VALUE (parmnode);
-
-	  parmnode = TREE_CHAIN (parmnode);
-
-	  /* The type of the implicit object parameter ('this') for
-	     overload resolution is not always the same as for the
-	     function itself; conversion functions are considered to
-	     be members of the class being converted, and functions
-	     introduced by a using-declaration are considered to be
-	     members of the class that uses them.
-
-	     Since build_over_call ignores the ICS for the `this'
-	     parameter, we can just change the parm type.  */
-	  if (ctype && is_this)
+	  if (i == 0
+	      && DECL_NONSTATIC_MEMBER_FUNCTION_P (fn)
+	      && !DECL_CONSTRUCTOR_P (fn))
+	    t = build_this_conversion (fn, ctype, parmtype, argtype, arg,
+				       flags, complain);
+	  else
 	    {
-	      parmtype = cp_build_qualified_type
-		(ctype, cp_type_quals (TREE_TYPE (parmtype)));
-	      if (FUNCTION_REF_QUALIFIED (TREE_TYPE (fn)))
-		{
-		  /* If the function has a ref-qualifier, the implicit
-		     object parameter has reference type.  */
-		  bool rv = FUNCTION_RVALUE_QUALIFIED (TREE_TYPE (fn));
-		  parmtype = cp_build_reference_type (parmtype, rv);
-		  /* The special handling of 'this' conversions in compare_ics
-		     does not apply if there is a ref-qualifier.  */
-		  is_this = false;
-		}
-	      else
-		{
-		  parmtype = build_pointer_type (parmtype);
-		  /* We don't use build_this here because we don't want to
-		     capture the object argument until we've chosen a
-		     non-static member function.  */
-		  arg = build_address (arg);
-		  argtype = lvalue_type (arg);
-		}
+	      int lflags = conv_flags (i, len-skip, fn, arg, flags);
+	      t = implicit_conversion (parmtype, argtype, arg,
+				       /*c_cast_p=*/false, lflags, complain);
 	    }
-
-	  int lflags = conv_flags (i, len-skip, fn, arg, flags);
-
-	  t = implicit_conversion (parmtype, argtype, arg,
-				   /*c_cast_p=*/false, lflags, complain);
 	  to_type = parmtype;
+	  parmnode = TREE_CHAIN (parmnode);
 	}
       else
 	{
@@ -2453,9 +2478,6 @@ add_function_candidate (struct z_candidate **candidates,
 	  to_type = argtype;
 	}
 
-      if (t && is_this)
-	t->this_p = true;
-
       convs[i] = t;
       if (! t)
 	{
@@ -2470,7 +2492,8 @@ add_function_candidate (struct z_candidate **candidates,
 	  viable = -1;
 	  reason = bad_arg_conversion_rejection (first_arg, i, arg, to_type,
 						 EXPR_LOCATION (arg));
-
+	  if (shortcut_bad_convs)
+	    break;
 	}
     }
 
@@ -3354,7 +3377,9 @@ add_builtin_candidates (struct z_candidate **candidates, enum tree_code code,
    This does not change ARGLIST.  The RETURN_TYPE is the desired type
    for conversion operators.  If OBJ is NULL_TREE, FLAGS and CTYPE are
    as for add_function_candidate.  If an OBJ is supplied, FLAGS and
-   CTYPE are ignored, and OBJ is as for add_conv_candidate.  */
+   CTYPE are ignored, and OBJ is as for add_conv_candidate.
+
+   SHORTCUT_BAD_CONVS is as in add_function_candidate.  */
 
 static struct z_candidate*
 add_template_candidate_real (struct z_candidate **candidates, tree tmpl,
@@ -3362,7 +3387,7 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl,
 			     const vec<tree, va_gc> *arglist, tree return_type,
 			     tree access_path, tree conversion_path,
 			     int flags, tree obj, unification_kind_t strict,
-			     tsubst_flags_t complain)
+			     bool shortcut_bad_convs, tsubst_flags_t complain)
 {
   int ntparms = DECL_NTPARMS (tmpl);
   tree targs = make_tree_vec (ntparms);
@@ -3454,7 +3479,33 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl,
 
   errs = errorcount+sorrycount;
   if (!obj)
-    convs = alloc_conversions (nargs);
+    {
+      convs = alloc_conversions (nargs);
+
+      if (shortcut_bad_convs
+	  && DECL_NONSTATIC_MEMBER_FUNCTION_P (tmpl)
+	  && !DECL_CONSTRUCTOR_P (tmpl))
+	{
+	  /* Check the 'this' conversion before proceeding with deduction.
+	     This is effectively an extension of the DR 1391 resolution
+	     that we perform in check_non_deducible_conversions, though it's
+	     convenient to do this extra check here instead of there.  */
+	  tree parmtype = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (tmpl)));
+	  tree argtype = lvalue_type (first_arg);
+	  tree arg = first_arg;
+	  conversion *t = build_this_conversion (tmpl, ctype,
+						 parmtype, argtype, arg,
+						 flags, complain);
+	  convs[0] = t;
+	  if (t->bad_p)
+	    {
+	      reason = bad_arg_conversion_rejection (first_arg, 0,
+						     arg, parmtype,
+						     EXPR_LOCATION (arg));
+	      goto fail;
+	    }
+	}
+    }
   fn = fn_type_unification (tmpl, explicit_targs, targs,
 			    args_without_in_chrg,
 			    nargs_without_in_chrg,
@@ -3501,7 +3552,8 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl,
   else
     cand = add_function_candidate (candidates, fn, ctype,
 				   first_arg, arglist, access_path,
-				   conversion_path, flags, convs, complain);
+				   conversion_path, flags, convs,
+				   shortcut_bad_convs, complain);
   if (DECL_TI_TEMPLATE (fn) != tmpl)
     /* This situation can occur if a member template of a template
        class is specialized.  Then, instantiate_template might return
@@ -3527,8 +3579,9 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl,
 
   return cand;
  fail:
-  return add_candidate (candidates, tmpl, first_arg, arglist, nargs, NULL,
-			access_path, conversion_path, 0, reason, flags);
+  int viable = (reason->code == rr_bad_arg_conversion ? -1 : 0);
+  return add_candidate (candidates, tmpl, first_arg, arglist, nargs, convs,
+			access_path, conversion_path, viable, reason, flags);
 }
 
 
@@ -3537,13 +3590,15 @@ add_template_candidate (struct z_candidate **candidates, tree tmpl, tree ctype,
 			tree explicit_targs, tree first_arg,
 			const vec<tree, va_gc> *arglist, tree return_type,
 			tree access_path, tree conversion_path, int flags,
-			unification_kind_t strict, tsubst_flags_t complain)
+			unification_kind_t strict, bool shortcut_bad_convs,
+			tsubst_flags_t complain)
 {
   return
     add_template_candidate_real (candidates, tmpl, ctype,
 				 explicit_targs, first_arg, arglist,
 				 return_type, access_path, conversion_path,
-				 flags, NULL_TREE, strict, complain);
+				 flags, NULL_TREE, strict, shortcut_bad_convs,
+				 complain);
 }
 
 /* Create an overload candidate for the conversion function template TMPL,
@@ -3569,7 +3624,7 @@ add_template_conv_candidate (struct z_candidate **candidates, tree tmpl,
     add_template_candidate_real (candidates, tmpl, NULL_TREE, NULL_TREE,
 				 NULL_TREE, arglist, return_type, access_path,
 				 conversion_path, 0, obj, DEDUCE_CALL,
-				 complain);
+				 /*shortcut_bad_convs=*/false, complain);
 }
 
 /* The CANDS are the set of candidates that were considered for
@@ -6013,6 +6068,7 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
     /* Delay creating the implicit this parameter until it is needed.  */
     non_static_args = NULL;
 
+  bool seen_strictly_viable = any_strictly_viable (*candidates);
   /* If there's a non-template perfect match, we don't need to consider
      templates.  So check non-templates first.  This optimization is only
      really needed for the defaulted copy constructor of tuple and the like
@@ -6024,6 +6080,19 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
   else /*if (flags & LOOKUP_DEFAULTED)*/
     which = non_templates;
 
+  /* During overload resolution, we first consider each function under the
+     assumption that we'll eventually find a strictly viable candidate.
+     This allows us to circumvent our defacto behavior when checking
+     argument conversions and shortcut consideration of the candidate
+     upon encountering the first bad conversion.  If this assumption
+     turns out to be false, and all candidates end up being non-strictly
+     viable, then we reconsider such candidates under the defacto behavior.
+     This trick is important in order to be able to prune member function
+     overloads according to their const/ref-qualifiers (since all 'this'
+     conversions are at worst bad) without breaking -fpermissive.  */
+  tree bad_fns = NULL_TREE;
+  bool shortcut_bad_convs = true;
+
  again:
   for (tree fn : lkp_range (fns))
     {
@@ -6070,18 +6139,22 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
 	}
 
       if (TREE_CODE (fn) == TEMPLATE_DECL)
-	add_template_candidate (candidates,
-				fn,
-				ctype,
-				explicit_targs,
-				fn_first_arg,
-				fn_args,
-				return_type,
-				access_path,
-				conversion_path,
-				flags,
-				strict,
-				complain);
+	{
+	  if (!add_template_candidate (candidates,
+				       fn,
+				       ctype,
+				       explicit_targs,
+				       fn_first_arg,
+				       fn_args,
+				       return_type,
+				       access_path,
+				       conversion_path,
+				       flags,
+				       strict,
+				       shortcut_bad_convs,
+				       complain))
+	    continue;
+	}
       else
 	{
 	  add_function_candidate (candidates,
@@ -6093,16 +6166,47 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
 				  conversion_path,
 				  flags,
 				  NULL,
+				  shortcut_bad_convs,
 				  complain);
 	  if (perfect_candidate_p (*candidates))
 	    seen_perfect = true;
 	}
+
+      z_candidate *cand = *candidates;
+      if (cand->viable == 1)
+	seen_strictly_viable = true;
+
+      if (cand->viable == -1
+	  && shortcut_bad_convs
+	  && !cand->convs[cand->num_convs - 1])
+	{
+	  /* This candidate has been tentatively marked non-strictly viable,
+	     and we didn't compute all argument conversions for it (having
+	     stopped at the first bad conversion).  Add the function to BAD_FNS
+	     to fully reconsider later if we fail to obtain any strictly viable
+	     candidates.  */
+	  bad_fns = lookup_add (fn, bad_fns);
+	  *candidates = (*candidates)->next;
+	}
     }
   if (which == non_templates && !seen_perfect)
     {
       which = templates;
       goto again;
     }
+  else if (which == templates
+	   && !seen_strictly_viable
+	   && shortcut_bad_convs
+	   && bad_fns)
+    {
+      /* None of the candidates are strictly viable, so check again those
+	 functions in BAD_FNS, this time without shortcutting bad conversions
+	 so that all their argument conversions are computed.  */
+      which = either;
+      fns = bad_fns;
+      shortcut_bad_convs = false;
+      goto again;
+    }
 }
 
 /* Returns 1 if P0145R2 says that the LHS of operator CODE is evaluated first,
diff --git a/gcc/testsuite/g++.dg/template/conv17.C b/gcc/testsuite/g++.dg/template/conv17.C
new file mode 100644
index 00000000000..794dcf00657
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/conv17.C
@@ -0,0 +1,56 @@
+// PR c++/101904
+// Verify we stop at the first bad conversion when considering a candidate
+// during overload resolution.
+
+template<class T>
+struct A { typedef typename T::type type; };
+
+struct B {
+  // A conversion function that always induces a hard error.
+  template<class T> B(T, typename A<T>::type = 0);
+};
+
+struct C {
+  template<class T> void f(T, typename A<T>::type); // #1
+  template<class T> void f(T, T) const;             // #2
+
+  static void g(int*, B);                           // #3
+  static void g(int, int);                          // #4
+
+#if __cpp_ref_qualifiers
+  void h(B) &;                                      // #5
+  void h(int) &&;                                   // #6
+#endif
+};
+
+int main() {
+  const C c;
+
+  // The bad conversion for the 'this' argument should preclude us from further
+  // considering the non-const #1 (which would have caused a hard error during
+  // instantiation).  This behavior is essentially DR 1391 extended to the
+  // 'this' argument.
+  c.f(0, 0); // resolves to #2
+  c.f<int>(0, 0);
+
+  // Likewise for the bad conversion for the 1st argument in #3.
+  C::g(42, 42); // resolves to #4
+
+#if __cpp_ref_qualifiers
+  // Likewise for the bad 'this' conversion in #5.
+  C().h(0); // resolves to #6
+#endif
+}
+
+#if __cpp_concepts
+// Test the same calls in a SFINAE context.
+template<class T>
+concept D = requires (const T t) {
+  t.f(0, 0);
+  t.f<int>(0, );
+  T::g(42, 42);
+  T().h(0);
+};
+
+static_assert(D<C>);
+#endif
-- 
2.33.0.69.gc4203212e3



More information about the Gcc-patches mailing list