Re: [PATCH v2 1/3] c++: sort candidates according to viability

2023-10-24 Thread Jason Merrill

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 *, 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



[PATCH v2 1/3] c++: sort candidates according to viability

2023-10-23 Thread 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 = _viable;
+
+  z_candidate *non_strictly_viable = nullptr;
+  z_candidate **non_strictly_viable_tail = _strictly_viable;
+
+  z_candidate *unviable = nullptr;
+  z_candidate **unviable_tail = 
 
   /* 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 = 
-  *any_viable_p = false;