public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] c++: sort candidates according to viability
@ 2023-10-23 23:51 Patrick Palka
  2023-10-23 23:51 ` [PATCH v2 2/3] c++: remember candidates that we ignored Patrick Palka
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Patrick Palka @ 2023-10-23 23:51 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason, Patrick Palka

The second patch in this series is new and ensures that the candidates
list isn't mysteriously missing some candidates when noting other
candidates due to deletedness.

-- >8 --

This patch:

  * changes splice_viable to move the non-viable candidates to the end
    of the list instead of removing them outright
  * makes tourney move the best candidate to the front of the candidate
    list
  * adjusts print_z_candidates to preserve our behavior of printing only
    viable candidates when diagnosing ambiguity
  * adds a parameter to print_z_candidates to control this default behavior
    (the follow-up patch will want to print all candidates when diagnosing
    deletedness)

Thus after this patch we have access to the entire candidate list through
the best viable candidate.

This change also happens to fix diagnostics for the below testcase where
we currently neglect to note the third candidate, since the presence of
the two unordered non-strictly viable candidates causes splice_viable to
prematurely get rid of the non-viable third candidate.

gcc/cp/ChangeLog:

	* call.cc: Include "tristate.h".
	(splice_viable): Sort the candidate list according to viability.
	Don't remove non-viable candidates from the list.
	(print_z_candidates): Add defaulted only_viable_p parameter.
	By default only print non-viable candidates if there is no
	viable candidate.
	(tourney): Make 'candidates' parameter a reference.  Ignore
	non-viable candidates.  Move the true champ to the front
	of the candidates list, and update 'candidates' to point to
	the front.

gcc/testsuite/ChangeLog:

	* g++.dg/overload/error5.C: New test.
---
 gcc/cp/call.cc                         | 163 +++++++++++++++----------
 gcc/testsuite/g++.dg/overload/error5.C |  12 ++
 2 files changed, 113 insertions(+), 62 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/overload/error5.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 2eb54b5b6ed..89d422f7220 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -43,6 +43,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "attribs.h"
 #include "decl.h"
 #include "gcc-rich-location.h"
+#include "tristate.h"
 
 /* The various kinds of conversion.  */
 
@@ -160,7 +161,7 @@ static struct obstack conversion_obstack;
 static bool conversion_obstack_initialized;
 struct rejection_reason;
 
-static struct z_candidate * tourney (struct z_candidate *, tsubst_flags_t);
+static struct z_candidate * tourney (struct z_candidate *&, tsubst_flags_t);
 static int equal_functions (tree, tree);
 static int joust (struct z_candidate *, struct z_candidate *, bool,
 		  tsubst_flags_t);
@@ -176,7 +177,8 @@ static void op_error (const op_location_t &, enum tree_code, enum tree_code,
 static struct z_candidate *build_user_type_conversion_1 (tree, tree, int,
 							 tsubst_flags_t);
 static void print_z_candidate (location_t, const char *, struct z_candidate *);
-static void print_z_candidates (location_t, struct z_candidate *);
+static void print_z_candidates (location_t, struct z_candidate *,
+				tristate = tristate::unknown ());
 static tree build_this (tree);
 static struct z_candidate *splice_viable (struct z_candidate *, bool, bool *);
 static bool any_strictly_viable (struct z_candidate *);
@@ -3700,68 +3702,60 @@ add_template_conv_candidate (struct z_candidate **candidates, tree tmpl,
 }
 
 /* The CANDS are the set of candidates that were considered for
-   overload resolution.  Return the set of viable candidates, or CANDS
-   if none are viable.  If any of the candidates were viable, set
+   overload resolution.  Sort CANDS so that the strictly viable
+   candidates appear first, followed by non-strictly viable candidates,
+   followed by unviable candidates.  Returns the first candidate
+   in this sorted list.  If any of the candidates were viable, set
    *ANY_VIABLE_P to true.  STRICT_P is true if a candidate should be
-   considered viable only if it is strictly viable.  */
+   considered viable only if it is strictly viable when setting
+   *ANY_VIABLE_P.  */
 
 static struct z_candidate*
 splice_viable (struct z_candidate *cands,
 	       bool strict_p,
 	       bool *any_viable_p)
 {
-  struct z_candidate *viable;
-  struct z_candidate **last_viable;
-  struct z_candidate **cand;
-  bool found_strictly_viable = false;
+  z_candidate *strictly_viable = nullptr;
+  z_candidate **strictly_viable_tail = &strictly_viable;
+
+  z_candidate *non_strictly_viable = nullptr;
+  z_candidate **non_strictly_viable_tail = &non_strictly_viable;
+
+  z_candidate *unviable = nullptr;
+  z_candidate **unviable_tail = &unviable;
 
   /* Be strict inside templates, since build_over_call won't actually
      do the conversions to get pedwarns.  */
   if (processing_template_decl)
     strict_p = true;
 
-  viable = NULL;
-  last_viable = &viable;
-  *any_viable_p = false;
-
-  cand = &cands;
-  while (*cand)
+  for (z_candidate *cand = cands; cand; cand = cand->next)
     {
-      struct z_candidate *c = *cand;
       if (!strict_p
-	  && (c->viable == 1 || TREE_CODE (c->fn) == TEMPLATE_DECL))
-	{
-	  /* Be strict in the presence of a viable candidate.  Also if
-	     there are template candidates, so that we get deduction errors
-	     for them instead of silently preferring a bad conversion.  */
-	  strict_p = true;
-	  if (viable && !found_strictly_viable)
-	    {
-	      /* Put any spliced near matches back onto the main list so
-		 that we see them if there is no strict match.  */
-	      *any_viable_p = false;
-	      *last_viable = cands;
-	      cands = viable;
-	      viable = NULL;
-	      last_viable = &viable;
-	    }
-	}
+	  && (cand->viable == 1 || TREE_CODE (cand->fn) == TEMPLATE_DECL))
+	/* Be strict in the presence of a viable candidate.  Also if
+	   there are template candidates, so that we get deduction errors
+	   for them instead of silently preferring a bad conversion.  */
+	strict_p = true;
 
-      if (strict_p ? c->viable == 1 : c->viable)
-	{
-	  *last_viable = c;
-	  *cand = c->next;
-	  c->next = NULL;
-	  last_viable = &c->next;
-	  *any_viable_p = true;
-	  if (c->viable == 1)
-	    found_strictly_viable = true;
-	}
-      else
-	cand = &c->next;
+      /* Move this candidate to the appropriate list according to
+	 its viability.  */
+      auto& tail = (cand->viable == 1 ? strictly_viable_tail
+		    : cand->viable == -1 ? non_strictly_viable_tail
+		    : unviable_tail);
+      *tail = cand;
+      tail = &cand->next;
     }
 
-  return viable ? viable : cands;
+  *any_viable_p = (strictly_viable != nullptr
+		   || (!strict_p && non_strictly_viable != nullptr));
+
+  /* Combine the lists.  */
+  *unviable_tail = nullptr;
+  *non_strictly_viable_tail = unviable;
+  *strictly_viable_tail = non_strictly_viable;
+
+  return strictly_viable;
 }
 
 static bool
@@ -3995,8 +3989,13 @@ print_z_candidate (location_t loc, const char *msgstr,
     }
 }
 
+/* Print information about each overload candidate in CANDIDATES,
+   which is assumed to have gone through splice_viable and tourney
+   (if splice_viable succeeded).  */
+
 static void
-print_z_candidates (location_t loc, struct z_candidate *candidates)
+print_z_candidates (location_t loc, struct z_candidate *candidates,
+		    tristate only_viable_p /* = tristate::unknown () */)
 {
   struct z_candidate *cand1;
   struct z_candidate **cand2;
@@ -4041,8 +4040,19 @@ print_z_candidates (location_t loc, struct z_candidate *candidates)
 	}
     }
 
+  /* Unless otherwise specified, if there's a (strictly) viable candidate then
+     we assume we're being called as part of diagnosing ambiguity, in which case
+     we want to print only viable candidates since unviable candidates couldn't
+     have contributed to the ambiguity.  */
+  if (only_viable_p.is_unknown ())
+    only_viable_p = candidates->viable == 1;
+
   for (; candidates; candidates = candidates->next)
-    print_z_candidate (loc, N_("candidate:"), candidates);
+    {
+      if (only_viable_p.is_true () && candidates->viable != 1)
+	break;
+      print_z_candidate (loc, N_("candidate:"), candidates);
+    }
 }
 
 /* USER_SEQ is a user-defined conversion sequence, beginning with a
@@ -13169,38 +13179,50 @@ tweak:
 /* Given a list of candidates for overloading, find the best one, if any.
    This algorithm has a worst case of O(2n) (winner is last), and a best
    case of O(n/2) (totally ambiguous); much better than a sorting
-   algorithm.  */
+   algorithm.  The candidates list is assumed to be sorted according
+   to viability (via splice_viable).  */
 
 static struct z_candidate *
-tourney (struct z_candidate *candidates, tsubst_flags_t complain)
+tourney (struct z_candidate *&candidates, tsubst_flags_t complain)
 {
   struct z_candidate *champ = candidates, *challenger;
   int fate;
   struct z_candidate *champ_compared_to_predecessor = nullptr;
+  struct z_candidate *champ_predecessor = nullptr;
+  struct z_candidate *challenger_predecessor = champ;
 
   /* Walk through the list once, comparing each current champ to the next
      candidate, knocking out a candidate or two with each comparison.  */
 
-  for (challenger = champ->next; challenger; )
+  for (challenger = champ->next; challenger && challenger->viable; )
     {
       fate = joust (champ, challenger, 0, complain);
       if (fate == 1)
-	challenger = challenger->next;
+	{
+	  challenger_predecessor = challenger;
+	  challenger = challenger->next;
+	}
       else
 	{
 	  if (fate == 0)
 	    {
+	      champ_predecessor = challenger;
 	      champ = challenger->next;
-	      if (champ == 0)
-		return NULL;
+	      if (!champ || !champ->viable)
+		{
+		  champ = nullptr;
+		  break;
+		}
 	      champ_compared_to_predecessor = nullptr;
 	    }
 	  else
 	    {
 	      champ_compared_to_predecessor = champ;
+	      champ_predecessor = challenger_predecessor;
 	      champ = challenger;
 	    }
 
+	  challenger_predecessor = champ;
 	  challenger = champ->next;
 	}
     }
@@ -13208,15 +13230,32 @@ tourney (struct z_candidate *candidates, tsubst_flags_t complain)
   /* Make sure the champ is better than all the candidates it hasn't yet
      been compared to.  */
 
-  for (challenger = candidates;
-       challenger != champ;
-       challenger = challenger->next)
+  if (champ)
+    for (challenger = candidates;
+	 challenger != champ;
+	 challenger = challenger->next)
+      {
+	if (challenger == champ_compared_to_predecessor)
+	  continue;
+	fate = joust (champ, challenger, 0, complain);
+	if (fate != 1)
+	  {
+	    champ = nullptr;
+	    break;
+	  }
+      }
+
+  if (!champ)
+    return nullptr;
+
+  /* Move the champ to the front of the candidate list.  */
+
+  if (champ != candidates)
     {
-      if (challenger == champ_compared_to_predecessor)
-	continue;
-      fate = joust (champ, challenger, 0, complain);
-      if (fate != 1)
-	return NULL;
+      gcc_checking_assert (champ_predecessor->next == champ);
+      champ_predecessor->next = champ->next ? champ->next->next : nullptr;
+      champ->next = candidates;
+      candidates = champ;
     }
 
   return champ;
diff --git a/gcc/testsuite/g++.dg/overload/error5.C b/gcc/testsuite/g++.dg/overload/error5.C
new file mode 100644
index 00000000000..6a2f3b5ba35
--- /dev/null
+++ b/gcc/testsuite/g++.dg/overload/error5.C
@@ -0,0 +1,12 @@
+// Verify we note all three candidates when diagnosing overload
+// resolution failure.  The presence of the first two (ambiguous)
+// non-strictly viable candidates used to make us prune the third
+// and not note it.
+
+void f(int, int*); // { dg-message "candidate" }
+void f(int*, int); // { dg-message "candidate" }
+void f(int, int, int); // { dg-message "candidate" }
+
+int main() {
+  f(1, 2); // { dg-error "no match|invalid conversion" }
+}
-- 
2.42.0.424.gceadf0f3cf


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

* [PATCH v2 2/3] c++: remember candidates that we ignored
  2023-10-23 23:51 [PATCH v2 1/3] c++: sort candidates according to viability Patrick Palka
@ 2023-10-23 23:51 ` Patrick Palka
  2023-10-24 21:51   ` Jason Merrill
  2023-10-23 23:51 ` [PATCH v2 3/3] c++: note other candidates when diagnosing deletedness Patrick Palka
  2023-10-24 21:28 ` [PATCH v2 1/3] c++: sort candidates according to viability Jason Merrill
  2 siblings, 1 reply; 6+ messages in thread
From: Patrick Palka @ 2023-10-23 23:51 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason, Patrick Palka

During overload resolution, we sometimes outright ignore a function from
the overload set and leave no trace of it in the candidates list, for
example when we find a perfect non-template candidate we discard all
function templates, or when the callee is a template-id we discard all
non-template functions.  We should still however make note of these
unviable functions when diagnosing overload resolution failure, but
that's not possible if they're not present in the returned candidates
list.

To that end, this patch reworks add_candidates to add such ignored
functions to the list.  The new rr_ignored rejection reason is somewhat
of a catch-all; we could perhaps split it up into more specific rejection
reasons, but I leave that as future work.

gcc/cp/ChangeLog:

	* call.cc (enum rejection_reason_code): Add rr_ignored.
	(add_ignored_candidate): Define.
	(ignored_candidate_p): Define.
	(add_template_candidate_real): Do add_ignored_candidate
	instead of returning NULL.
	(splice_viable): Put ignored (unviable) candidates last.
	(print_z_candidate): Handle ignored candidates.
	(build_new_function_call): Refine shortcut that calls
	cp_build_function_call_vec now that non-templates can
	appear in the candidate list for a template-id call.
	(add_candidates): Replace 'bad_fns' overload with 'bad_cands'
	candidate list.  When not considering a candidate, add it
	to the list as an ignored candidate.  Add all 'bad_cands'
	to the overload set as well.

gcc/testsuite/ChangeLog:

	* g++.dg/diagnostic/param-type-mismatch-2.C: Rename template
	function test_7 that accidentally (perhaps) shares the same
	name as its non-template callee.
	* g++.dg/overload/error6.C: New test.
---
 gcc/cp/call.cc                                | 149 +++++++++++++-----
 .../g++.dg/diagnostic/param-type-mismatch-2.C |  20 +--
 gcc/testsuite/g++.dg/overload/error6.C        |   9 ++
 3 files changed, 132 insertions(+), 46 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/overload/error6.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 89d422f7220..3212d5268e0 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -441,7 +441,8 @@ enum rejection_reason_code {
   rr_template_unification,
   rr_invalid_copy,
   rr_inherited_ctor,
-  rr_constraint_failure
+  rr_constraint_failure,
+  rr_ignored,
 };
 
 struct conversion_info {
@@ -2224,6 +2225,34 @@ add_candidate (struct z_candidate **candidates,
   return cand;
 }
 
+/* FN is a function from the overload set that we outright didn't even consider
+   (for some reason); add it to the list as an unviable "ignored" candidate.  */
+
+static z_candidate *
+add_ignored_candidate (z_candidate **candidates, tree fn)
+{
+  /* No need to dynamically allocate these.  */
+  static const rejection_reason reason_ignored = { rr_ignored, {} };
+
+  struct z_candidate *cand = (struct z_candidate *)
+    conversion_obstack_alloc (sizeof (struct z_candidate));
+
+  cand->fn = fn;
+  cand->reason = const_cast<rejection_reason *> (&reason_ignored);
+  cand->next = *candidates;
+  *candidates = cand;
+
+  return cand;
+}
+
+/* True iff CAND is a candidate added by add_ignored_candidate.  */
+
+static bool
+ignored_candidate_p (const z_candidate *cand)
+{
+  return cand->reason && cand->reason->code == rr_ignored;
+}
+
 /* Return the number of remaining arguments in the parameter list
    beginning with ARG.  */
 
@@ -3471,7 +3500,7 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl,
     }
 
   if (len < skip_without_in_chrg)
-    return NULL;
+    return add_ignored_candidate (candidates, tmpl);
 
   if (DECL_CONSTRUCTOR_P (tmpl) && nargs == 2
       && same_type_ignoring_top_level_qualifiers_p (TREE_TYPE (first_arg),
@@ -3609,7 +3638,7 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl,
   if (((flags & (LOOKUP_ONLYCONVERTING|LOOKUP_LIST_INIT_CTOR))
        == LOOKUP_ONLYCONVERTING)
       && DECL_NONCONVERTING_P (fn))
-    return NULL;
+    return add_ignored_candidate (candidates, fn);
 
   if (DECL_CONSTRUCTOR_P (fn) && nargs == 2)
     {
@@ -3724,6 +3753,9 @@ splice_viable (struct z_candidate *cands,
   z_candidate *unviable = nullptr;
   z_candidate **unviable_tail = &unviable;
 
+  z_candidate *unviable_ignored = nullptr;
+  z_candidate **unviable_ignored_tail = &unviable_ignored;
+
   /* Be strict inside templates, since build_over_call won't actually
      do the conversions to get pedwarns.  */
   if (processing_template_decl)
@@ -3742,6 +3774,7 @@ splice_viable (struct z_candidate *cands,
 	 its viability.  */
       auto& tail = (cand->viable == 1 ? strictly_viable_tail
 		    : cand->viable == -1 ? non_strictly_viable_tail
+		    : ignored_candidate_p (cand) ? unviable_ignored_tail
 		    : unviable_tail);
       *tail = cand;
       tail = &cand->next;
@@ -3751,7 +3784,8 @@ splice_viable (struct z_candidate *cands,
 		   || (!strict_p && non_strictly_viable != nullptr));
 
   /* Combine the lists.  */
-  *unviable_tail = nullptr;
+  *unviable_ignored_tail = nullptr;
+  *unviable_tail = unviable_ignored;
   *non_strictly_viable_tail = unviable;
   *strictly_viable_tail = non_strictly_viable;
 
@@ -3901,6 +3935,8 @@ print_z_candidate (location_t loc, const char *msgstr,
     inform (cloc, "%s%qT (conversion)", msg, fn);
   else if (candidate->viable == -1)
     inform (cloc, "%s%#qD (near match)", msg, fn);
+  else if (ignored_candidate_p (candidate))
+    inform (cloc, "%s%#qD (ignored)", msg, fn);
   else if (DECL_DELETED_FN (fn))
     inform (cloc, "%s%#qD (deleted)", msg, fn);
   else if (candidate->reversed ())
@@ -3980,6 +4016,8 @@ print_z_candidate (location_t loc, const char *msgstr,
 		  "initialization from an expression of the same or derived "
 		  "type");
 	  break;
+	case rr_ignored:
+	  break;
 	case rr_none:
 	default:
 	  /* This candidate didn't have any issues or we failed to
@@ -5023,7 +5061,12 @@ build_new_function_call (tree fn, vec<tree, va_gc> **args,
 	  // If there is a single (non-viable) function candidate,
 	  // let the error be diagnosed by cp_build_function_call_vec.
 	  if (!any_viable_p && candidates && ! candidates->next
-	      && (TREE_CODE (candidates->fn) == FUNCTION_DECL))
+	      && TREE_CODE (candidates->fn) == FUNCTION_DECL
+	      /* A template-id callee consisting of a single (ignored)
+		 non-template candidate needs to be diagnosed the
+		 ordinary way.  */
+	      && (TREE_CODE (fn) != TEMPLATE_ID_EXPR
+		  || candidates->template_decl))
 	    return cp_build_function_call_vec (candidates->fn, args, complain);
 
 	  // Otherwise, emit notes for non-viable candidates.
@@ -6509,6 +6552,10 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
   else /*if (flags & LOOKUP_DEFAULTED)*/
     which = non_templates;
 
+  /* Template candidates that we'll potentially ignore if the
+     perfect candidate optimization succeeds.  */
+  z_candidate *ignored_template_cands = nullptr;
+
   /* 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
@@ -6519,20 +6566,29 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
      This trick is important for pruning 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;
+  z_candidate *bad_cands = nullptr;
   bool shortcut_bad_convs = true;
 
  again:
   for (tree fn : lkp_range (fns))
     {
-      if (check_converting && DECL_NONCONVERTING_P (fn))
-	continue;
-      if (check_list_ctor && !is_list_ctor (fn))
-	continue;
       if (which == templates && TREE_CODE (fn) != TEMPLATE_DECL)
-	continue;
+	{
+	  if (template_only)
+	    add_ignored_candidate (candidates, fn);
+	  continue;
+	}
       if (which == non_templates && TREE_CODE (fn) == TEMPLATE_DECL)
-	continue;
+	{
+	  add_ignored_candidate (&ignored_template_cands, fn);
+	  continue;
+	}
+      if ((check_converting && DECL_NONCONVERTING_P (fn))
+	  || (check_list_ctor && !is_list_ctor (fn)))
+	{
+	  add_ignored_candidate (candidates, fn);
+	  continue;
+	}
 
       tree fn_first_arg = NULL_TREE;
       const vec<tree, va_gc> *fn_args = args;
@@ -6589,22 +6645,19 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
 	}
 
       if (TREE_CODE (fn) == TEMPLATE_DECL)
-	{
-	  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;
-	}
+	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);
       else
 	{
 	  add_function_candidate (candidates,
@@ -6632,13 +6685,14 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
 	{
 	  /* 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 don't find any strictly viable
-	     candidates.  */
+	     stopped at the first bad conversion).  Move the candidate to
+	     BAD_CANDS to fully reconsider later if we don't find any strictly
+	     viable candidates.  */
 	  if (complain & (tf_error | tf_conv))
 	    {
-	      bad_fns = lookup_add (fn, bad_fns);
-	      *candidates = (*candidates)->next;
+	      *candidates = cand->next;
+	      cand->next = bad_cands;
+	      bad_cands = cand;
 	    }
 	  else
 	    /* But if we're in a SFINAE context, just mark this candidate as
@@ -6652,21 +6706,44 @@ add_candidates (tree fns, tree first_arg, const vec<tree, va_gc> *args,
   if (which == non_templates && !seen_perfect)
     {
       which = templates;
+      ignored_template_cands = nullptr;
       goto again;
     }
   else if (which == templates
 	   && !seen_strictly_viable
 	   && shortcut_bad_convs
-	   && bad_fns)
+	   && bad_cands)
     {
       /* None of the candidates are strictly viable, so consider again those
-	 functions in BAD_FNS, this time without shortcutting bad conversions
+	 functions in BAD_CANDS, this time without shortcutting bad conversions
 	 so that all their argument conversions are computed.  */
       which = either;
-      fns = bad_fns;
+      fns = NULL_TREE;
+      for (z_candidate *cand = bad_cands; cand; cand = cand->next)
+	{
+	  tree fn = cand->fn;
+	  if (cand->template_decl)
+	    fn = TI_TEMPLATE (cand->template_decl);
+	  fns = ovl_make (fn, fns);
+	}
       shortcut_bad_convs = false;
+      bad_cands = nullptr;
       goto again;
     }
+
+  if (complain & tf_error)
+    {
+      /* Remember any omitted candidates if we need to print candidates
+	 as part of overload resolution failure diagnostics.  */
+      for (z_candidate *omitted_cands : { ignored_template_cands, bad_cands })
+	{
+	  z_candidate **omitted_cands_tail = &omitted_cands;
+	  while (*omitted_cands_tail)
+	    omitted_cands_tail = &(*omitted_cands_tail)->next;
+	  *omitted_cands_tail = *candidates;
+	  *candidates = omitted_cands;
+	}
+    }
 }
 
 /* Returns 1 if P0145R2 says that the LHS of operator CODE is evaluated first,
diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
index de7570a6efa..50c25cd49b7 100644
--- a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
+++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
@@ -129,22 +129,22 @@ int test_6 (int first, const char *second, float third, s6 *ptr)
 /* Template function.  */
 
 template <typename T>
-int test_7 (int one, T two, float three); // { dg-line test_7_decl }
+int callee_7 (int one, T two, float three); // { dg-line callee_7_decl }
 
 int test_7 (int first, const char *second, float third)
 {
-  return test_7 <const char **> (first, second, third); // { dg-line test_7_usage }
-  // { dg-message "cannot convert 'const char\\*' to 'const char\\*\\*'" "" { target *-*-* } test_7_usage }
+  return callee_7 <const char **> (first, second, third); // { dg-line callee_7_usage }
+  // { dg-message "cannot convert 'const char\\*' to 'const char\\*\\*'" "" { target *-*-* } callee_7_usage }
   /* { dg-begin-multiline-output "" }
-   return test_7 <const char **> (first, second, third);
-                                         ^~~~~~
-                                         |
-                                         const char*
+   return callee_7 <const char **> (first, second, third);
+                                           ^~~~~~
+                                           |
+                                           const char*
      { dg-end-multiline-output "" } */
-  // { dg-message "initializing argument 2 of 'int test_7\\(int, T, float\\) .with T = const char\\*\\*.'" "" { target *-*-* } test_7_decl }
+  // { dg-message "initializing argument 2 of 'int callee_7\\(int, T, float\\) .with T = const char\\*\\*.'" "" { target *-*-* } callee_7_decl }
   /* { dg-begin-multiline-output "" }
- int test_7 (int one, T two, float three);
-                      ~~^~~
+ int callee_7 (int one, T two, float three);
+                        ~~^~~
      { dg-end-multiline-output "" } */
 }
 
diff --git a/gcc/testsuite/g++.dg/overload/error6.C b/gcc/testsuite/g++.dg/overload/error6.C
new file mode 100644
index 00000000000..86a12eaa8de
--- /dev/null
+++ b/gcc/testsuite/g++.dg/overload/error6.C
@@ -0,0 +1,9 @@
+// Verify we note even non-template candidates when diagnosing
+// overload resolution failure for a template-id.
+
+template<class T> void f(T); // { dg-message "candidate" }
+void f(int); // { dg-message {candidate: 'void f\(int\)' \(ignored\)} }
+
+int main() {
+  f<int>(0, 0); // { dg-error "no match" }
+}
-- 
2.42.0.424.gceadf0f3cf


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

* [PATCH v2 3/3] c++: note other candidates when diagnosing deletedness
  2023-10-23 23:51 [PATCH v2 1/3] c++: sort candidates according to viability Patrick Palka
  2023-10-23 23:51 ` [PATCH v2 2/3] c++: remember candidates that we ignored Patrick Palka
@ 2023-10-23 23:51 ` Patrick Palka
  2023-10-24 21:56   ` Jason Merrill
  2023-10-24 21:28 ` [PATCH v2 1/3] c++: sort candidates according to viability Jason Merrill
  2 siblings, 1 reply; 6+ messages in thread
From: Patrick Palka @ 2023-10-23 23:51 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason, Patrick Palka

With the previous two patches in place, we can now extend our
deletedness diagnostic to note the other considered candidates, e.g.:

  deleted16.C: In function 'int main()':
  deleted16.C:10:4: error: use of deleted function 'void f(int)'
     10 |   f(0);
        |   ~^~~
  deleted16.C:5:6: note: declared here
      5 | void f(int) = delete;
        |      ^
  deleted16.C:5:6: note: candidate: 'void f(int)' (deleted)
  deleted16.C:6:6: note: candidate: 'void f(...)'
      6 | void f(...);
        |      ^
  deleted16.C:7:6: note: candidate: 'void f(int, int)'
      7 | void f(int, int);
        |      ^
  deleted16.C:7:6: note:   candidate expects 2 arguments, 1 provided

For now, these these notes are disabled when a deleted special member
function is selected because it introduces a lot of new "cannot bind
reference" errors in the testsuite when noting non-viable candidates,
e.g. in cpp0x/initlist-opt1.C we would need to expect an error when
noting unviability of A(A&&).  (It'd be nice if we could downgrade such
errors into notes when noting candidates...)

gcc/cp/ChangeLog:

	* call.cc (build_over_call): Call print_z_candidates when
	diagnosing deletedness.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/deleted16.C: New test.
---
 gcc/cp/call.cc                         | 10 +++++++++-
 gcc/testsuite/g++.dg/cpp0x/deleted16.C | 24 ++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/deleted16.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 3212d5268e0..1313d6516bd 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9932,7 +9932,15 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
   if (DECL_DELETED_FN (fn))
     {
       if (complain & tf_error)
-	mark_used (fn);
+	{
+	  mark_used (fn);
+	  /* Note the other candidates we considered unless we selected a
+	     special member function since the mismatch reasons for other
+	     candidates are usually uninteresting, e.g. rvalue vs lvalue
+	     reference binding .  */
+	  if (cand->next && !special_memfn_p (fn))
+	    print_z_candidates (input_location, cand, /*only_viable_p=*/false);
+	}
       return error_mark_node;
     }
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/deleted16.C b/gcc/testsuite/g++.dg/cpp0x/deleted16.C
new file mode 100644
index 00000000000..55acbfd9188
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/deleted16.C
@@ -0,0 +1,24 @@
+// Verify we note other candidates when a deleted function is
+// selected by overload resolution.
+// { dg-do compile { target c++11 } }
+
+void f(int) = delete; // { dg-message "declared here|candidate" }
+void f(...); // { dg-message "candidate" }
+void f(int, int); // { dg-message "candidate" }
+
+// An example where the perfect candidate optimization causes us
+// to ignore function templates.
+void g(int) = delete; // { dg-message "declared here|candidate" }
+template<class T> void g(T); // { dg-message "candidate" }
+
+// An example where we have a strictly viable candidate and
+// an incompletely considered bad candidate.
+template<class T> void h(T, T) = delete; // { dg-message "declared here|candidate" }
+void h(int*, int) = delete; // { dg-message "candidate" }
+
+int main() {
+  f(0); // { dg-error "deleted" }
+  g(0); // { dg-error "deleted" }
+  h(1, 1); // { dg-error "deleted" }
+           // { dg-error "invalid conversion" "" { target *-*-* } .-1 } when noting 2nd cand
+}
-- 
2.42.0.424.gceadf0f3cf


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

* Re: [PATCH v2 1/3] c++: sort candidates according to viability
  2023-10-23 23:51 [PATCH v2 1/3] c++: sort candidates according to viability Patrick Palka
  2023-10-23 23:51 ` [PATCH v2 2/3] c++: remember candidates that we ignored Patrick Palka
  2023-10-23 23:51 ` [PATCH v2 3/3] c++: note other candidates when diagnosing deletedness Patrick Palka
@ 2023-10-24 21:28 ` Jason Merrill
  2 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2023-10-24 21:28 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

On 10/23/23 19:51, Patrick Palka wrote:
> The second patch in this series is new and ensures that the candidates
> list isn't mysteriously missing some candidates when noting other
> candidates due to deletedness.
> 
> -- >8 --
> 
> This patch:
> 
>    * changes splice_viable to move the non-viable candidates to the end
>      of the list instead of removing them outright

Please consistently spell this "non-viable" rather than "unviable".

> +tourney (struct z_candidate *&candidates, tsubst_flags_t complain)
>   {
>     struct z_candidate *champ = candidates, *challenger;
>     int fate;
>     struct z_candidate *champ_compared_to_predecessor = nullptr;
> +  struct z_candidate *champ_predecessor = nullptr;
> +  struct z_candidate *challenger_predecessor = champ;

Rather than adding two more variables to keep synchronized, maybe we 
want champ and challenger to be **, like the tail variables in 
splice_viable?

Jason


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

* Re: [PATCH v2 2/3] c++: remember candidates that we ignored
  2023-10-23 23:51 ` [PATCH v2 2/3] c++: remember candidates that we ignored Patrick Palka
@ 2023-10-24 21:51   ` Jason Merrill
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2023-10-24 21:51 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

On 10/23/23 19:51, Patrick Palka wrote:
> During overload resolution, we sometimes outright ignore a function from
> the overload set and leave no trace of it in the candidates list, for
> example when we find a perfect non-template candidate we discard all
> function templates, or when the callee is a template-id we discard all
> non-template functions.  We should still however make note of these
> unviable functions when diagnosing overload resolution failure, but
> that's not possible if they're not present in the returned candidates
> list.
> 
> To that end, this patch reworks add_candidates to add such ignored
> functions to the list.  The new rr_ignored rejection reason is somewhat
> of a catch-all; we could perhaps split it up into more specific rejection
> reasons, but I leave that as future work.

OK with the same unviable -> non-viable change as the 1/3 patch.

Jason


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

* Re: [PATCH v2 3/3] c++: note other candidates when diagnosing deletedness
  2023-10-23 23:51 ` [PATCH v2 3/3] c++: note other candidates when diagnosing deletedness Patrick Palka
@ 2023-10-24 21:56   ` Jason Merrill
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2023-10-24 21:56 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

On 10/23/23 19:51, Patrick Palka wrote:
> With the previous two patches in place, we can now extend our
> deletedness diagnostic to note the other considered candidates, e.g.:
> 
>    deleted16.C: In function 'int main()':
>    deleted16.C:10:4: error: use of deleted function 'void f(int)'
>       10 |   f(0);
>          |   ~^~~
>    deleted16.C:5:6: note: declared here
>        5 | void f(int) = delete;
>          |      ^
>    deleted16.C:5:6: note: candidate: 'void f(int)' (deleted)
>    deleted16.C:6:6: note: candidate: 'void f(...)'
>        6 | void f(...);
>          |      ^
>    deleted16.C:7:6: note: candidate: 'void f(int, int)'
>        7 | void f(int, int);
>          |      ^
>    deleted16.C:7:6: note:   candidate expects 2 arguments, 1 provided
> 
> For now, these these notes are disabled when a deleted special member
> function is selected because it introduces a lot of new "cannot bind
> reference" errors in the testsuite when noting non-viable candidates,
> e.g. in cpp0x/initlist-opt1.C we would need to expect an error when
> noting unviability of A(A&&).  (It'd be nice if we could downgrade such
> errors into notes when noting candidates...)

What about my suggestion to make printing the other candidates an 
opt-in, with the normal output just suggesting the use of that option 
for more information, like -ftemplate-backtrace-limit?

Jason


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

end of thread, other threads:[~2023-10-24 21:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-23 23:51 [PATCH v2 1/3] c++: sort candidates according to viability Patrick Palka
2023-10-23 23:51 ` [PATCH v2 2/3] c++: remember candidates that we ignored Patrick Palka
2023-10-24 21:51   ` Jason Merrill
2023-10-23 23:51 ` [PATCH v2 3/3] c++: note other candidates when diagnosing deletedness Patrick Palka
2023-10-24 21:56   ` Jason Merrill
2023-10-24 21:28 ` [PATCH v2 1/3] c++: sort candidates according to viability 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).