[patch] Fix overload resolution of int* vs void*

[patch] Fix overload resolution of int* vs void*

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

A fix for this bug http://sourceware.org/bugzilla/show_bug.cgi?id=10343

This patch makes it a little bit cheaper to convert a pointer to void* 
than any other pointer conversion.

Sami

[-- Attachment #2: overload_voidp.patch --]
[-- Type: text/x-patch, Size: 4525 bytes --]

Fixed void* vs int* overload issue (PR C++/10343).

2010-08-30  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdbtypes.h: Made VOID_PTR_CONVERSION_BADNESS better than
	INTEGER_PROMOTION_BADNESS.

2010-08-30  Sami Wagiaalla  <swagiaal@redhat.com>

	PR C++/10343
	* gdb.cp/overload.cc: Added test for void* vs int* overload.
	* gdb.cp/overload.exp: Ditto.

diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 6fc47f2..ed8d613 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1374,25 +1374,25 @@ extern int is_unique_ancestor (struct type *, struct value *);
 #define INCOMPATIBLE_TYPE_BADNESS    100
 
 /* Badness of integral promotion */
-#define INTEGER_PROMOTION_BADNESS      1
+#define INTEGER_PROMOTION_BADNESS      2
 /* Badness of floating promotion */
-#define FLOAT_PROMOTION_BADNESS        1
+#define FLOAT_PROMOTION_BADNESS        2
 /* Badness of integral conversion */
-#define INTEGER_CONVERSION_BADNESS     2
+#define INTEGER_CONVERSION_BADNESS     3
 /* Badness of floating conversion */
-#define FLOAT_CONVERSION_BADNESS       2
+#define FLOAT_CONVERSION_BADNESS       3
 /* Badness of integer<->floating conversions */
-#define INT_FLOAT_CONVERSION_BADNESS   2
+#define INT_FLOAT_CONVERSION_BADNESS   3
 /* Badness of converting to a boolean */
-#define BOOLEAN_CONVERSION_BADNESS     2
+#define BOOLEAN_CONVERSION_BADNESS     3
 /* Badness of pointer conversion */
-#define POINTER_CONVERSION_BADNESS     2
+#define POINTER_CONVERSION_BADNESS     3
 /* Badness of conversion of pointer to void pointer */
-#define VOID_PTR_CONVERSION_BADNESS    2
+#define VOID_PTR_CONVERSION_BADNESS    1
 /* Badness of converting derived to base class */
-#define BASE_CONVERSION_BADNESS        2
+#define BASE_CONVERSION_BADNESS        3
 /* Badness of converting from non-reference to reference */
-#define REFERENCE_CONVERSION_BADNESS   2
+#define REFERENCE_CONVERSION_BADNESS   3
 
 /* Non-standard conversions allowed by the debugger */
 /* Converting a pointer to an int is usually OK */
diff --git a/gdb/testsuite/gdb.cp/overload.cc b/gdb/testsuite/gdb.cp/overload.cc
index 78fae14..dc117fb 100644
--- a/gdb/testsuite/gdb.cp/overload.cc
+++ b/gdb/testsuite/gdb.cp/overload.cc
@@ -24,6 +24,9 @@ int overload1arg (unsigned long);
 int overload1arg (float);
 int overload1arg (double);
 
+int overload1arg (int*);
+int overload1arg (void*);
+
 int overloadfnarg (void);
 int overloadfnarg (int);
 int overloadfnarg (int, int (*) (int));
@@ -99,6 +102,8 @@ int main ()
     unsigned long arg10 =10;
     float arg11 =100.0;
     double arg12 = 200.0;
+    int arg13 = 200.0;
+    char arg14 = 'a';
 
     char *str = (char *) "A";
     foo foo_instance1(111);
@@ -150,6 +155,8 @@ int foo::overload1arg (long arg)            { arg = 0; return 9;}
 int foo::overload1arg (unsigned long arg)   { arg = 0; return 10;}
 int foo::overload1arg (float arg)           { arg = 0; return 11;}
 int foo::overload1arg (double arg)          { arg = 0; return 12;}
+int foo::overload1arg (int* arg)            { arg = 0; return 13;}
+int foo::overload1arg (void* arg)           { arg = 0; return 14;}
 
 /* Test to see that we can explicitly request overloaded functions
    with function pointers in the prototype. */
diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
index f05cc23..25aeb07 100644
--- a/gdb/testsuite/gdb.cp/overload.exp
+++ b/gdb/testsuite/gdb.cp/overload.exp
@@ -80,6 +80,8 @@ set re_methods	"${re_methods}${ws}int overload1arg\\(long( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\((unsigned long|long unsigned)( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(float\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(double\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(int \\*\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(void \\*\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\((void|)\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int, int ?\\(\\*\\) ?\\(int\\)\\);"
@@ -256,6 +258,14 @@ gdb_test "print foo_instance1.overload1arg((double)arg12)" \
     "\\$\[0-9\]+ = 12" \
     "print call overloaded func double arg"
 
+gdb_test "print foo_instance1.overload1arg(&arg13)" \
+    "\\$\[0-9\]+ = 13" \
+    "print call overloaded func int\\* arg"
+
+gdb_test "print foo_instance1.overload1arg(&arg14)" \
+    "\\$\[0-9\]+ = 14" \
+    "print call overloaded func char\\* arg"
+
 # ---
 
 # List overloaded functions.

Re: [patch] Fix overload resolution of int* vs void*

[sami wagiaalla]

On 08/30/2010 11:24 AM, sami wagiaalla wrote:
> A fix for this bug http://sourceware.org/bugzilla/show_bug.cgi?id=10343
>
> This patch makes it a little bit cheaper to convert a pointer to void*
> than any other pointer conversion.
>

This patch was regression tested on Fedora 13 on x8664 with gcc 4.4.4. 
No regressions.

Re: [patch] Fix overload resolution of int* vs void*

[Tom Tromey]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> A fix for this bug http://sourceware.org/bugzilla/show_bug.cgi?id=10343
Sami> This patch makes it a little bit cheaper to convert a pointer to void*
Sami> than any other pointer conversion.

Consider this test:

    struct B { };
    struct D : public B { };
    void f (void *x) { }
    void f (B *x) { }
    void g(D *x) { f(x); }

This should call f(B*), but with your patch I suspect it will call
f(void*).

Tom

Re: [patch 1/2] Fix overload resolution of int* vs void*

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 275 bytes --]

is_ancestor and is_public_ancestor have a lot of redundant code. This 
patch shares the code between that two. This way improvements the the 
function can be shared between the two functions. I don't like the name 
do_is_ancestor but I could not come up with anything better

[-- Attachment #2: is_ancestor_cleanup.patch --]
[-- Type: text/x-patch, Size: 2467 bytes --]

Eliminate 'is_ancestor' redundant code.

2010-10-08  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdbtypes.c (do_is_ancestor): New function.
	(is_ancestor): Use do_is_ancestor.
	(is_public_ancestor): Use do_is_ancestor.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index c35adbb..e2a8a62 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1871,13 +1871,12 @@ class_types_same_p (const struct type *a, const struct type *b)
 }
 
 /* Check whether BASE is an ancestor or base class or DCLASS 
-   Return 1 if so, and 0 if not.
-   Note: callers may want to check for identity of the types before
-   calling this function -- identical types are considered to satisfy
-   the ancestor relationship even if they're identical.  */
+   Return 1 if so, and 0 if not.  If PUBLIC is 1 then only public
+   ancestors ancestors are considered, and the function returns 1 only
+   if BASE is a public ancestor of DCLASS.  */
 
-int
-is_ancestor (struct type *base, struct type *dclass)
+static int
+do_is_ancestor (struct type *base, struct type *dclass, int public)
 {
   int i;
 
@@ -1889,6 +1888,9 @@ is_ancestor (struct type *base, struct type *dclass)
 
   for (i = 0; i < TYPE_N_BASECLASSES (dclass); i++)
     {
+      if (public && ! BASETYPE_VIA_PUBLIC (dclass, i))
+	continue;
+
       if (is_ancestor (base, TYPE_BASECLASS (dclass, i)))
 	return 1;
     }
@@ -1896,29 +1898,25 @@ is_ancestor (struct type *base, struct type *dclass)
   return 0;
 }
 
+/* Check whether BASE is an ancestor or base class or DCLASS
+   Return 1 if so, and 0 if not.
+   Note: callers may want to check for identity of the types before
+   calling this function -- identical types are considered to satisfy
+   the ancestor relationship even if they're identical.  */
+
+int
+is_ancestor (struct type *base, struct type *dclass)
+{
+  return do_is_ancestor (base, dclass, 0);
+}
+
 /* Like is_ancestor, but only returns true when BASE is a public
    ancestor of DCLASS.  */
 
 int
 is_public_ancestor (struct type *base, struct type *dclass)
 {
-  int i;
-
-  CHECK_TYPEDEF (base);
-  CHECK_TYPEDEF (dclass);
-
-  if (class_types_same_p (base, dclass))
-    return 1;
-
-  for (i = 0; i < TYPE_N_BASECLASSES (dclass); ++i)
-    {
-      if (! BASETYPE_VIA_PUBLIC (dclass, i))
-	continue;
-      if (is_public_ancestor (base, TYPE_BASECLASS (dclass, i)))
-	return 1;
-    }
-
-  return 0;
+  return do_is_ancestor (base, dclass, 1);
 }
 
 /* A helper function for is_unique_ancestor.  */

Re: [patch 1/2] Fix overload resolution of int* vs void*

[Tom Tromey]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> +   Return 1 if so, and 0 if not.  If PUBLIC is 1 then only public
Sami> +   ancestors ancestors are considered, and the function returns 1 only

One of those "ancestors" is redundant.

Sami>        if (is_ancestor (base, TYPE_BASECLASS (dclass, i)))

This should call do_is_ancestor.

Sami> +/* Check whether BASE is an ancestor or base class or DCLASS

"of DCLASS".

Sami> +   Return 1 if so, and 0 if not.
Sami> +   Note: callers may want to check for identity of the types before
Sami> +   calling this function -- identical types are considered to satisfy
Sami> +   the ancestor relationship even if they're identical.  */

I know this is just copied text, but it isn't very clear.  If you want
to clean it up, I would appreciate it, but it isn't a requirement.

Tom

Re: [patch 2/2] Fix overload resolution of int* vs void*

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

> Sami>  A fix for this bug http://sourceware.org/bugzilla/show_bug.cgi?id=10343
> Sami>  This patch makes it a little bit cheaper to convert a pointer to void*
> Sami>  than any other pointer conversion.
>

This is a better patch. One problem with the rank function is that it 
assumed that the conversion 'badness' of converting an int* to a char* 
is the same as the badness of converting int to char, which is not 
correct. A conversion of int* to char* is just not allowed. This patch 
corrects that.

This patch also introduces BASE_PTR_CONVERSION_BADNESS. In the C++ spec 
this is not its own type of conversion. It is said to fall under pointer 
conversion and have the same rank, but the spec later specifies that a 
conversion of a pointer to a pointer to one of its bases is is preferred 
to a conversion of that pointer to a void* as you have specified in the 
following example. So, I decided to just make it its own rank.

> Consider this test:
>
>      struct B { };
>      struct D : public B { };
>      void f (void *x) { }
>      void f (B *x) { }
>      void g(D *x) { f(x); }
>
> This should call f(B*), but with your patch I suspect it will call
> f(void*).
>

This is correctly handled but the current patch, and tested by oranking.exp.

This patch series was regression tested on x8664 with gcc-4.4.4-f13

Thanks,
   Sami

[-- Attachment #2: overload_voidp.patch --]
[-- Type: text/x-patch, Size: 12083 bytes --]

Fixed void* vs int* overload issue (PR C++/10343).

2010-10-08  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdbtypes.h: Create BASE_PTR_CONVERSION_BADNESS.
	* gdbtypes.c (rank_one_type): Move type comparison code out of here
	to...
	(types_equal): ...here. And changed it as follows:
	Outside of typedefs type must be of the same TYPE_CODE.
	When compairing two pointers or references they are equal if their
	targets are equal.
	Correct pointer conversions.

2010-10-08  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdb.cp/converts.cc: New test program.
	* gdb.cp/converts.exp: New test.
	* gdb.cp/overload.exp: Added test for void* vs int*.
	* gdb.cp/overload.exp: Ditto.
	* gdb.cp/oranking.exp: Removed related kfail.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index e2a8a62..c6691a7 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1886,6 +1886,13 @@ do_is_ancestor (struct type *base, struct type *dclass, int public)
   if (class_types_same_p (base, dclass))
     return 1;
 
+  if ((TYPE_CODE (base) == TYPE_CODE_PTR
+       && TYPE_CODE (dclass) == TYPE_CODE_PTR)
+       || (TYPE_CODE (base) == TYPE_CODE_REF
+	   && TYPE_CODE (dclass) == TYPE_CODE_REF))
+    return is_ancestor (TYPE_TARGET_TYPE (base),
+                        TYPE_TARGET_TYPE (dclass));
+
   for (i = 0; i < TYPE_N_BASECLASSES (dclass); i++)
     {
       if (public && ! BASETYPE_VIA_PUBLIC (dclass, i))
@@ -2104,6 +2111,56 @@ integer_types_same_name_p (const char *first, const char *second)
   return 1;
 }
 
+/* Compares type A to type B returns 1 if the represent the same type
+   0 otherwise.  */
+
+static int
+types_equal (struct type *a, struct type *b)
+{
+  /* Identical type pointers.  */
+  /* However, this still doesn't catch all cases of same type for b
+     and a.  The reason is that builtin types are different from
+     the same ones constructed from the object.  */
+  if (a == b)
+    return 1;
+
+  /* Resolve typedefs */
+  if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
+    a = check_typedef (a);
+  if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
+    b = check_typedef (b);
+
+  /* If after resolving typedefs a and b are not of the same type
+     code then they are not equal.  */
+  if (TYPE_CODE (a) != TYPE_CODE (b))
+    return 0;
+
+  /* If a and b are both pointers types or both reference types then
+     they are equal of the same type iff the objects they refer to are
+     of the same type.  */
+  if (TYPE_CODE (a) == TYPE_CODE_PTR
+      || TYPE_CODE (a) == TYPE_CODE_REF)
+    return types_equal (TYPE_TARGET_TYPE (a),
+                        TYPE_TARGET_TYPE (b));
+
+  /*
+     Well, damnit, if the names are exactly the same, I'll say they
+     are exactly the same.  This happens when we generate method
+     stubs.  The types won't point to the same address, but they
+     really are the same.
+  */
+
+  if (TYPE_NAME (a) && TYPE_NAME (b)
+      && !strcmp (TYPE_NAME (a), TYPE_NAME (b)))
+    return 1;
+
+  /* Check if identical after resolving typedefs.  */
+  if (a == b)
+    return 1;
+
+  return 0;
+}
+
 /* Compare one type (PARM) for compatibility with another (ARG).
  * PARM is intended to be the parameter type of a function; and
  * ARG is the supplied argument's type.  This function tests if
@@ -2117,11 +2174,8 @@ integer_types_same_name_p (const char *first, const char *second)
 int
 rank_one_type (struct type *parm, struct type *arg)
 {
-  /* Identical type pointers.  */
-  /* However, this still doesn't catch all cases of same type for arg
-     and param.  The reason is that builtin types are different from
-     the same ones constructed from the object.  */
-  if (parm == arg)
+
+  if (types_equal (parm, arg))
     return 0;
 
   /* Resolve typedefs */
@@ -2130,21 +2184,6 @@ rank_one_type (struct type *parm, struct type *arg)
   if (TYPE_CODE (arg) == TYPE_CODE_TYPEDEF)
     arg = check_typedef (arg);
 
-  /*
-     Well, damnit, if the names are exactly the same, I'll say they
-     are exactly the same.  This happens when we generate method
-     stubs.  The types won't point to the same address, but they
-     really are the same.
-  */
-
-  if (TYPE_NAME (parm) && TYPE_NAME (arg) 
-      && !strcmp (TYPE_NAME (parm), TYPE_NAME (arg)))
-    return 0;
-
-  /* Check if identical after resolving typedefs.  */
-  if (parm == arg)
-    return 0;
-
   /* See through references, since we can almost make non-references
      references.  */
   if (TYPE_CODE (arg) == TYPE_CODE_REF)
@@ -2168,15 +2207,22 @@ rank_one_type (struct type *parm, struct type *arg)
       switch (TYPE_CODE (arg))
 	{
 	case TYPE_CODE_PTR:
-	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID
-	      && TYPE_CODE (TYPE_TARGET_TYPE (arg)) != TYPE_CODE_VOID)
+
+	  /* Allowed pointer conversions are:
+	     (a) pointer to void-pointer conversion.  */
+	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID)
 	    return VOID_PTR_CONVERSION_BADNESS;
-	  else
-	    return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				  TYPE_TARGET_TYPE (arg));
+
+	  /* (b) pointer to ancestor-pointer conversion.  */
+	  if (is_public_ancestor (parm, arg))
+	    return BASE_PTR_CONVERSION_BADNESS;
+
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_ARRAY:
-	  return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				TYPE_TARGET_TYPE (arg));
+	  if (types_equal (TYPE_TARGET_TYPE (parm),
+	                   TYPE_TARGET_TYPE (arg)))
+	    return 0;
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_FUNC:
 	  return rank_one_type (TYPE_TARGET_TYPE (parm), arg);
 	case TYPE_CODE_INT:
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index dba7284..62ade5f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1400,6 +1400,9 @@ extern int is_unique_ancestor (struct type *, struct value *);
 #define INTEGER_PROMOTION_BADNESS      1
 /* Badness of floating promotion */
 #define FLOAT_PROMOTION_BADNESS        1
+/* Badness of converting a derived class pointer
+   to a base class pointer.  */
+#define BASE_PTR_CONVERSION_BADNESS    1
 /* Badness of integral conversion */
 #define INTEGER_CONVERSION_BADNESS     2
 /* Badness of floating conversion */
diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
new file mode 100644
index 0000000..9fbe833
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.cc
@@ -0,0 +1,38 @@
+class A {};
+class B : public A {};
+
+int foo1_1 (char *) {return 11;}
+int foo1_2 (char[]) {return 12;}
+int foo1_3 (int*)   {return 13;}
+int foo1_4 (A*)     {return 14;}
+int foo1_5 (void*)  {return 15;}
+int foo1_6 (void**) {return 15;}
+
+int foo2_1 (char**  )  {return 21;}
+int foo2_2 (char[][1]) {return 22;}
+int foo2_3 (char *[])  {return 23;}
+int foo2_4 (int  *[])  {return 24;}
+
+int main()
+{
+
+  char *a;             // pointer to..
+  B *bp;
+  foo1_1 (a);          // ..pointer
+  foo1_2 (a);          // ..array
+  foo1_3 ((int*)a);    // ..pointer of wrong type
+  foo1_3 ((int*)bp);   // ..pointer of wrong type
+  foo1_4 (bp);         // ..ancestor pointer
+  foo1_4 (bp);         // ..ancestor pointer
+  foo1_5 (bp);         // ..void pointer
+  foo1_6 ((void**)bp); // ..void pointer
+
+  char **b;          // pointer pointer to..
+  char ba[1][1];
+  foo1_5 (b);        // ..void pointer
+  foo2_1 (b);        // ..pointer pointer
+  foo2_2 (ba);       // ..array of arrays
+  foo2_3 (b);        // ..array of pointers
+  foo2_4 ((int**)b); // ..array of wrong pointers
+  return 0;          // end of main
+}
diff --git a/gdb/testsuite/gdb.cp/converts.exp b/gdb/testsuite/gdb.cp/converts.exp
new file mode 100644
index 0000000..40d8fbd
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.exp
@@ -0,0 +1,44 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile converts
+set srcfile ${testfile}.cc
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+     return -1
+}
+
+############################################
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "end of main"]
+gdb_continue_to_breakpoint "end of main"
+
+
+gdb_test "p foo1_1 (a)"  "= 11"
+gdb_test "p foo1_2 (a)"  "= 12"
+gdb_test "p foo1_3 (a)"  "Cannot resolve function.*"
+gdb_test "p foo1_3 (bp)" "Cannot resolve function.*"
+gdb_test "p foo1_4 (bp)" "= 14"
+gdb_test "p foo1_5 (bp)" "= 15"
+
+gdb_test "p foo1_5 (b)" "= 15"
+gdb_test "p foo2_1 (b)" "= 21"
+gdb_test "p foo2_2 (b)" "Cannot resolve function.*"
+gdb_test "p foo2_3 (b)" "= 23"
+gdb_test "p foo2_4 (b)" "Cannot resolve function.*"
diff --git a/gdb/testsuite/gdb.cp/oranking.exp b/gdb/testsuite/gdb.cp/oranking.exp
index abe8252..f06933a 100644
--- a/gdb/testsuite/gdb.cp/oranking.exp
+++ b/gdb/testsuite/gdb.cp/oranking.exp
@@ -56,7 +56,6 @@ setup_kfail "gdb/12098" *-*-*
 gdb_test "p foo5(c)" "26"
 
 gdb_test "p test6()"  "28"
-setup_kfail "gdb/10343" *-*-*
 gdb_test "p foo6(bp)" "28"
 
 gdb_test "p test7()"  "210"
diff --git a/gdb/testsuite/gdb.cp/overload.cc b/gdb/testsuite/gdb.cp/overload.cc
index 78fae14..dc117fb 100644
--- a/gdb/testsuite/gdb.cp/overload.cc
+++ b/gdb/testsuite/gdb.cp/overload.cc
@@ -24,6 +24,9 @@ int overload1arg (unsigned long);
 int overload1arg (float);
 int overload1arg (double);
 
+int overload1arg (int*);
+int overload1arg (void*);
+
 int overloadfnarg (void);
 int overloadfnarg (int);
 int overloadfnarg (int, int (*) (int));
@@ -99,6 +102,8 @@ int main ()
     unsigned long arg10 =10;
     float arg11 =100.0;
     double arg12 = 200.0;
+    int arg13 = 200.0;
+    char arg14 = 'a';
 
     char *str = (char *) "A";
     foo foo_instance1(111);
@@ -150,6 +155,8 @@ int foo::overload1arg (long arg)            { arg = 0; return 9;}
 int foo::overload1arg (unsigned long arg)   { arg = 0; return 10;}
 int foo::overload1arg (float arg)           { arg = 0; return 11;}
 int foo::overload1arg (double arg)          { arg = 0; return 12;}
+int foo::overload1arg (int* arg)            { arg = 0; return 13;}
+int foo::overload1arg (void* arg)           { arg = 0; return 14;}
 
 /* Test to see that we can explicitly request overloaded functions
    with function pointers in the prototype. */
diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
index f05cc23..25aeb07 100644
--- a/gdb/testsuite/gdb.cp/overload.exp
+++ b/gdb/testsuite/gdb.cp/overload.exp
@@ -80,6 +80,8 @@ set re_methods	"${re_methods}${ws}int overload1arg\\(long( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\((unsigned long|long unsigned)( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(float\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(double\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(int \\*\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(void \\*\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\((void|)\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int, int ?\\(\\*\\) ?\\(int\\)\\);"
@@ -256,6 +258,14 @@ gdb_test "print foo_instance1.overload1arg((double)arg12)" \
     "\\$\[0-9\]+ = 12" \
     "print call overloaded func double arg"
 
+gdb_test "print foo_instance1.overload1arg(&arg13)" \
+    "\\$\[0-9\]+ = 13" \
+    "print call overloaded func int\\* arg"
+
+gdb_test "print foo_instance1.overload1arg(&arg14)" \
+    "\\$\[0-9\]+ = 14" \
+    "print call overloaded func char\\* arg"
+
 # ---
 
 # List overloaded functions.

Re: [patch 1/2] Fix overload resolution of int* vs void*

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 59 bytes --]

Thanks for the prompt review. A revised patch is attached.

[-- Attachment #2: is_ancestor_cleanup.patch --]
[-- Type: text/x-patch, Size: 2515 bytes --]

Eliminate 'is_ancestor' redundant code.

2010-10-08  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdbtypes.c (do_is_ancestor): New function.
	(is_ancestor): Use do_is_ancestor.
	(is_public_ancestor): Use do_is_ancestor.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index c35adbb..9c3152d 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1870,14 +1870,13 @@ class_types_same_p (const struct type *a, const struct type *b)
 	      && !strcmp (TYPE_NAME (a), TYPE_NAME (b))));
 }
 
-/* Check whether BASE is an ancestor or base class or DCLASS 
-   Return 1 if so, and 0 if not.
-   Note: callers may want to check for identity of the types before
-   calling this function -- identical types are considered to satisfy
-   the ancestor relationship even if they're identical.  */
+/* Check whether BASE is an ancestor or base class of DCLASS
+   Return 1 if so, and 0 if not.  If PUBLIC is 1 then only public
+   ancestors are considered, and the function returns 1 only if
+   BASE is a public ancestor of DCLASS.  */
 
-int
-is_ancestor (struct type *base, struct type *dclass)
+static int
+do_is_ancestor (struct type *base, struct type *dclass, int public)
 {
   int i;
 
@@ -1889,36 +1888,35 @@ is_ancestor (struct type *base, struct type *dclass)
 
   for (i = 0; i < TYPE_N_BASECLASSES (dclass); i++)
     {
-      if (is_ancestor (base, TYPE_BASECLASS (dclass, i)))
+      if (public && ! BASETYPE_VIA_PUBLIC (dclass, i))
+	continue;
+
+      if (do_is_ancestor (base, TYPE_BASECLASS (dclass, i), public))
 	return 1;
     }
 
   return 0;
 }
 
+/* Check whether BASE is an ancestor or base class or DCLASS
+   Return 1 if so, and 0 if not.
+   Note: If BASE and DCLASS are of the same type, this function
+   will return 1. So for some class A, is_ancestor (A, A) will
+   return 1.  */
+
+int
+is_ancestor (struct type *base, struct type *dclass)
+{
+  return do_is_ancestor (base, dclass, 0);
+}
+
 /* Like is_ancestor, but only returns true when BASE is a public
    ancestor of DCLASS.  */
 
 int
 is_public_ancestor (struct type *base, struct type *dclass)
 {
-  int i;
-
-  CHECK_TYPEDEF (base);
-  CHECK_TYPEDEF (dclass);
-
-  if (class_types_same_p (base, dclass))
-    return 1;
-
-  for (i = 0; i < TYPE_N_BASECLASSES (dclass); ++i)
-    {
-      if (! BASETYPE_VIA_PUBLIC (dclass, i))
-	continue;
-      if (is_public_ancestor (base, TYPE_BASECLASS (dclass, i)))
-	return 1;
-    }
-
-  return 0;
+  return do_is_ancestor (base, dclass, 1);
 }
 
 /* A helper function for is_unique_ancestor.  */

Re: [patch 2/2] Fix overload resolution of int* vs void*

[Eli Zaretskii]

> Date: Fri, 08 Oct 2010 15:05:23 -0400
> From: sami wagiaalla <swagiaal@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> +  if (TYPE_NAME (a) && TYPE_NAME (b)
> +      && !strcmp (TYPE_NAME (a), TYPE_NAME (b)))
> +    return 1;

A minor stylistic nit: can we please use `strcmp (...) == 0' instead
of `!strcmp (...)'?  It will produce the same code, but is slightly
more readable.

Thanks.

Re: [patch 2/2] Fix overload resolution of int* vs void*

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]


>> +  if (TYPE_NAME (a)&&  TYPE_NAME (b)
>> +&&  !strcmp (TYPE_NAME (a), TYPE_NAME (b)))
>> +    return 1;
>
> A minor stylistic nit: can we please use `strcmp (...) == 0' instead
> of `!strcmp (...)'?  It will produce the same code, but is slightly
> more readable.
>

Oh yeah, I noticed that too but forgot to change it.

This patch is revised for the above and contains a fix similar to the 
'do_is_ancestor' one pointed out by Tromey in patch 1 of this series. 
Some tests need to be update as a result.

[-- Attachment #2: overload_voidp.patch --]
[-- Type: text/x-patch, Size: 13509 bytes --]

Fixed void* vs int* overload issue (PR C++/10343).

2010-10-08  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdbtypes.h: Create BASE_PTR_CONVERSION_BADNESS.
	* gdbtypes.c (rank_one_type): Move type comparison code out of here
	to...
	(types_equal): ...here. And changed it as follows:
	Outside of typedefs type must be of the same TYPE_CODE.
	When compairing two pointers or references they are equal if their
	targets are equal.
	Correct pointer conversions.

2010-10-08  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdb.cp/converts.cc: New test program.
	* gdb.cp/converts.exp: New test.
	* gdb.cp/overload.exp: Added test for void* vs int*.
	* gdb.cp/overload.exp: Ditto.
	* gdb.cp/oranking.exp: Removed related kfail.
	* gdb.cp/virtfunc2.cc (Obj2): Change inheritance to public.
	Add calls to the function calls being tested.
	* gdb.cp/derivation.cc (G): Changed inheritance to public.
	Add calls to the function calls being tested.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 9c3152d..071ae1b 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1886,6 +1886,14 @@ do_is_ancestor (struct type *base, struct type *dclass, int public)
   if (class_types_same_p (base, dclass))
     return 1;
 
+  if ((TYPE_CODE (base) == TYPE_CODE_PTR
+       && TYPE_CODE (dclass) == TYPE_CODE_PTR)
+       || (TYPE_CODE (base) == TYPE_CODE_REF
+	   && TYPE_CODE (dclass) == TYPE_CODE_REF))
+    return do_is_ancestor (TYPE_TARGET_TYPE (base),
+                           TYPE_TARGET_TYPE (dclass),
+                           public);
+
   for (i = 0; i < TYPE_N_BASECLASSES (dclass); i++)
     {
       if (public && ! BASETYPE_VIA_PUBLIC (dclass, i))
@@ -2104,6 +2112,56 @@ integer_types_same_name_p (const char *first, const char *second)
   return 1;
 }
 
+/* Compares type A to type B returns 1 if the represent the same type
+   0 otherwise.  */
+
+static int
+types_equal (struct type *a, struct type *b)
+{
+  /* Identical type pointers.  */
+  /* However, this still doesn't catch all cases of same type for b
+     and a.  The reason is that builtin types are different from
+     the same ones constructed from the object.  */
+  if (a == b)
+    return 1;
+
+  /* Resolve typedefs */
+  if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
+    a = check_typedef (a);
+  if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
+    b = check_typedef (b);
+
+  /* If after resolving typedefs a and b are not of the same type
+     code then they are not equal.  */
+  if (TYPE_CODE (a) != TYPE_CODE (b))
+    return 0;
+
+  /* If a and b are both pointers types or both reference types then
+     they are equal of the same type iff the objects they refer to are
+     of the same type.  */
+  if (TYPE_CODE (a) == TYPE_CODE_PTR
+      || TYPE_CODE (a) == TYPE_CODE_REF)
+    return types_equal (TYPE_TARGET_TYPE (a),
+                        TYPE_TARGET_TYPE (b));
+
+  /*
+     Well, damnit, if the names are exactly the same, I'll say they
+     are exactly the same.  This happens when we generate method
+     stubs.  The types won't point to the same address, but they
+     really are the same.
+  */
+
+  if (TYPE_NAME (a) && TYPE_NAME (b)
+      && strcmp (TYPE_NAME (a), TYPE_NAME (b)) == 0)
+    return 1;
+
+  /* Check if identical after resolving typedefs.  */
+  if (a == b)
+    return 1;
+
+  return 0;
+}
+
 /* Compare one type (PARM) for compatibility with another (ARG).
  * PARM is intended to be the parameter type of a function; and
  * ARG is the supplied argument's type.  This function tests if
@@ -2117,11 +2175,8 @@ integer_types_same_name_p (const char *first, const char *second)
 int
 rank_one_type (struct type *parm, struct type *arg)
 {
-  /* Identical type pointers.  */
-  /* However, this still doesn't catch all cases of same type for arg
-     and param.  The reason is that builtin types are different from
-     the same ones constructed from the object.  */
-  if (parm == arg)
+
+  if (types_equal (parm, arg))
     return 0;
 
   /* Resolve typedefs */
@@ -2130,21 +2185,6 @@ rank_one_type (struct type *parm, struct type *arg)
   if (TYPE_CODE (arg) == TYPE_CODE_TYPEDEF)
     arg = check_typedef (arg);
 
-  /*
-     Well, damnit, if the names are exactly the same, I'll say they
-     are exactly the same.  This happens when we generate method
-     stubs.  The types won't point to the same address, but they
-     really are the same.
-  */
-
-  if (TYPE_NAME (parm) && TYPE_NAME (arg) 
-      && !strcmp (TYPE_NAME (parm), TYPE_NAME (arg)))
-    return 0;
-
-  /* Check if identical after resolving typedefs.  */
-  if (parm == arg)
-    return 0;
-
   /* See through references, since we can almost make non-references
      references.  */
   if (TYPE_CODE (arg) == TYPE_CODE_REF)
@@ -2168,15 +2208,22 @@ rank_one_type (struct type *parm, struct type *arg)
       switch (TYPE_CODE (arg))
 	{
 	case TYPE_CODE_PTR:
-	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID
-	      && TYPE_CODE (TYPE_TARGET_TYPE (arg)) != TYPE_CODE_VOID)
+
+	  /* Allowed pointer conversions are:
+	     (a) pointer to void-pointer conversion.  */
+	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID)
 	    return VOID_PTR_CONVERSION_BADNESS;
-	  else
-	    return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				  TYPE_TARGET_TYPE (arg));
+
+	  /* (b) pointer to ancestor-pointer conversion.  */
+	  if (is_public_ancestor (parm, arg))
+	    return BASE_PTR_CONVERSION_BADNESS;
+
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_ARRAY:
-	  return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				TYPE_TARGET_TYPE (arg));
+	  if (types_equal (TYPE_TARGET_TYPE (parm),
+	                   TYPE_TARGET_TYPE (arg)))
+	    return 0;
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_FUNC:
 	  return rank_one_type (TYPE_TARGET_TYPE (parm), arg);
 	case TYPE_CODE_INT:
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index dba7284..62ade5f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1400,6 +1400,9 @@ extern int is_unique_ancestor (struct type *, struct value *);
 #define INTEGER_PROMOTION_BADNESS      1
 /* Badness of floating promotion */
 #define FLOAT_PROMOTION_BADNESS        1
+/* Badness of converting a derived class pointer
+   to a base class pointer.  */
+#define BASE_PTR_CONVERSION_BADNESS    1
 /* Badness of integral conversion */
 #define INTEGER_CONVERSION_BADNESS     2
 /* Badness of floating conversion */
diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
new file mode 100644
index 0000000..9fbe833
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.cc
@@ -0,0 +1,38 @@
+class A {};
+class B : public A {};
+
+int foo1_1 (char *) {return 11;}
+int foo1_2 (char[]) {return 12;}
+int foo1_3 (int*)   {return 13;}
+int foo1_4 (A*)     {return 14;}
+int foo1_5 (void*)  {return 15;}
+int foo1_6 (void**) {return 15;}
+
+int foo2_1 (char**  )  {return 21;}
+int foo2_2 (char[][1]) {return 22;}
+int foo2_3 (char *[])  {return 23;}
+int foo2_4 (int  *[])  {return 24;}
+
+int main()
+{
+
+  char *a;             // pointer to..
+  B *bp;
+  foo1_1 (a);          // ..pointer
+  foo1_2 (a);          // ..array
+  foo1_3 ((int*)a);    // ..pointer of wrong type
+  foo1_3 ((int*)bp);   // ..pointer of wrong type
+  foo1_4 (bp);         // ..ancestor pointer
+  foo1_4 (bp);         // ..ancestor pointer
+  foo1_5 (bp);         // ..void pointer
+  foo1_6 ((void**)bp); // ..void pointer
+
+  char **b;          // pointer pointer to..
+  char ba[1][1];
+  foo1_5 (b);        // ..void pointer
+  foo2_1 (b);        // ..pointer pointer
+  foo2_2 (ba);       // ..array of arrays
+  foo2_3 (b);        // ..array of pointers
+  foo2_4 ((int**)b); // ..array of wrong pointers
+  return 0;          // end of main
+}
diff --git a/gdb/testsuite/gdb.cp/converts.exp b/gdb/testsuite/gdb.cp/converts.exp
new file mode 100644
index 0000000..40d8fbd
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.exp
@@ -0,0 +1,44 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile converts
+set srcfile ${testfile}.cc
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+     return -1
+}
+
+############################################
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "end of main"]
+gdb_continue_to_breakpoint "end of main"
+
+
+gdb_test "p foo1_1 (a)"  "= 11"
+gdb_test "p foo1_2 (a)"  "= 12"
+gdb_test "p foo1_3 (a)"  "Cannot resolve function.*"
+gdb_test "p foo1_3 (bp)" "Cannot resolve function.*"
+gdb_test "p foo1_4 (bp)" "= 14"
+gdb_test "p foo1_5 (bp)" "= 15"
+
+gdb_test "p foo1_5 (b)" "= 15"
+gdb_test "p foo2_1 (b)" "= 21"
+gdb_test "p foo2_2 (b)" "Cannot resolve function.*"
+gdb_test "p foo2_3 (b)" "= 23"
+gdb_test "p foo2_4 (b)" "Cannot resolve function.*"
diff --git a/gdb/testsuite/gdb.cp/derivation.cc b/gdb/testsuite/gdb.cp/derivation.cc
index f6d42e7..02999c1 100644
--- a/gdb/testsuite/gdb.cp/derivation.cc
+++ b/gdb/testsuite/gdb.cp/derivation.cc
@@ -96,7 +96,7 @@ public:
     
 };
 
-class G : private A, public B, protected C {
+class G : public A, public B, public C {
 public:
     int g;
     int gg;
@@ -207,7 +207,10 @@ int main(void)
     E e_instance;
     F f_instance;
     G g_instance;
-    
+
+    g_instance.afoo();
+    g_instance.cfoo();
+
     #ifdef usestubs
        set_debug_traps();
        breakpoint();
diff --git a/gdb/testsuite/gdb.cp/oranking.exp b/gdb/testsuite/gdb.cp/oranking.exp
index abe8252..f06933a 100644
--- a/gdb/testsuite/gdb.cp/oranking.exp
+++ b/gdb/testsuite/gdb.cp/oranking.exp
@@ -56,7 +56,6 @@ setup_kfail "gdb/12098" *-*-*
 gdb_test "p foo5(c)" "26"
 
 gdb_test "p test6()"  "28"
-setup_kfail "gdb/10343" *-*-*
 gdb_test "p foo6(bp)" "28"
 
 gdb_test "p test7()"  "210"
diff --git a/gdb/testsuite/gdb.cp/overload.cc b/gdb/testsuite/gdb.cp/overload.cc
index 78fae14..dc117fb 100644
--- a/gdb/testsuite/gdb.cp/overload.cc
+++ b/gdb/testsuite/gdb.cp/overload.cc
@@ -24,6 +24,9 @@ int overload1arg (unsigned long);
 int overload1arg (float);
 int overload1arg (double);
 
+int overload1arg (int*);
+int overload1arg (void*);
+
 int overloadfnarg (void);
 int overloadfnarg (int);
 int overloadfnarg (int, int (*) (int));
@@ -99,6 +102,8 @@ int main ()
     unsigned long arg10 =10;
     float arg11 =100.0;
     double arg12 = 200.0;
+    int arg13 = 200.0;
+    char arg14 = 'a';
 
     char *str = (char *) "A";
     foo foo_instance1(111);
@@ -150,6 +155,8 @@ int foo::overload1arg (long arg)            { arg = 0; return 9;}
 int foo::overload1arg (unsigned long arg)   { arg = 0; return 10;}
 int foo::overload1arg (float arg)           { arg = 0; return 11;}
 int foo::overload1arg (double arg)          { arg = 0; return 12;}
+int foo::overload1arg (int* arg)            { arg = 0; return 13;}
+int foo::overload1arg (void* arg)           { arg = 0; return 14;}
 
 /* Test to see that we can explicitly request overloaded functions
    with function pointers in the prototype. */
diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
index f05cc23..25aeb07 100644
--- a/gdb/testsuite/gdb.cp/overload.exp
+++ b/gdb/testsuite/gdb.cp/overload.exp
@@ -80,6 +80,8 @@ set re_methods	"${re_methods}${ws}int overload1arg\\(long( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\((unsigned long|long unsigned)( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(float\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(double\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(int \\*\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(void \\*\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\((void|)\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int, int ?\\(\\*\\) ?\\(int\\)\\);"
@@ -256,6 +258,14 @@ gdb_test "print foo_instance1.overload1arg((double)arg12)" \
     "\\$\[0-9\]+ = 12" \
     "print call overloaded func double arg"
 
+gdb_test "print foo_instance1.overload1arg(&arg13)" \
+    "\\$\[0-9\]+ = 13" \
+    "print call overloaded func int\\* arg"
+
+gdb_test "print foo_instance1.overload1arg(&arg14)" \
+    "\\$\[0-9\]+ = 14" \
+    "print call overloaded func char\\* arg"
+
 # ---
 
 # List overloaded functions.
diff --git a/gdb/testsuite/gdb.cp/virtfunc2.cc b/gdb/testsuite/gdb.cp/virtfunc2.cc
index 90f3eda..666a495 100644
--- a/gdb/testsuite/gdb.cp/virtfunc2.cc
+++ b/gdb/testsuite/gdb.cp/virtfunc2.cc
@@ -27,7 +27,7 @@ public:
   virtual int do_print() { return 123456; }
 };
 
-class Obj2 : Obj,  virtual public interface
+class Obj2 : public Obj,  virtual public interface
 {
   virtual int do_print2() { return 654321; }
 };
@@ -35,5 +35,7 @@ class Obj2 : Obj,  virtual public interface
 int main(int argc, char** argv) {
   Obj o;
   Obj2 o2;
+  o2.do_print();
+
   return 0;	// marker 1
 }

Re: [patch 1/2] Fix overload resolution of int* vs void*

[Tom Tromey]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> Eliminate 'is_ancestor' redundant code.

Sami> 2010-10-08  Sami Wagiaalla  <swagiaal@redhat.com>
Sami> 	* gdbtypes.c (do_is_ancestor): New function.
Sami> 	(is_ancestor): Use do_is_ancestor.
Sami> 	(is_public_ancestor): Use do_is_ancestor.

Ok.  Thanks.

Tom

Re: [patch 2/2] Fix overload resolution of int* vs void*

[Tom Tromey]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> @@ -1886,6 +1886,14 @@ do_is_ancestor (struct type *base, struct type *dclass, int public)
Sami>    if (class_types_same_p (base, dclass))
Sami>      return 1;
 
Sami> +  if ((TYPE_CODE (base) == TYPE_CODE_PTR
Sami> +       && TYPE_CODE (dclass) == TYPE_CODE_PTR)
Sami> +       || (TYPE_CODE (base) == TYPE_CODE_REF
Sami> +	   && TYPE_CODE (dclass) == TYPE_CODE_REF))
Sami> +    return do_is_ancestor (TYPE_TARGET_TYPE (base),
Sami> +                           TYPE_TARGET_TYPE (dclass),
Sami> +                           public);

This seems strange to me.  It ought to only be applicable at the
outermost call.  But in that case, I think a wrapper function would be
more appropriate, so that do_is_ancestor has a single purpose.

Also, is this the right thing for the caller when there are multiple
pointers (like "Class ***")?  Or do you intend to only strip a single
pointer?

Sami> +  /* Resolve typedefs */
Sami> +  if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
Sami> +    a = check_typedef (a);
Sami> +  if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
Sami> +    b = check_typedef (b);

It is ok to call check_typedef unconditionally, but I see this is just
copied from rank_one_type.

Sami> +	  /* (b) pointer to ancestor-pointer conversion.  */
Sami> +	  if (is_public_ancestor (parm, arg))
Sami> +	    return BASE_PTR_CONVERSION_BADNESS;

I am curious why you didn't just give POINTER_CONVERSION_BADNESS a new
value and instead introduced BASE_PTR_CONVERSION_BADNESS.

But then, I also don't understand the existing code that returns
POINTER_CONVERSION_BADNESS...

Also, why specifically is_public_ancestor and not is_ancestor?

I suppose this is an incremental patch -- still not truly correct with
regards to the distance-to-base problem, but an improvement over what
gdb currently does.  This is fine, even great, but it is handy for
reviewers if you note that sort of thing in the patch explanation.

Sami>  	case TYPE_CODE_ARRAY:
Sami> -	  return rank_one_type (TYPE_TARGET_TYPE (parm), 
Sami> -				TYPE_TARGET_TYPE (arg));
Sami> +	  if (types_equal (TYPE_TARGET_TYPE (parm),
Sami> +	                   TYPE_TARGET_TYPE (arg)))
Sami> +	    return 0;
Sami> +	  return INCOMPATIBLE_TYPE_BADNESS;

Do the new tests cover this case?  I could not immediately tell.

Tom

Re: [patch 2/2] Fix overload resolution of int* vs void*

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 3676 bytes --]

On 10/08/2010 06:54 PM, Tom Tromey wrote:
>>>>>> "Sami" == sami wagiaalla<swagiaal@redhat.com>  writes:
>
> Sami>  @@ -1886,6 +1886,14 @@ do_is_ancestor (struct type *base, struct type *dclass, int public)
> Sami>     if (class_types_same_p (base, dclass))
> Sami>       return 1;
>
> Sami>  +  if ((TYPE_CODE (base) == TYPE_CODE_PTR
> Sami>  +&&  TYPE_CODE (dclass) == TYPE_CODE_PTR)
> Sami>  +       || (TYPE_CODE (base) == TYPE_CODE_REF
> Sami>  +	&&  TYPE_CODE (dclass) == TYPE_CODE_REF))
> Sami>  +    return do_is_ancestor (TYPE_TARGET_TYPE (base),
> Sami>  +                           TYPE_TARGET_TYPE (dclass),
> Sami>  +                           public);
>
> This seems strange to me.  It ought to only be applicable at the
> outermost call.  But in that case, I think a wrapper function would be
> more appropriate, so that do_is_ancestor has a single purpose.
>
> Also, is this the right thing for the caller when there are multiple
> pointers (like "Class ***")?  Or do you intend to only strip a single
> pointer?
>

I understood that one could convert 'Class ***' to 'BaseClass ***', but 
it turns out that is in correct. I should not have assumed that without 
testing, and there is nothing in the spec that should have made me think 
so. Since that is the case, the calling function can just deference the 
pointers.

> Sami>  +  /* Resolve typedefs */
> Sami>  +  if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
> Sami>  +    a = check_typedef (a);
> Sami>  +  if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
> Sami>  +    b = check_typedef (b);
>
> It is ok to call check_typedef unconditionally, but I see this is just
> copied from rank_one_type.
>

Yeah I wondered about that when I copied the code but could not think of 
a reason not to do so.

I added a test case for the typedef scenario.

> Sami>  +	  /* (b) pointer to ancestor-pointer conversion.  */
> Sami>  +	  if (is_public_ancestor (parm, arg))
> Sami>  +	    return BASE_PTR_CONVERSION_BADNESS;
>
> I am curious why you didn't just give POINTER_CONVERSION_BADNESS a new
> value and instead introduced BASE_PTR_CONVERSION_BADNESS.
>
> But then, I also don't understand the existing code that returns
> POINTER_CONVERSION_BADNESS...
>

base pointer conversion (BASE_PTR_CONVERSION_BADNESS) is meant to be a 
slightly better option than generic (POINTER_CONVERSION_BADNESS)

> Also, why specifically is_public_ancestor and not is_ancestor?
>

You can convert a pointer to B to a pointer to A only if A is an 
accessible ancestor of B.

> I suppose this is an incremental patch -- still not truly correct with
> regards to the distance-to-base problem, but an improvement over what
> gdb currently does.

Yes. I am working on a second patch to solve the distance to base 
problem. I will post it in reply to my previous attempt at that problem.

   This is fine, even great, but it is handy for
> reviewers if you note that sort of thing in the patch explanation.
>

Cool, will do in the future. Incidentally, I should mention that is 
there is also the conversion of pointer to other data types and ranking 
within them that needs to be covered.

> Sami>   	case TYPE_CODE_ARRAY:
> Sami>  -	  return rank_one_type (TYPE_TARGET_TYPE (parm),
> Sami>  -				TYPE_TARGET_TYPE (arg));
> Sami>  +	  if (types_equal (TYPE_TARGET_TYPE (parm),
> Sami>  +	                   TYPE_TARGET_TYPE (arg)))
> Sami>  +	    return 0;
> Sami>  +	  return INCOMPATIBLE_TYPE_BADNESS;
>
> Do the new tests cover this case?  I could not immediately tell.
>

Yes, take a look at foo1_2 foo2_2, and foo2_4. Let me know if the tests 
need elaborations it is easy to miss cases when you are so close to the 
code.

Revised patch attached.

[-- Attachment #2: overload_voidp.patch --]
[-- Type: text/x-patch, Size: 13277 bytes --]

Fixed void* vs int* overload issue (PR C++/10343).

2010-10-12  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdbtypes.h: Create BASE_PTR_CONVERSION_BADNESS.
	* gdbtypes.c (rank_one_type): Move type comparison code out of here
	to...
	(types_equal): ...here. And changed it as follows:
	Outside of typedefs type must be of the same TYPE_CODE.
	When compairing two pointers or references they are equal if their
	targets are equal.
	Correct pointer conversions.

2010-10-12  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdb.cp/converts.cc: New test program.
	* gdb.cp/converts.exp: New test.
	* gdb.cp/overload.exp: Added test for void* vs int*.
	* gdb.cp/overload.exp: Ditto.
	* gdb.cp/oranking.exp: Removed related kfail.
	* gdb.cp/virtfunc2.cc (Obj2): Change inheritance to public.
	Add calls to the function calls being tested.
	* gdb.cp/derivation.cc (G): Changed inheritance to public.
	Add calls to the function calls being tested.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 9c3152d..5af7491 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2104,6 +2104,56 @@ integer_types_same_name_p (const char *first, const char *second)
   return 1;
 }
 
+/* Compares type A to type B returns 1 if the represent the same type
+   0 otherwise.  */
+
+static int
+types_equal (struct type *a, struct type *b)
+{
+  /* Identical type pointers.  */
+  /* However, this still doesn't catch all cases of same type for b
+     and a.  The reason is that builtin types are different from
+     the same ones constructed from the object.  */
+  if (a == b)
+    return 1;
+
+  /* Resolve typedefs */
+  if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
+    a = check_typedef (a);
+  if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
+    b = check_typedef (b);
+
+  /* If after resolving typedefs a and b are not of the same type
+     code then they are not equal.  */
+  if (TYPE_CODE (a) != TYPE_CODE (b))
+    return 0;
+
+  /* If a and b are both pointers types or both reference types then
+     they are equal of the same type iff the objects they refer to are
+     of the same type.  */
+  if (TYPE_CODE (a) == TYPE_CODE_PTR
+      || TYPE_CODE (a) == TYPE_CODE_REF)
+    return types_equal (TYPE_TARGET_TYPE (a),
+                        TYPE_TARGET_TYPE (b));
+
+  /*
+     Well, damnit, if the names are exactly the same, I'll say they
+     are exactly the same.  This happens when we generate method
+     stubs.  The types won't point to the same address, but they
+     really are the same.
+  */
+
+  if (TYPE_NAME (a) && TYPE_NAME (b)
+      && strcmp (TYPE_NAME (a), TYPE_NAME (b)) == 0)
+    return 1;
+
+  /* Check if identical after resolving typedefs.  */
+  if (a == b)
+    return 1;
+
+  return 0;
+}
+
 /* Compare one type (PARM) for compatibility with another (ARG).
  * PARM is intended to be the parameter type of a function; and
  * ARG is the supplied argument's type.  This function tests if
@@ -2117,11 +2167,8 @@ integer_types_same_name_p (const char *first, const char *second)
 int
 rank_one_type (struct type *parm, struct type *arg)
 {
-  /* Identical type pointers.  */
-  /* However, this still doesn't catch all cases of same type for arg
-     and param.  The reason is that builtin types are different from
-     the same ones constructed from the object.  */
-  if (parm == arg)
+
+  if (types_equal (parm, arg))
     return 0;
 
   /* Resolve typedefs */
@@ -2130,21 +2177,6 @@ rank_one_type (struct type *parm, struct type *arg)
   if (TYPE_CODE (arg) == TYPE_CODE_TYPEDEF)
     arg = check_typedef (arg);
 
-  /*
-     Well, damnit, if the names are exactly the same, I'll say they
-     are exactly the same.  This happens when we generate method
-     stubs.  The types won't point to the same address, but they
-     really are the same.
-  */
-
-  if (TYPE_NAME (parm) && TYPE_NAME (arg) 
-      && !strcmp (TYPE_NAME (parm), TYPE_NAME (arg)))
-    return 0;
-
-  /* Check if identical after resolving typedefs.  */
-  if (parm == arg)
-    return 0;
-
   /* See through references, since we can almost make non-references
      references.  */
   if (TYPE_CODE (arg) == TYPE_CODE_REF)
@@ -2168,15 +2200,23 @@ rank_one_type (struct type *parm, struct type *arg)
       switch (TYPE_CODE (arg))
 	{
 	case TYPE_CODE_PTR:
-	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID
-	      && TYPE_CODE (TYPE_TARGET_TYPE (arg)) != TYPE_CODE_VOID)
+
+	  /* Allowed pointer conversions are:
+	     (a) pointer to void-pointer conversion.  */
+	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID)
 	    return VOID_PTR_CONVERSION_BADNESS;
-	  else
-	    return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				  TYPE_TARGET_TYPE (arg));
+
+	  /* (b) pointer to ancestor-pointer conversion.  */
+	  if (is_public_ancestor (TYPE_TARGET_TYPE (parm),
+	                          TYPE_TARGET_TYPE (arg)))
+	    return BASE_PTR_CONVERSION_BADNESS;
+
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_ARRAY:
-	  return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				TYPE_TARGET_TYPE (arg));
+	  if (types_equal (TYPE_TARGET_TYPE (parm),
+	                   TYPE_TARGET_TYPE (arg)))
+	    return 0;
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_FUNC:
 	  return rank_one_type (TYPE_TARGET_TYPE (parm), arg);
 	case TYPE_CODE_INT:
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index dba7284..62ade5f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1400,6 +1400,9 @@ extern int is_unique_ancestor (struct type *, struct value *);
 #define INTEGER_PROMOTION_BADNESS      1
 /* Badness of floating promotion */
 #define FLOAT_PROMOTION_BADNESS        1
+/* Badness of converting a derived class pointer
+   to a base class pointer.  */
+#define BASE_PTR_CONVERSION_BADNESS    1
 /* Badness of integral conversion */
 #define INTEGER_CONVERSION_BADNESS     2
 /* Badness of floating conversion */
diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
new file mode 100644
index 0000000..435ed13
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.cc
@@ -0,0 +1,49 @@
+class A {};
+class B : public A {};
+
+typedef A TA1;
+typedef A TA2;
+typedef TA2 TA3;
+
+int foo0_1 (TA1) { return 1; }
+int foo0_2 (TA3) { return 2; }
+
+int foo1_1 (char *) {return 11;}
+int foo1_2 (char[]) {return 12;}
+int foo1_3 (int*)   {return 13;}
+int foo1_4 (A*)     {return 14;}
+int foo1_5 (void*)  {return 15;}
+int foo1_6 (void**) {return 15;}
+
+int foo2_1 (char**  )  {return 21;}
+int foo2_2 (char[][1]) {return 22;}
+int foo2_3 (char *[])  {return 23;}
+int foo2_4 (int  *[])  {return 24;}
+
+int main()
+{
+
+  TA2 ta;      // typedef to..
+  foo0_1 (ta); // ..another typedef
+  foo0_2 (ta); // ..typedef of a typedef
+
+  char *a;             // pointer to..
+  B *bp;
+  foo1_1 (a);          // ..pointer
+  foo1_2 (a);          // ..array
+  foo1_3 ((int*)a);    // ..pointer of wrong type
+  foo1_3 ((int*)bp);   // ..pointer of wrong type
+  foo1_4 (bp);         // ..ancestor pointer
+  foo1_4 (bp);         // ..ancestor pointer
+  foo1_5 (bp);         // ..void pointer
+  foo1_6 ((void**)bp); // ..void pointer
+
+  char **b;          // pointer pointer to..
+  char ba[1][1];
+  foo1_5 (b);        // ..void pointer
+  foo2_1 (b);        // ..pointer pointer
+  foo2_2 (ba);       // ..array of arrays
+  foo2_3 (b);        // ..array of pointers
+  foo2_4 ((int**)b); // ..array of wrong pointers
+  return 0;          // end of main
+}
diff --git a/gdb/testsuite/gdb.cp/converts.exp b/gdb/testsuite/gdb.cp/converts.exp
new file mode 100644
index 0000000..894f1cc
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.exp
@@ -0,0 +1,46 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile converts
+set srcfile ${testfile}.cc
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+     return -1
+}
+
+############################################
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "end of main"]
+gdb_continue_to_breakpoint "end of main"
+
+gdb_test "p foo0_1 (ta)"  "= 1"
+gdb_test "p foo0_2 (ta)"  "= 2"
+
+gdb_test "p foo1_1 (a)"  "= 11"
+gdb_test "p foo1_2 (a)"  "= 12"
+gdb_test "p foo1_3 (a)"  "Cannot resolve function.*"
+gdb_test "p foo1_3 (bp)" "Cannot resolve function.*"
+gdb_test "p foo1_4 (bp)" "= 14"
+gdb_test "p foo1_5 (bp)" "= 15"
+
+gdb_test "p foo1_5 (b)" "= 15"
+gdb_test "p foo2_1 (b)" "= 21"
+gdb_test "p foo2_2 (b)" "Cannot resolve function.*"
+gdb_test "p foo2_3 (b)" "= 23"
+gdb_test "p foo2_4 (b)" "Cannot resolve function.*"
diff --git a/gdb/testsuite/gdb.cp/derivation.cc b/gdb/testsuite/gdb.cp/derivation.cc
index f6d42e7..02999c1 100644
--- a/gdb/testsuite/gdb.cp/derivation.cc
+++ b/gdb/testsuite/gdb.cp/derivation.cc
@@ -96,7 +96,7 @@ public:
     
 };
 
-class G : private A, public B, protected C {
+class G : public A, public B, public C {
 public:
     int g;
     int gg;
@@ -207,7 +207,10 @@ int main(void)
     E e_instance;
     F f_instance;
     G g_instance;
-    
+
+    g_instance.afoo();
+    g_instance.cfoo();
+
     #ifdef usestubs
        set_debug_traps();
        breakpoint();
diff --git a/gdb/testsuite/gdb.cp/oranking.exp b/gdb/testsuite/gdb.cp/oranking.exp
index abe8252..f06933a 100644
--- a/gdb/testsuite/gdb.cp/oranking.exp
+++ b/gdb/testsuite/gdb.cp/oranking.exp
@@ -56,7 +56,6 @@ setup_kfail "gdb/12098" *-*-*
 gdb_test "p foo5(c)" "26"
 
 gdb_test "p test6()"  "28"
-setup_kfail "gdb/10343" *-*-*
 gdb_test "p foo6(bp)" "28"
 
 gdb_test "p test7()"  "210"
diff --git a/gdb/testsuite/gdb.cp/overload.cc b/gdb/testsuite/gdb.cp/overload.cc
index 78fae14..dc117fb 100644
--- a/gdb/testsuite/gdb.cp/overload.cc
+++ b/gdb/testsuite/gdb.cp/overload.cc
@@ -24,6 +24,9 @@ int overload1arg (unsigned long);
 int overload1arg (float);
 int overload1arg (double);
 
+int overload1arg (int*);
+int overload1arg (void*);
+
 int overloadfnarg (void);
 int overloadfnarg (int);
 int overloadfnarg (int, int (*) (int));
@@ -99,6 +102,8 @@ int main ()
     unsigned long arg10 =10;
     float arg11 =100.0;
     double arg12 = 200.0;
+    int arg13 = 200.0;
+    char arg14 = 'a';
 
     char *str = (char *) "A";
     foo foo_instance1(111);
@@ -150,6 +155,8 @@ int foo::overload1arg (long arg)            { arg = 0; return 9;}
 int foo::overload1arg (unsigned long arg)   { arg = 0; return 10;}
 int foo::overload1arg (float arg)           { arg = 0; return 11;}
 int foo::overload1arg (double arg)          { arg = 0; return 12;}
+int foo::overload1arg (int* arg)            { arg = 0; return 13;}
+int foo::overload1arg (void* arg)           { arg = 0; return 14;}
 
 /* Test to see that we can explicitly request overloaded functions
    with function pointers in the prototype. */
diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
index f05cc23..25aeb07 100644
--- a/gdb/testsuite/gdb.cp/overload.exp
+++ b/gdb/testsuite/gdb.cp/overload.exp
@@ -80,6 +80,8 @@ set re_methods	"${re_methods}${ws}int overload1arg\\(long( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\((unsigned long|long unsigned)( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(float\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(double\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(int \\*\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(void \\*\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\((void|)\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int, int ?\\(\\*\\) ?\\(int\\)\\);"
@@ -256,6 +258,14 @@ gdb_test "print foo_instance1.overload1arg((double)arg12)" \
     "\\$\[0-9\]+ = 12" \
     "print call overloaded func double arg"
 
+gdb_test "print foo_instance1.overload1arg(&arg13)" \
+    "\\$\[0-9\]+ = 13" \
+    "print call overloaded func int\\* arg"
+
+gdb_test "print foo_instance1.overload1arg(&arg14)" \
+    "\\$\[0-9\]+ = 14" \
+    "print call overloaded func char\\* arg"
+
 # ---
 
 # List overloaded functions.
diff --git a/gdb/testsuite/gdb.cp/virtfunc2.cc b/gdb/testsuite/gdb.cp/virtfunc2.cc
index 90f3eda..666a495 100644
--- a/gdb/testsuite/gdb.cp/virtfunc2.cc
+++ b/gdb/testsuite/gdb.cp/virtfunc2.cc
@@ -27,7 +27,7 @@ public:
   virtual int do_print() { return 123456; }
 };
 
-class Obj2 : Obj,  virtual public interface
+class Obj2 : public Obj,  virtual public interface
 {
   virtual int do_print2() { return 654321; }
 };
@@ -35,5 +35,7 @@ class Obj2 : Obj,  virtual public interface
 int main(int argc, char** argv) {
   Obj o;
   Obj2 o2;
+  o2.do_print();
+
   return 0;	// marker 1
 }

Re: [patch 2/2] Fix overload resolution of int* vs void*

[Tom Tromey]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> I understood that one could convert 'Class ***' to 'BaseClass ***',
Sami> but it turns out that is in correct. I should not have assumed that
Sami> without testing, and there is nothing in the spec that should have
Sami> made me think so. Since that is the case, the calling function can
Sami> just deference the pointers.

Is there a test for this?  There should be.  (I didn't check.)

Tom> I am curious why you didn't just give POINTER_CONVERSION_BADNESS a new
Tom> value and instead introduced BASE_PTR_CONVERSION_BADNESS.

Tom> But then, I also don't understand the existing code that returns
Tom> POINTER_CONVERSION_BADNESS...

Sami> base pointer conversion (BASE_PTR_CONVERSION_BADNESS) is meant to be a
Sami> slightly better option than generic (POINTER_CONVERSION_BADNESS)

I still don't understand.

I guess POINTER_CONVERSION_BADNESS is used for really bogus operations,
like converting an int to a pointer.  It seems to be a gdb extension.

I'm not sure this is worth supporting.

Tom> Also, why specifically is_public_ancestor and not is_ancestor?

Sami> You can convert a pointer to B to a pointer to A only if A is an
Sami> accessible ancestor of B.

GDB generally ignores access protection.  It seems like it ought to here
as well.

Sami> +  /* If a and b are both pointers types or both reference types then
Sami> +     they are equal of the same type iff the objects they refer to are
Sami> +     of the same type.  */
Sami> +  if (TYPE_CODE (a) == TYPE_CODE_PTR
Sami> +      || TYPE_CODE (a) == TYPE_CODE_REF)
Sami> +    return types_equal (TYPE_TARGET_TYPE (a),
Sami> +                        TYPE_TARGET_TYPE (b));

This recursive call seems a little odd.
The existence of a check for the "Class ***" case would help prove that
it is ok, though.

Tom

Re: [patch 2/2] Fix overload resolution of int* vs void*

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 3300 bytes --]

On 10/12/2010 04:40 PM, Tom Tromey wrote:
>>>>>> "Sami" == sami wagiaalla<swagiaal@redhat.com>  writes:
>
> Sami>  I understood that one could convert 'Class ***' to 'BaseClass ***',
> Sami>  but it turns out that is in correct. I should not have assumed that
> Sami>  without testing, and there is nothing in the spec that should have
> Sami>  made me think so. Since that is the case, the calling function can
> Sami>  just deference the pointers.
>
> Is there a test for this?  There should be.  (I didn't check.)
>

I added one. The test attempts to make the conversion and fails as expected.

> Tom>  I am curious why you didn't just give POINTER_CONVERSION_BADNESS a new
> Tom>  value and instead introduced BASE_PTR_CONVERSION_BADNESS.
>
> Tom>  But then, I also don't understand the existing code that returns
> Tom>  POINTER_CONVERSION_BADNESS...
>
> Sami>  base pointer conversion (BASE_PTR_CONVERSION_BADNESS) is meant to be a
> Sami>  slightly better option than generic (POINTER_CONVERSION_BADNESS)
>
> I still don't understand.
>
> I guess POINTER_CONVERSION_BADNESS is used for really bogus operations,
> like converting an int to a pointer.  It seems to be a gdb extension.
>
> I'm not sure this is worth supporting.
>

A pointer can be legally converted to a base pointer a void pointer or a 
boolean. These are ranked in that order. All other conversions are 
indeed bogus. To rank these we use:

BASE_PTR_CONVERSION_BADNESS 1
VOID_PTR_CONVERSION_BADNESS 2
POINTER_CONVERSION_BADNESS  2

Note: Converting a pointer to a matching array is considered an exact 
match (rank 0).

Having typed this out, I think POINTER_CONVERSION_BADNESS should be 
renamed to BOOL_PTR_CONVERSION_BADNESS and given a rank of 3. And all 
other conversions outlawed. I'll do this in and test it another patch. 
This patch really only deals with the first clauses of the nested switch 
statements.

I totally agree on not supporting the other conversions as extensions.

> Tom>  Also, why specifically is_public_ancestor and not is_ancestor?
>
> Sami>  You can convert a pointer to B to a pointer to A only if A is an
> Sami>  accessible ancestor of B.
>
> GDB generally ignores access protection.  It seems like it ought to here
> as well.
>

Hmm, I am worried that this will make overload resolution more 
complicated, and lead to incorrect resolution. I have no objection to 
calling the inaccessible base function if the user casts the the type:
B b;
((A)b).foo();
or
foo((A)b))
but in overload resolution I think we should stick to the standard at 
least until we are more confident in our implementation.

> Sami>  +  /* If a and b are both pointers types or both reference types then
> Sami>  +     they are equal of the same type iff the objects they refer to are
> Sami>  +     of the same type.  */
> Sami>  +  if (TYPE_CODE (a) == TYPE_CODE_PTR
> Sami>  +      || TYPE_CODE (a) == TYPE_CODE_REF)
> Sami>  +    return types_equal (TYPE_TARGET_TYPE (a),
> Sami>  +                        TYPE_TARGET_TYPE (b));
>
> This recursive call seems a little odd.
> The existence of a check for the "Class ***" case would help prove that
> it is ok, though.
>

I added the 'Class ***' check as requested, and I should point out that 
there is also foo2_2, foo2_3, and foo2_4 to test this spot of the code.

[-- Attachment #2: overload_voidp.patch --]
[-- Type: text/x-patch, Size: 13557 bytes --]

Fixed void* vs int* overload issue (PR C++/10343).

2010-10-12  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdbtypes.h: Create BASE_PTR_CONVERSION_BADNESS.
	* gdbtypes.c (rank_one_type): Move type comparison code out of here
	to...
	(types_equal): ...here. And changed it as follows:
	Outside of typedefs type must be of the same TYPE_CODE.
	When compairing two pointers or references they are equal if their
	targets are equal.
	Correct pointer conversions.

2010-10-12  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdb.cp/converts.cc: New test program.
	* gdb.cp/converts.exp: New test.
	* gdb.cp/overload.exp: Added test for void* vs int*.
	* gdb.cp/overload.exp: Ditto.
	* gdb.cp/oranking.exp: Removed related kfail.
	* gdb.cp/virtfunc2.cc (Obj2): Change inheritance to public.
	Add calls to the function calls being tested.
	* gdb.cp/derivation.cc (G): Changed inheritance to public.
	Add calls to the function calls being tested.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 9c3152d..5af7491 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2104,6 +2104,56 @@ integer_types_same_name_p (const char *first, const char *second)
   return 1;
 }
 
+/* Compares type A to type B returns 1 if the represent the same type
+   0 otherwise.  */
+
+static int
+types_equal (struct type *a, struct type *b)
+{
+  /* Identical type pointers.  */
+  /* However, this still doesn't catch all cases of same type for b
+     and a.  The reason is that builtin types are different from
+     the same ones constructed from the object.  */
+  if (a == b)
+    return 1;
+
+  /* Resolve typedefs */
+  if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
+    a = check_typedef (a);
+  if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
+    b = check_typedef (b);
+
+  /* If after resolving typedefs a and b are not of the same type
+     code then they are not equal.  */
+  if (TYPE_CODE (a) != TYPE_CODE (b))
+    return 0;
+
+  /* If a and b are both pointers types or both reference types then
+     they are equal of the same type iff the objects they refer to are
+     of the same type.  */
+  if (TYPE_CODE (a) == TYPE_CODE_PTR
+      || TYPE_CODE (a) == TYPE_CODE_REF)
+    return types_equal (TYPE_TARGET_TYPE (a),
+                        TYPE_TARGET_TYPE (b));
+
+  /*
+     Well, damnit, if the names are exactly the same, I'll say they
+     are exactly the same.  This happens when we generate method
+     stubs.  The types won't point to the same address, but they
+     really are the same.
+  */
+
+  if (TYPE_NAME (a) && TYPE_NAME (b)
+      && strcmp (TYPE_NAME (a), TYPE_NAME (b)) == 0)
+    return 1;
+
+  /* Check if identical after resolving typedefs.  */
+  if (a == b)
+    return 1;
+
+  return 0;
+}
+
 /* Compare one type (PARM) for compatibility with another (ARG).
  * PARM is intended to be the parameter type of a function; and
  * ARG is the supplied argument's type.  This function tests if
@@ -2117,11 +2167,8 @@ integer_types_same_name_p (const char *first, const char *second)
 int
 rank_one_type (struct type *parm, struct type *arg)
 {
-  /* Identical type pointers.  */
-  /* However, this still doesn't catch all cases of same type for arg
-     and param.  The reason is that builtin types are different from
-     the same ones constructed from the object.  */
-  if (parm == arg)
+
+  if (types_equal (parm, arg))
     return 0;
 
   /* Resolve typedefs */
@@ -2130,21 +2177,6 @@ rank_one_type (struct type *parm, struct type *arg)
   if (TYPE_CODE (arg) == TYPE_CODE_TYPEDEF)
     arg = check_typedef (arg);
 
-  /*
-     Well, damnit, if the names are exactly the same, I'll say they
-     are exactly the same.  This happens when we generate method
-     stubs.  The types won't point to the same address, but they
-     really are the same.
-  */
-
-  if (TYPE_NAME (parm) && TYPE_NAME (arg) 
-      && !strcmp (TYPE_NAME (parm), TYPE_NAME (arg)))
-    return 0;
-
-  /* Check if identical after resolving typedefs.  */
-  if (parm == arg)
-    return 0;
-
   /* See through references, since we can almost make non-references
      references.  */
   if (TYPE_CODE (arg) == TYPE_CODE_REF)
@@ -2168,15 +2200,23 @@ rank_one_type (struct type *parm, struct type *arg)
       switch (TYPE_CODE (arg))
 	{
 	case TYPE_CODE_PTR:
-	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID
-	      && TYPE_CODE (TYPE_TARGET_TYPE (arg)) != TYPE_CODE_VOID)
+
+	  /* Allowed pointer conversions are:
+	     (a) pointer to void-pointer conversion.  */
+	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID)
 	    return VOID_PTR_CONVERSION_BADNESS;
-	  else
-	    return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				  TYPE_TARGET_TYPE (arg));
+
+	  /* (b) pointer to ancestor-pointer conversion.  */
+	  if (is_public_ancestor (TYPE_TARGET_TYPE (parm),
+	                          TYPE_TARGET_TYPE (arg)))
+	    return BASE_PTR_CONVERSION_BADNESS;
+
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_ARRAY:
-	  return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				TYPE_TARGET_TYPE (arg));
+	  if (types_equal (TYPE_TARGET_TYPE (parm),
+	                   TYPE_TARGET_TYPE (arg)))
+	    return 0;
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_FUNC:
 	  return rank_one_type (TYPE_TARGET_TYPE (parm), arg);
 	case TYPE_CODE_INT:
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index dba7284..62ade5f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1400,6 +1400,9 @@ extern int is_unique_ancestor (struct type *, struct value *);
 #define INTEGER_PROMOTION_BADNESS      1
 /* Badness of floating promotion */
 #define FLOAT_PROMOTION_BADNESS        1
+/* Badness of converting a derived class pointer
+   to a base class pointer.  */
+#define BASE_PTR_CONVERSION_BADNESS    1
 /* Badness of integral conversion */
 #define INTEGER_CONVERSION_BADNESS     2
 /* Badness of floating conversion */
diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
new file mode 100644
index 0000000..17d1476
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.cc
@@ -0,0 +1,54 @@
+class A {};
+class B : public A {};
+
+typedef A TA1;
+typedef A TA2;
+typedef TA2 TA3;
+
+int foo0_1 (TA1)  { return 1; }
+int foo0_2 (TA3)  { return 2; }
+int foo0_3 (A***) { return 3; }
+
+int foo1_1 (char *) {return 11;}
+int foo1_2 (char[]) {return 12;}
+int foo1_3 (int*)   {return 13;}
+int foo1_4 (A*)     {return 14;}
+int foo1_5 (void*)  {return 15;}
+int foo1_6 (void**) {return 15;}
+
+int foo2_1 (char**  )  {return 21;}
+int foo2_2 (char[][1]) {return 22;}
+int foo2_3 (char *[])  {return 23;}
+int foo2_4 (int  *[])  {return 24;}
+
+int main()
+{
+
+  TA2 ta;      // typedef to..
+  foo0_1 (ta); // ..another typedef
+  foo0_2 (ta); // ..typedef of a typedef
+
+  B*** bppp;    // Pointer-to-pointer-to-pointer-to-derived..
+//foo0_3(bppp); // Pointer-to-pointer-to-pointer base.
+  foo0_3((A***)bppp); // to ensure that the function is emitted.
+
+  char *a;             // pointer to..
+  B *bp;
+  foo1_1 (a);          // ..pointer
+  foo1_2 (a);          // ..array
+  foo1_3 ((int*)a);    // ..pointer of wrong type
+  foo1_3 ((int*)bp);   // ..pointer of wrong type
+  foo1_4 (bp);         // ..ancestor pointer
+  foo1_4 (bp);         // ..ancestor pointer
+  foo1_5 (bp);         // ..void pointer
+  foo1_6 ((void**)bp); // ..void pointer
+
+  char **b;          // pointer pointer to..
+  char ba[1][1];
+  foo1_5 (b);        // ..void pointer
+  foo2_1 (b);        // ..pointer pointer
+  foo2_2 (ba);       // ..array of arrays
+  foo2_3 (b);        // ..array of pointers
+  foo2_4 ((int**)b); // ..array of wrong pointers
+  return 0;          // end of main
+}
diff --git a/gdb/testsuite/gdb.cp/converts.exp b/gdb/testsuite/gdb.cp/converts.exp
new file mode 100644
index 0000000..d22998f
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.exp
@@ -0,0 +1,47 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile converts
+set srcfile ${testfile}.cc
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+     return -1
+}
+
+############################################
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "end of main"]
+gdb_continue_to_breakpoint "end of main"
+
+gdb_test "p foo0_1 (ta)"   "= 1"
+gdb_test "p foo0_2 (ta)"   "= 2"
+gdb_test "p foo0_3 (bppp)" "Cannot resolve function.*"
+
+gdb_test "p foo1_1 (a)"  "= 11"
+gdb_test "p foo1_2 (a)"  "= 12"
+gdb_test "p foo1_3 (a)"  "Cannot resolve function.*"
+gdb_test "p foo1_3 (bp)" "Cannot resolve function.*"
+gdb_test "p foo1_4 (bp)" "= 14"
+gdb_test "p foo1_5 (bp)" "= 15"
+
+gdb_test "p foo1_5 (b)" "= 15"
+gdb_test "p foo2_1 (b)" "= 21"
+gdb_test "p foo2_2 (b)" "Cannot resolve function.*"
+gdb_test "p foo2_3 (b)" "= 23"
+gdb_test "p foo2_4 (b)" "Cannot resolve function.*"
diff --git a/gdb/testsuite/gdb.cp/derivation.cc b/gdb/testsuite/gdb.cp/derivation.cc
index f6d42e7..02999c1 100644
--- a/gdb/testsuite/gdb.cp/derivation.cc
+++ b/gdb/testsuite/gdb.cp/derivation.cc
@@ -96,7 +96,7 @@ public:
     
 };
 
-class G : private A, public B, protected C {
+class G : public A, public B, public C {
 public:
     int g;
     int gg;
@@ -207,7 +207,10 @@ int main(void)
     E e_instance;
     F f_instance;
     G g_instance;
-    
+
+    g_instance.afoo();
+    g_instance.cfoo();
+
     #ifdef usestubs
        set_debug_traps();
        breakpoint();
diff --git a/gdb/testsuite/gdb.cp/oranking.exp b/gdb/testsuite/gdb.cp/oranking.exp
index abe8252..f06933a 100644
--- a/gdb/testsuite/gdb.cp/oranking.exp
+++ b/gdb/testsuite/gdb.cp/oranking.exp
@@ -56,7 +56,6 @@ setup_kfail "gdb/12098" *-*-*
 gdb_test "p foo5(c)" "26"
 
 gdb_test "p test6()"  "28"
-setup_kfail "gdb/10343" *-*-*
 gdb_test "p foo6(bp)" "28"
 
 gdb_test "p test7()"  "210"
diff --git a/gdb/testsuite/gdb.cp/overload.cc b/gdb/testsuite/gdb.cp/overload.cc
index 78fae14..dc117fb 100644
--- a/gdb/testsuite/gdb.cp/overload.cc
+++ b/gdb/testsuite/gdb.cp/overload.cc
@@ -24,6 +24,9 @@ int overload1arg (unsigned long);
 int overload1arg (float);
 int overload1arg (double);
 
+int overload1arg (int*);
+int overload1arg (void*);
+
 int overloadfnarg (void);
 int overloadfnarg (int);
 int overloadfnarg (int, int (*) (int));
@@ -99,6 +102,8 @@ int main ()
     unsigned long arg10 =10;
     float arg11 =100.0;
     double arg12 = 200.0;
+    int arg13 = 200.0;
+    char arg14 = 'a';
 
     char *str = (char *) "A";
     foo foo_instance1(111);
@@ -150,6 +155,8 @@ int foo::overload1arg (long arg)            { arg = 0; return 9;}
 int foo::overload1arg (unsigned long arg)   { arg = 0; return 10;}
 int foo::overload1arg (float arg)           { arg = 0; return 11;}
 int foo::overload1arg (double arg)          { arg = 0; return 12;}
+int foo::overload1arg (int* arg)            { arg = 0; return 13;}
+int foo::overload1arg (void* arg)           { arg = 0; return 14;}
 
 /* Test to see that we can explicitly request overloaded functions
    with function pointers in the prototype. */
diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
index f05cc23..25aeb07 100644
--- a/gdb/testsuite/gdb.cp/overload.exp
+++ b/gdb/testsuite/gdb.cp/overload.exp
@@ -80,6 +80,8 @@ set re_methods	"${re_methods}${ws}int overload1arg\\(long( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\((unsigned long|long unsigned)( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(float\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(double\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(int \\*\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(void \\*\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\((void|)\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int, int ?\\(\\*\\) ?\\(int\\)\\);"
@@ -256,6 +258,14 @@ gdb_test "print foo_instance1.overload1arg((double)arg12)" \
     "\\$\[0-9\]+ = 12" \
     "print call overloaded func double arg"
 
+gdb_test "print foo_instance1.overload1arg(&arg13)" \
+    "\\$\[0-9\]+ = 13" \
+    "print call overloaded func int\\* arg"
+
+gdb_test "print foo_instance1.overload1arg(&arg14)" \
+    "\\$\[0-9\]+ = 14" \
+    "print call overloaded func char\\* arg"
+
 # ---
 
 # List overloaded functions.
diff --git a/gdb/testsuite/gdb.cp/virtfunc2.cc b/gdb/testsuite/gdb.cp/virtfunc2.cc
index 90f3eda..666a495 100644
--- a/gdb/testsuite/gdb.cp/virtfunc2.cc
+++ b/gdb/testsuite/gdb.cp/virtfunc2.cc
@@ -27,7 +27,7 @@ public:
   virtual int do_print() { return 123456; }
 };
 
-class Obj2 : Obj,  virtual public interface
+class Obj2 : public Obj,  virtual public interface
 {
   virtual int do_print2() { return 654321; }
 };
@@ -35,5 +35,7 @@ class Obj2 : Obj,  virtual public interface
 int main(int argc, char** argv) {
   Obj o;
   Obj2 o2;
+  o2.do_print();
+
   return 0;	// marker 1
 }

Re: [patch 2/2] Fix overload resolution of int* vs void*

[Tom Tromey]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> I added one. The test attempts to make the conversion and fails as
Sami> expected.

Thanks for adding the tests, and for the explanations.

Sami> Having typed this out, I think POINTER_CONVERSION_BADNESS should be
Sami> renamed to BOOL_PTR_CONVERSION_BADNESS and given a rank of 3. And all
Sami> other conversions outlawed. I'll do this in and test it another
Sami> patch. This patch really only deals with the first clauses of the
Sami> nested switch statements.

Thanks.

Tom> GDB generally ignores access protection.  It seems like it ought to here
Tom> as well.

Sami> Hmm, I am worried that this will make overload resolution more
Sami> complicated, and lead to incorrect resolution.

Yeah, that makes sense to me.  But I still think ignoring it is probably
the best thing to do here.

Consider if you are stopped in a method or friend function of a derived
class.  In this case you do have access to the base -- so overload
resolution should work.  But with this patch, it will not.

Sami> I have no objection to
Sami> calling the inaccessible base function if the user casts the the type:

Casting is only ok if the base is accessible :)

Access bits are a real problem.  I don't know what to do in the long
run.  Consider something like "print obj.field".  If we strictly follow
C++, that can only work when "field" is accessible -- but in a debugger
that is basically useless.

Maybe we could come up with some heuristic for when it would be
acceptable and when it would not be.  And maybe overloading should be on
the "respect access" list -- but I think for the time being we should
still ignore it, until we have a good idea of what we want and the
capability to implement it correctly.

Aside from this final issue the patch looks ok to me.

Tom

Re: [patch 2/2] Fix overload resolution of int* vs void*

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 952 bytes --]


> Consider if you are stopped in a method or friend function of a derived
> class.  In this case you do have access to the base -- so overload
> resolution should work.  But with this patch, it will not.
>

Gotcha. This makes sense. I have made the change.

> Maybe we could come up with some heuristic for when it would be
> acceptable and when it would not be.  And maybe overloading should be on
> the "respect access" list -- but I think for the time being we should
> still ignore it, until we have a good idea of what we want and the
> capability to implement it correctly.
>

If it becomes an issue in the future I think we can solve the problem by 
introducing a rank for private base conversions that is better than void 
conversions but worse than public conversions.

> Aside from this final issue the patch looks ok to me.

I will commit the patch as attached. I have added comments to the test 
cases to make debugging the .exp easier.



[-- Attachment #2: overload_voidp.patch --]
[-- Type: text/x-patch, Size: 12693 bytes --]

Fixed void* vs int* overload issue (PR C++/10343).

2010-10-13  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdbtypes.h: Create BASE_PTR_CONVERSION_BADNESS.
	* gdbtypes.c (rank_one_type): Move type comparison code out of here
	to...
	(types_equal): ...here. And changed it as follows:
	Outside of typedefs type must be of the same TYPE_CODE.
	When compairing two pointers or references they are equal if their
	targets are equal.
	Correct pointer conversions.

2010-10-13  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdb.cp/converts.cc: New test program.
	* gdb.cp/converts.exp: New test.
	* gdb.cp/overload.exp: Added test for void* vs int*.
	* gdb.cp/overload.exp: Ditto.
	* gdb.cp/oranking.exp: Removed related kfail.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 9c3152d..adced8e 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2104,6 +2104,56 @@ integer_types_same_name_p (const char *first, const char *second)
   return 1;
 }
 
+/* Compares type A to type B returns 1 if the represent the same type
+   0 otherwise.  */
+
+static int
+types_equal (struct type *a, struct type *b)
+{
+  /* Identical type pointers.  */
+  /* However, this still doesn't catch all cases of same type for b
+     and a.  The reason is that builtin types are different from
+     the same ones constructed from the object.  */
+  if (a == b)
+    return 1;
+
+  /* Resolve typedefs */
+  if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
+    a = check_typedef (a);
+  if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
+    b = check_typedef (b);
+
+  /* If after resolving typedefs a and b are not of the same type
+     code then they are not equal.  */
+  if (TYPE_CODE (a) != TYPE_CODE (b))
+    return 0;
+
+  /* If a and b are both pointers types or both reference types then
+     they are equal of the same type iff the objects they refer to are
+     of the same type.  */
+  if (TYPE_CODE (a) == TYPE_CODE_PTR
+      || TYPE_CODE (a) == TYPE_CODE_REF)
+    return types_equal (TYPE_TARGET_TYPE (a),
+                        TYPE_TARGET_TYPE (b));
+
+  /*
+     Well, damnit, if the names are exactly the same, I'll say they
+     are exactly the same.  This happens when we generate method
+     stubs.  The types won't point to the same address, but they
+     really are the same.
+  */
+
+  if (TYPE_NAME (a) && TYPE_NAME (b)
+      && strcmp (TYPE_NAME (a), TYPE_NAME (b)) == 0)
+    return 1;
+
+  /* Check if identical after resolving typedefs.  */
+  if (a == b)
+    return 1;
+
+  return 0;
+}
+
 /* Compare one type (PARM) for compatibility with another (ARG).
  * PARM is intended to be the parameter type of a function; and
  * ARG is the supplied argument's type.  This function tests if
@@ -2117,11 +2167,8 @@ integer_types_same_name_p (const char *first, const char *second)
 int
 rank_one_type (struct type *parm, struct type *arg)
 {
-  /* Identical type pointers.  */
-  /* However, this still doesn't catch all cases of same type for arg
-     and param.  The reason is that builtin types are different from
-     the same ones constructed from the object.  */
-  if (parm == arg)
+
+  if (types_equal (parm, arg))
     return 0;
 
   /* Resolve typedefs */
@@ -2130,21 +2177,6 @@ rank_one_type (struct type *parm, struct type *arg)
   if (TYPE_CODE (arg) == TYPE_CODE_TYPEDEF)
     arg = check_typedef (arg);
 
-  /*
-     Well, damnit, if the names are exactly the same, I'll say they
-     are exactly the same.  This happens when we generate method
-     stubs.  The types won't point to the same address, but they
-     really are the same.
-  */
-
-  if (TYPE_NAME (parm) && TYPE_NAME (arg) 
-      && !strcmp (TYPE_NAME (parm), TYPE_NAME (arg)))
-    return 0;
-
-  /* Check if identical after resolving typedefs.  */
-  if (parm == arg)
-    return 0;
-
   /* See through references, since we can almost make non-references
      references.  */
   if (TYPE_CODE (arg) == TYPE_CODE_REF)
@@ -2168,15 +2200,23 @@ rank_one_type (struct type *parm, struct type *arg)
       switch (TYPE_CODE (arg))
 	{
 	case TYPE_CODE_PTR:
-	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID
-	      && TYPE_CODE (TYPE_TARGET_TYPE (arg)) != TYPE_CODE_VOID)
+
+	  /* Allowed pointer conversions are:
+	     (a) pointer to void-pointer conversion.  */
+	  if (TYPE_CODE (TYPE_TARGET_TYPE (parm)) == TYPE_CODE_VOID)
 	    return VOID_PTR_CONVERSION_BADNESS;
-	  else
-	    return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				  TYPE_TARGET_TYPE (arg));
+
+	  /* (b) pointer to ancestor-pointer conversion.  */
+	  if (is_ancestor (TYPE_TARGET_TYPE (parm),
+	                          TYPE_TARGET_TYPE (arg)))
+	    return BASE_PTR_CONVERSION_BADNESS;
+
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_ARRAY:
-	  return rank_one_type (TYPE_TARGET_TYPE (parm), 
-				TYPE_TARGET_TYPE (arg));
+	  if (types_equal (TYPE_TARGET_TYPE (parm),
+	                   TYPE_TARGET_TYPE (arg)))
+	    return 0;
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_FUNC:
 	  return rank_one_type (TYPE_TARGET_TYPE (parm), arg);
 	case TYPE_CODE_INT:
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index dba7284..62ade5f 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1400,6 +1400,9 @@ extern int is_unique_ancestor (struct type *, struct value *);
 #define INTEGER_PROMOTION_BADNESS      1
 /* Badness of floating promotion */
 #define FLOAT_PROMOTION_BADNESS        1
+/* Badness of converting a derived class pointer
+   to a base class pointer.  */
+#define BASE_PTR_CONVERSION_BADNESS    1
 /* Badness of integral conversion */
 #define INTEGER_CONVERSION_BADNESS     2
 /* Badness of floating conversion */
diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
new file mode 100644
index 0000000..b5e7bde
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.cc
@@ -0,0 +1,53 @@
+class A {};
+class B : public A {};
+
+typedef A TA1;
+typedef A TA2;
+typedef TA2 TA3;
+
+int foo0_1 (TA1)  { return 1; }
+int foo0_2 (TA3)  { return 2; }
+int foo0_3 (A***) { return 3; }
+
+int foo1_1 (char *) {return 11;}
+int foo1_2 (char[]) {return 12;}
+int foo1_3 (int*)   {return 13;}
+int foo1_4 (A*)     {return 14;}
+int foo1_5 (void*)  {return 15;}
+int foo1_6 (void**) {return 15;}
+
+int foo2_1 (char**  )  {return 21;}
+int foo2_2 (char[][1]) {return 22;}
+int foo2_3 (char *[])  {return 23;}
+int foo2_4 (int  *[])  {return 24;}
+
+int main()
+{
+
+  TA2 ta;      // typedef to..
+  foo0_1 (ta); // ..another typedef
+  foo0_2 (ta); // ..typedef of a typedef
+
+  B*** bppp;    // Pointer-to-pointer-to-pointer-to-derived..
+//foo0_3(bppp); // Pointer-to-pointer-to-pointer base.
+  foo0_3((A***)bppp); // to ensure that the function is emitted.
+
+  char *a;             // pointer to..
+  B *bp;
+  foo1_1 (a);          // ..pointer
+  foo1_2 (a);          // ..array
+  foo1_3 ((int*)a);    // ..pointer of wrong type
+  foo1_3 ((int*)bp);   // ..pointer of wrong type
+  foo1_4 (bp);         // ..ancestor pointer
+  foo1_5 (bp);         // ..void pointer
+  foo1_6 ((void**)bp); // ..void pointer
+
+  char **b;          // pointer pointer to..
+  char ba[1][1];
+  foo1_5 (b);        // ..void pointer
+  foo2_1 (b);        // ..pointer pointer
+  foo2_2 (ba);       // ..array of arrays
+  foo2_3 (b);        // ..array of pointers
+  foo2_4 ((int**)b); // ..array of wrong pointers
+  return 0;          // end of main
+}
diff --git a/gdb/testsuite/gdb.cp/converts.exp b/gdb/testsuite/gdb.cp/converts.exp
new file mode 100644
index 0000000..121bcad
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/converts.exp
@@ -0,0 +1,48 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+set testfile converts
+set srcfile ${testfile}.cc
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+     return -1
+}
+
+############################################
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+gdb_breakpoint [gdb_get_line_number "end of main"]
+gdb_continue_to_breakpoint "end of main"
+
+gdb_test "p foo0_1 (ta)"   "= 1"              "typedef to another typedef"
+gdb_test "p foo0_2 (ta)"   "= 2"              "typedef to typedef of a typedef"
+gdb_test "p foo0_3 (bppp)" "Cannot resolve.*" \
+ "Pointer-to-pointer-to-pointer derived to Pointer-to-pointer-to-pointer base."
+
+gdb_test "p foo1_1 (a)"  "= 11"             "pointer to pointer"
+gdb_test "p foo1_2 (a)"  "= 12"             "pointer to array"
+gdb_test "p foo1_3 (a)"  "Cannot resolve.*" "pointer to pointer of wrong type"
+gdb_test "p foo1_3 (bp)" "Cannot resolve.*" "pointer to pointer of wrong type"
+gdb_test "p foo1_4 (bp)" "= 14"             "pointer to ancestor pointer"
+gdb_test "p foo1_5 (bp)" "= 15"             "pointer to void pointer"
+
+gdb_test "p foo1_5 (b)" "= 15"             "pointer pointer to void pointer"
+gdb_test "p foo2_1 (b)" "= 21"             "pointer pointer to pointer pointer"
+gdb_test "p foo2_2 (b)" "Cannot resolve.*" "pointer pointer to array of arrays"
+gdb_test "p foo2_3 (b)" "= 23"             "pointer pointer to array of pointers"
+gdb_test "p foo2_4 (b)" "Cannot resolve.*" "pointer pointer to array of wrong pointers"
diff --git a/gdb/testsuite/gdb.cp/oranking.exp b/gdb/testsuite/gdb.cp/oranking.exp
index abe8252..f06933a 100644
--- a/gdb/testsuite/gdb.cp/oranking.exp
+++ b/gdb/testsuite/gdb.cp/oranking.exp
@@ -56,7 +56,6 @@ setup_kfail "gdb/12098" *-*-*
 gdb_test "p foo5(c)" "26"
 
 gdb_test "p test6()"  "28"
-setup_kfail "gdb/10343" *-*-*
 gdb_test "p foo6(bp)" "28"
 
 gdb_test "p test7()"  "210"
diff --git a/gdb/testsuite/gdb.cp/overload.cc b/gdb/testsuite/gdb.cp/overload.cc
index 78fae14..dc117fb 100644
--- a/gdb/testsuite/gdb.cp/overload.cc
+++ b/gdb/testsuite/gdb.cp/overload.cc
@@ -24,6 +24,9 @@ int overload1arg (unsigned long);
 int overload1arg (float);
 int overload1arg (double);
 
+int overload1arg (int*);
+int overload1arg (void*);
+
 int overloadfnarg (void);
 int overloadfnarg (int);
 int overloadfnarg (int, int (*) (int));
@@ -99,6 +102,8 @@ int main ()
     unsigned long arg10 =10;
     float arg11 =100.0;
     double arg12 = 200.0;
+    int arg13 = 200.0;
+    char arg14 = 'a';
 
     char *str = (char *) "A";
     foo foo_instance1(111);
@@ -150,6 +155,8 @@ int foo::overload1arg (long arg)            { arg = 0; return 9;}
 int foo::overload1arg (unsigned long arg)   { arg = 0; return 10;}
 int foo::overload1arg (float arg)           { arg = 0; return 11;}
 int foo::overload1arg (double arg)          { arg = 0; return 12;}
+int foo::overload1arg (int* arg)            { arg = 0; return 13;}
+int foo::overload1arg (void* arg)           { arg = 0; return 14;}
 
 /* Test to see that we can explicitly request overloaded functions
    with function pointers in the prototype. */
diff --git a/gdb/testsuite/gdb.cp/overload.exp b/gdb/testsuite/gdb.cp/overload.exp
index f05cc23..25aeb07 100644
--- a/gdb/testsuite/gdb.cp/overload.exp
+++ b/gdb/testsuite/gdb.cp/overload.exp
@@ -80,6 +80,8 @@ set re_methods	"${re_methods}${ws}int overload1arg\\(long( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\((unsigned long|long unsigned)( int)?\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(float\\);"
 set re_methods	"${re_methods}${ws}int overload1arg\\(double\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(int \\*\\);"
+set re_methods	"${re_methods}${ws}int overload1arg\\(void \\*\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\((void|)\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int\\);"
 set re_methods	"${re_methods}${ws}int overloadfnarg\\(int, int ?\\(\\*\\) ?\\(int\\)\\);"
@@ -256,6 +258,14 @@ gdb_test "print foo_instance1.overload1arg((double)arg12)" \
     "\\$\[0-9\]+ = 12" \
     "print call overloaded func double arg"
 
+gdb_test "print foo_instance1.overload1arg(&arg13)" \
+    "\\$\[0-9\]+ = 13" \
+    "print call overloaded func int\\* arg"
+
+gdb_test "print foo_instance1.overload1arg(&arg14)" \
+    "\\$\[0-9\]+ = 14" \
+    "print call overloaded func char\\* arg"
+
 # ---
 
 # List overloaded functions.

Re: [patch 2/2] Fix overload resolution of int* vs void*

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]


> Sami>  Having typed this out, I think POINTER_CONVERSION_BADNESS should be
> Sami>  renamed to BOOL_PTR_CONVERSION_BADNESS and given a rank of 3. And all
> Sami>  other conversions outlawed. I'll do this in and test it another
> Sami>  patch. This patch really only deals with the first clauses of the
> Sami>  nested switch statements.
>
> Thanks.

Patch attached as promised.

[-- Attachment #2: overload_pointer_to_bool.patch --]
[-- Type: text/x-patch, Size: 4223 bytes --]

Support pointer to bool conversion.

2010-10-13  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdbtypes.h: Introduce BOOL_PTR_CONVERSION_BADNESS.
	* gdbtypes.c (rank_one_type): Use BOOL_PTR_CONVERSION_BADNESS
	for conversion.
	Make all other conversions illegal.

2010-10-13  Sami Wagiaalla  <swagiaal@redhat.com>

	* gdb.cp/converts.exp: Test pointer to bool conversion.
	Test pointer to long conversion.
	* gdb.cp/oranking.exp: Removed relevant kfail.

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index adced8e..5f6e6d8 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -2225,7 +2225,6 @@ rank_one_type (struct type *parm, struct type *arg)
 	case TYPE_CODE_CHAR:
 	case TYPE_CODE_RANGE:
 	case TYPE_CODE_BOOL:
-	  return POINTER_CONVERSION_BADNESS;
 	default:
 	  return INCOMPATIBLE_TYPE_BADNESS;
 	}
@@ -2403,8 +2402,9 @@ rank_one_type (struct type *parm, struct type *arg)
 	case TYPE_CODE_RANGE:
 	case TYPE_CODE_ENUM:
 	case TYPE_CODE_FLT:
+	  return INCOMPATIBLE_TYPE_BADNESS;
 	case TYPE_CODE_PTR:
-	  return BOOLEAN_CONVERSION_BADNESS;
+	  return BOOL_PTR_CONVERSION_BADNESS;
 	case TYPE_CODE_BOOL:
 	  return 0;
 	default:
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 62ade5f..6119114 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1409,12 +1409,10 @@ extern int is_unique_ancestor (struct type *, struct value *);
 #define FLOAT_CONVERSION_BADNESS       2
 /* Badness of integer<->floating conversions */
 #define INT_FLOAT_CONVERSION_BADNESS   2
-/* Badness of converting to a boolean */
-#define BOOLEAN_CONVERSION_BADNESS     2
-/* Badness of pointer conversion */
-#define POINTER_CONVERSION_BADNESS     2
 /* Badness of conversion of pointer to void pointer */
 #define VOID_PTR_CONVERSION_BADNESS    2
+/* Badness of conversion of pointer to boolean.  */
+#define BOOL_PTR_CONVERSION_BADNESS    3
 /* Badness of converting derived to base class */
 #define BASE_CONVERSION_BADNESS        2
 /* Badness of converting from non-reference to reference */
diff --git a/gdb/testsuite/gdb.cp/converts.cc b/gdb/testsuite/gdb.cp/converts.cc
index b5e7bde..34b6927 100644
--- a/gdb/testsuite/gdb.cp/converts.cc
+++ b/gdb/testsuite/gdb.cp/converts.cc
@@ -14,7 +14,9 @@ int foo1_2 (char[]) {return 12;}
 int foo1_3 (int*)   {return 13;}
 int foo1_4 (A*)     {return 14;}
 int foo1_5 (void*)  {return 15;}
-int foo1_6 (void**) {return 15;}
+int foo1_6 (void**) {return 16;}
+int foo1_7 (bool)   {return 17;}
+int foo1_8 (long)   {return 18;}
 
 int foo2_1 (char**  )  {return 21;}
 int foo2_2 (char[][1]) {return 22;}
@@ -40,7 +42,9 @@ int main()
   foo1_3 ((int*)bp);   // ..pointer of wrong type
   foo1_4 (bp);         // ..ancestor pointer
   foo1_5 (bp);         // ..void pointer
-  foo1_6 ((void**)bp); // ..void pointer
+  foo1_6 ((void**)bp); // ..void pointer pointer
+  foo1_7 (bp);         // ..boolean
+  foo1_8 ((long)bp);   // ..long int
 
   char **b;          // pointer pointer to..
   char ba[1][1];
diff --git a/gdb/testsuite/gdb.cp/converts.exp b/gdb/testsuite/gdb.cp/converts.exp
index 121bcad..4e4c2ea 100644
--- a/gdb/testsuite/gdb.cp/converts.exp
+++ b/gdb/testsuite/gdb.cp/converts.exp
@@ -40,6 +40,9 @@ gdb_test "p foo1_3 (a)"  "Cannot resolve.*" "pointer to pointer of wrong type"
 gdb_test "p foo1_3 (bp)" "Cannot resolve.*" "pointer to pointer of wrong type"
 gdb_test "p foo1_4 (bp)" "= 14"             "pointer to ancestor pointer"
 gdb_test "p foo1_5 (bp)" "= 15"             "pointer to void pointer"
+gdb_test "p foo1_6 (bp)" "Cannot resolve.*"     "pointer to void pointer pointer"
+gdb_test "p foo1_7 (bp)" "= 17"                 "pointer to boolean"
+gdb_test "p foo1_8 (bp)" "Using non-standard.*" "pointer to long int"
 
 gdb_test "p foo1_5 (b)" "= 15"             "pointer pointer to void pointer"
 gdb_test "p foo2_1 (b)" "= 21"             "pointer pointer to pointer pointer"
diff --git a/gdb/testsuite/gdb.cp/oranking.exp b/gdb/testsuite/gdb.cp/oranking.exp
index f06933a..f1efb77 100644
--- a/gdb/testsuite/gdb.cp/oranking.exp
+++ b/gdb/testsuite/gdb.cp/oranking.exp
@@ -52,7 +52,6 @@ setup_kfail "gdb/12098" *-*-*
 gdb_test "p foo4(&a)" "24"
 
 gdb_test "p test5()" "26"
-setup_kfail "gdb/12098" *-*-*
 gdb_test "p foo5(c)" "26"
 
 gdb_test "p test6()"  "28"

Re: [patch 2/2] Fix overload resolution of int* vs void*

[Tom Tromey]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> Having typed this out, I think POINTER_CONVERSION_BADNESS should be
Sami> renamed to BOOL_PTR_CONVERSION_BADNESS and given a rank of 3. And all
Sami> other conversions outlawed. I'll do this in and test it another
Sami> patch. This patch really only deals with the first clauses of the
Sami> nested switch statements.

Sami> Patch attached as promised.

Sami> Support pointer to bool conversion.

Ok.  Thanks.

Tom