Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: sami wagiaalla <swagiaal@redhat.com>
To: Tom Tromey <tromey@redhat.com>
Cc: Eli Zaretskii <eliz@gnu.org>, gdb-patches@sourceware.org
Subject: Re: [patch 2/2] Fix overload resolution of int* vs void*
Date: Wed, 13 Oct 2010 18:29:00 -0000	[thread overview]
Message-ID: <4CB5FA85.605@redhat.com> (raw)
In-Reply-To: <m34ocqcctf.fsf@fleche.redhat.com>

[-- 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.

  reply	other threads:[~2010-10-13 18:29 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-30 15:24 [patch] " sami wagiaalla
2010-08-30 16:13 ` sami wagiaalla
2010-08-30 20:01 ` Tom Tromey
2010-10-08 18:39   ` [patch 1/2] " sami wagiaalla
2010-10-08 18:54     ` Tom Tromey
2010-10-08 19:35       ` sami wagiaalla
2010-10-08 21:30         ` Tom Tromey
2010-10-08 19:05   ` [patch 2/2] " sami wagiaalla
2010-10-08 20:46     ` Eli Zaretskii
2010-10-08 21:10       ` sami wagiaalla
2010-10-08 22:54         ` Tom Tromey
2010-10-12 20:01           ` sami wagiaalla
2010-10-12 20:41             ` Tom Tromey
2010-10-13 15:16               ` sami wagiaalla
2010-10-13 15:49                 ` Tom Tromey
2010-10-13 18:29                   ` sami wagiaalla [this message]
2010-10-15 14:46                   ` sami wagiaalla
2010-10-15 22:48                     ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4CB5FA85.605@redhat.com \
    --to=swagiaal@redhat.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=tromey@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox