From ac03c8d8fd6cf7f9080068589683cb06531879c2 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tromey@adacore.com>
Date: Fri, 18 Feb 2022 14:03:03 -0700
Subject: [PATCH] Fix bug in C++ overload resolution

PR c++/28901 points out a bug in C++ overload resolution.  When
comparing two overloads, one might be better than the other for
certain parameters -- but, if that one also has some invalid
conversion, then it should never be considered the better choice.
Instead, a valid-but-not-apparently-quite-as-good overload should be
preferred.

This patch fixes this problem by changing how overload comparisons are
done.  I don't believe it should affect any currently valid overload
resolution; nor should it affect resolutions where all the choices are
equally invalid.
---
 gdb/gdbtypes.c                    | 43 ++++++++++++++++++++++++-------
 gdb/testsuite/gdb.cp/overload.cc  | 10 +++++++
 gdb/testsuite/gdb.cp/overload.exp |  3 +++
 3 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index a2dbe7551b5..f41d6bd960e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -44,12 +44,15 @@
 #include <algorithm>
 #include "gmp-utils.h"
 
+/* The value of an invalid conversion badness.  */
+#define INVALID_CONVERSION 100
+
 /* Initialize BADNESS constants.  */
 
-const struct rank LENGTH_MISMATCH_BADNESS = {100,0};
+const struct rank LENGTH_MISMATCH_BADNESS = {INVALID_CONVERSION,0};
 
-const struct rank TOO_FEW_PARAMS_BADNESS = {100,0};
-const struct rank INCOMPATIBLE_TYPE_BADNESS = {100,0};
+const struct rank TOO_FEW_PARAMS_BADNESS = {INVALID_CONVERSION,0};
+const struct rank INCOMPATIBLE_TYPE_BADNESS = {INVALID_CONVERSION,0};
 
 const struct rank EXACT_MATCH_BADNESS = {0,0};
 
@@ -3966,8 +3969,14 @@ compare_badness (const badness_vector &a, const badness_vector &b)
 {
   int i;
   int tmp;
-  short found_pos = 0;		/* any positives in c? */
-  short found_neg = 0;		/* any negatives in c? */
+  /* Any positives in comparison? */
+  bool found_pos = false;
+  /* Any negatives in comparison? */
+  bool found_neg = false;
+  /* Did A have any INVALID_CONVERSION entries.  */
+  bool a_invalid = false;
+  /* Did B have any INVALID_CONVERSION entries.  */
+  bool b_invalid = false;
 
   /* differing sizes => incomparable */
   if (a.size () != b.size ())
@@ -3978,12 +3987,27 @@ compare_badness (const badness_vector &a, const badness_vector &b)
     {
       tmp = compare_ranks (b[i], a[i]);
       if (tmp > 0)
-	found_pos = 1;
+	found_pos = true;
       else if (tmp < 0)
-	found_neg = 1;
+	found_neg = true;
+      if (a[i].rank >= INVALID_CONVERSION)
+	a_invalid = true;
+      if (b[i].rank >= INVALID_CONVERSION)
+	b_invalid = true;
     }
 
-  if (found_pos)
+  /* B will only be considered better than or incomparable to A if
+     they both have invalid entries, or if neither does.  That is, if
+     A has only valid entries, and B has an invalid entry, then A will
+     be considered better than B, even if B happens to be better for
+     some parameter.  */
+  if (a_invalid != b_invalid)
+    {
+      if (a_invalid)
+	return 3;		/* A > B */
+      return 2;			/* A < B */
+    }
+  else if (found_pos)
     {
       if (found_neg)
 	return 1;		/* incomparable */
@@ -4742,7 +4766,8 @@ rank_one_type_parm_set (struct type *parm, struct type *arg, struct value *value
  * Return 0 if they are identical types;
  * Otherwise, return an integer which corresponds to how compatible
  * PARM is to ARG.  The higher the return value, the worse the match.
- * Generally the "bad" conversions are all uniformly assigned a 100.  */
+ * Generally the "bad" conversions are all uniformly assigned
+ * INVALID_CONVERSION.  */
 
 struct rank
 rank_one_type (struct type *parm, struct type *arg, struct value *value)
diff --git a/gdb/testsuite/gdb.cp/overload.cc b/gdb/testsuite/gdb.cp/overload.cc
index 5c782a46104..ab015721b2b 100644
--- a/gdb/testsuite/gdb.cp/overload.cc
+++ b/gdb/testsuite/gdb.cp/overload.cc
@@ -93,10 +93,15 @@ class A {};
 class B: public A {};
 class C: public B {};
 class D: C {};
+class E {};
+class F {};
 
 int bar (A) { return 11; }
 int bar (B) { return 22; }
 
+int bar2 (E &, A &) { return 33; }
+int bar2 (F &, B &) { return 44; }
+
 int intintfunc (int x) { return x; }
 
 int main () 
@@ -119,11 +124,16 @@ int main ()
     B b;
     C c;
     D d;
+    E e;
+    F f;
 
     bar (a);
     bar (b);
     bar (c);
 
+    bar2 (e, b);
+    bar2 (f, b);
+
     char *str = (char *) "A";
     foo foo_instance1(111);
     foo foo_instance2(222, str);
diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
index 56cd5ac2106..73ca0d2d55c 100644
--- a/gdb/testsuite/gdb.cp/overload.exp
+++ b/gdb/testsuite/gdb.cp/overload.exp
@@ -259,6 +259,9 @@ gdb_test "print bar(b)" "= 22"
 gdb_test "print bar(c)" "= 22"
 gdb_test "print bar(d)" "= 22"
 
+# PR c++/28901 - gdb thought this was ambiguous.
+gdb_test "print bar2(e, b)" " = 33"
+
 # ---
 
 # List overloaded functions.
-- 
2.30.2