public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [review] gdb: fix overload resolution for see-through references
@ 2019-11-12 15:23 Tankut Baris Aktemur (Code Review)
  2019-11-27  7:46 ` Tankut Baris Aktemur (Code Review)
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-11-12 15:23 UTC (permalink / raw)
  To: gdb-patches

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/617
......................................................................

gdb: fix overload resolution for see-through references

The overload resolution mechanism assigns badness values to the
necessary conversions to be made on types to pick a champion.  A
badness value consists of a "rank" that scores the conversion and a
"subrank" to differentiate conversions of the same kind.

An auxiliary function, 'sum_ranks', is used for adding two badness
values.  In all of its uses, except two, 'sum_ranks' is used for
populating the subrank of a badness value.  The two exceptions are in
'rank_one_type':

~~~
  /* See through references, since we can almost make non-references
     references.  */

  if (TYPE_IS_REFERENCE (arg))
    return (sum_ranks (rank_one_type (parm, TYPE_TARGET_TYPE (arg), NULL),
		       REFERENCE_CONVERSION_BADNESS));
  if (TYPE_IS_REFERENCE (parm))
    return (sum_ranks (rank_one_type (TYPE_TARGET_TYPE (parm), arg, NULL),
		       REFERENCE_CONVERSION_BADNESS));
~~~

Here, the result of a recursive call is combined with
REFERENCE_CONVERSION_BADNESS.  This leads to the problem of
over-punishment by combining two ranks.  Consider this:

    void an_overloaded_function (const foo &);
    void an_overloaded_function (const foo &&);
    ...
    foo arg;
    an_overloaded_function(arg);

When ranking 'an_overloaded_function (const foo &)', the badness
values REFERENCE_CONVERSION_BADNESS and CV_CONVERSION_BADNESS are
combined, whereas 'rank_one_type' assigns only the
REFERENCE_CONVERSION_BADNESS value to 'an_overloaded_function (const
foo &&)' (there is a different execution flow for that).  This yields
in GDB picking the latter function as the overload champion instead of
the former.

In fact, the 'rank_one_type' function should have given
'an_overloaded_function (const foo &)' the CV_CONVERSION_BADNESS
value, with the see-through referencing increasing the subrank a
little bit.  This can be achieved by introducing a new badness value,
REFERENCE_SEE_THROUGH_BADNESS, which bumps up the subrank only, and
using it in the two "exceptional" cases of 'sum_ranks'.

gdb/ChangeLog:
2019-11-12  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdbtypes.h: Define the REFERENCE_SEE_THROUGH_BADNESS value.
	* gdbtypes.c (rank_one_type): Use REFERENCE_SEE_THROUGH_BADNESS
	for ranking see-through reference cases.

gdb/testsuite/ChangeLog:
2019-11-12  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.cp/rvalue-ref-overload.cc: Add a case that involves both
	CV and reference conversion for overload resolution.
	* gdb.cp/rvalue-ref-overload.exp: Test it.

Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
---
M gdb/gdbtypes.c
M gdb/gdbtypes.h
M gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
M gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
4 files changed, 17 insertions(+), 2 deletions(-)



diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index fd1c765..2747012 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -60,6 +60,7 @@
 const struct rank BOOL_CONVERSION_BADNESS = {3,0};
 const struct rank BASE_CONVERSION_BADNESS = {2,0};
 const struct rank REFERENCE_CONVERSION_BADNESS = {2,0};
+const struct rank REFERENCE_SEE_THROUGH_BADNESS = {0,1};
 const struct rank NULL_POINTER_CONVERSION_BADNESS = {2,0};
 const struct rank NS_POINTER_CONVERSION_BADNESS = {10,0};
 const struct rank NS_INTEGER_POINTER_CONVERSION_BADNESS = {3,0};
@@ -4236,10 +4237,10 @@
 
   if (TYPE_IS_REFERENCE (arg))
     return (sum_ranks (rank_one_type (parm, TYPE_TARGET_TYPE (arg), NULL),
-                       REFERENCE_CONVERSION_BADNESS));
+                       REFERENCE_SEE_THROUGH_BADNESS));
   if (TYPE_IS_REFERENCE (parm))
     return (sum_ranks (rank_one_type (TYPE_TARGET_TYPE (parm), arg, NULL),
-                       REFERENCE_CONVERSION_BADNESS));
+                       REFERENCE_SEE_THROUGH_BADNESS));
   if (overload_debug)
   /* Debugging only.  */
     fprintf_filtered (gdb_stderr, 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 6d6ff59..590943e 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2072,6 +2072,7 @@
 /* * Badness of converting from non-reference to reference.  Subrank
    is the type of reference conversion being done.  */
 extern const struct rank REFERENCE_CONVERSION_BADNESS;
+extern const struct rank REFERENCE_SEE_THROUGH_BADNESS;
 /* * Conversion to rvalue reference.  */
 #define REFERENCE_CONVERSION_RVALUE 1
 /* * Conversion to const lvalue reference.  */
diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc b/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
index fa6cab0..e3111d5 100644
--- a/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
+++ b/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
@@ -35,6 +35,8 @@
 
   int overload1arg (foo_lval_ref);
   int overload1arg (foo_rval_ref);
+  int overloadConst (const foo &);
+  int overloadConst (const foo &&);
 };
 
 void
@@ -71,6 +73,11 @@
   // result = 1 + 2 + 3 + 3 = 9
   int result = f (i) + f (ci) + f (0) + f (std::move (i));
 
+  /* Overload resolution below requires both a CV-conversion
+     and reference conversion.  */
+  int test_const // = 3
+    = foo_rr_instance1.overloadConst (arg);
+
   marker1 (); // marker1-returns-here
   return result;
 }
@@ -84,3 +91,5 @@
 
 int foo::overload1arg (foo_lval_ref arg)           { return 1; }
 int foo::overload1arg (foo_rval_ref arg)           { return 2; }
+int foo::overloadConst (const foo &arg)            { return 3; }
+int foo::overloadConst (const foo &&arg)           { return 4; }
diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp b/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
index 61f81b4..693c7ca 100644
--- a/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
+++ b/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
@@ -49,6 +49,8 @@
 	{ method public "~foo();" }
 	{ method public "int overload1arg(foo_lval_ref);" }
 	{ method public "int overload1arg(foo_rval_ref);" }
+	{ method public "int overloadConst(const foo &);" }
+	{ method public "int overloadConst(const foo &&);" }
     }
 
 gdb_test "print foo_rr_instance1.overload1arg(arg)" \
@@ -59,6 +61,8 @@
     "\\$\[0-9\]+ = 2" \
     "print call overloaded func foo && arg"
 
+gdb_test "print foo_rr_instance1.overloadConst(arg)" "3"
+
 # Test lvalue vs rvalue function overloads
 gdb_test "print f (i)" "= 1" "lvalue reference overload"
 

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
Gerrit-Change-Number: 617
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-MessageType: newchange

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

* [review] gdb: fix overload resolution for see-through references
  2019-11-12 15:23 [review] gdb: fix overload resolution for see-through references Tankut Baris Aktemur (Code Review)
@ 2019-11-27  7:46 ` Tankut Baris Aktemur (Code Review)
  2019-11-29 16:43 ` Simon Marchi (Code Review)
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-11-27  7:46 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey, Keith Seitz

Tankut Baris Aktemur has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/617
......................................................................


Patch Set 1:

Kindly ping'ing.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
Gerrit-Change-Number: 617
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Keith Seitz <keiths@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Wed, 27 Nov 2019 07:45:44 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

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

* [review] gdb: fix overload resolution for see-through references
  2019-11-12 15:23 [review] gdb: fix overload resolution for see-through references Tankut Baris Aktemur (Code Review)
  2019-11-27  7:46 ` Tankut Baris Aktemur (Code Review)
@ 2019-11-29 16:43 ` Simon Marchi (Code Review)
  2019-11-29 18:57 ` Tankut Baris Aktemur (Code Review)
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi (Code Review) @ 2019-11-29 16:43 UTC (permalink / raw)
  To: Tankut Baris Aktemur, gdb-patches; +Cc: Tom Tromey, Keith Seitz

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/617
......................................................................


Patch Set 1:

I don't know much about the special C++ overload resolution rules, so I'd appreciate if somebody with better knowledge in that area reviewed the patch.  I have tested though, and it works as far as I can tell.

One question about the 'an_overloaded_function (const foo &&)' overload.  It would never be legal for the compiler to call this overload when doing:

 struct foo f;
 an_overloaded_function (f);

So why does GDB even consider it when searching for a possible overload to call?  Let's say there exists the overload 'an_overloaded_function (const foo &&)' but the overload 'an_overloaded_function (const foo &)' does not exist, and the user calls 'an_overloaded_function (f)'.  Should GDB call the && overload, or should GDB say that no compatible overload was found?


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
Gerrit-Change-Number: 617
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Keith Seitz <keiths@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Fri, 29 Nov 2019 16:43:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

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

* [review] gdb: fix overload resolution for see-through references
  2019-11-12 15:23 [review] gdb: fix overload resolution for see-through references Tankut Baris Aktemur (Code Review)
  2019-11-27  7:46 ` Tankut Baris Aktemur (Code Review)
  2019-11-29 16:43 ` Simon Marchi (Code Review)
@ 2019-11-29 18:57 ` Tankut Baris Aktemur (Code Review)
  2019-12-05 20:13 ` Tom Tromey (Code Review)
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Tankut Baris Aktemur (Code Review) @ 2019-11-29 18:57 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, Tom Tromey, Keith Seitz

Tankut Baris Aktemur has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/617
......................................................................


Patch Set 1:

> One question about the 'an_overloaded_function (const foo &&)' overload.  It would never be legal for the compiler to call this overload when doing:
> 
>  struct foo f;
>  an_overloaded_function (f);
> 
> So why does GDB even consider it when searching for a possible overload to call?  Let's say there exists the overload 'an_overloaded_function (const foo &&)' but the overload 'an_overloaded_function (const foo &)' does not exist, and the user calls 'an_overloaded_function (f)'.  Should GDB call the && overload, or should GDB say that no compatible overload was found?

I believe it's expected that GDB will search the function during overload resolution.  For an illegal case like this, I'd expect that it ranks the function with a very high badness value, so that an infcall is refused, just like the compiler rejects it.

Currently, if you give 'f' a const type, GDB makes the call (but it's still illegal).  If non-const like above, GDB says "Attempt to take address of value not located in memory."  It seems this is a separate bug; GDB resolved the function and attempted to call it but a failure occurred during call preparation.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
Gerrit-Change-Number: 617
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Keith Seitz <keiths@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Fri, 29 Nov 2019 18:57:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

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

* [review] gdb: fix overload resolution for see-through references
  2019-11-12 15:23 [review] gdb: fix overload resolution for see-through references Tankut Baris Aktemur (Code Review)
                   ` (2 preceding siblings ...)
  2019-11-29 18:57 ` Tankut Baris Aktemur (Code Review)
@ 2019-12-05 20:13 ` Tom Tromey (Code Review)
  2019-12-06  7:25 ` [pushed] " Sourceware to Gerrit sync (Code Review)
  2019-12-06  7:25 ` Sourceware to Gerrit sync (Code Review)
  5 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey (Code Review) @ 2019-12-05 20:13 UTC (permalink / raw)
  To: Tankut Baris Aktemur, gdb-patches; +Cc: Simon Marchi, Keith Seitz

Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/617
......................................................................


Patch Set 1: Code-Review+2

Thank you for doing this.  I appreciated the explanation.
The patch looks good to me.


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
Gerrit-Change-Number: 617
Gerrit-PatchSet: 1
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Keith Seitz <keiths@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Thu, 05 Dec 2019 20:13:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

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

* [pushed] gdb: fix overload resolution for see-through references
  2019-11-12 15:23 [review] gdb: fix overload resolution for see-through references Tankut Baris Aktemur (Code Review)
                   ` (4 preceding siblings ...)
  2019-12-06  7:25 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2019-12-06  7:25 ` Sourceware to Gerrit sync (Code Review)
  5 siblings, 0 replies; 7+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-12-06  7:25 UTC (permalink / raw)
  To: Tankut Baris Aktemur, Keith Seitz, Tom Tromey, gdb-patches; +Cc: Simon Marchi

The original change was created by Tankut Baris Aktemur.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/617
......................................................................

gdb: fix overload resolution for see-through references

The overload resolution mechanism assigns badness values to the
necessary conversions to be made on types to pick a champion.  A
badness value consists of a "rank" that scores the conversion and a
"subrank" to differentiate conversions of the same kind.

An auxiliary function, 'sum_ranks', is used for adding two badness
values.  In all of its uses, except two, 'sum_ranks' is used for
populating the subrank of a badness value.  The two exceptions are in
'rank_one_type':

~~~
  /* See through references, since we can almost make non-references
     references.  */

  if (TYPE_IS_REFERENCE (arg))
    return (sum_ranks (rank_one_type (parm, TYPE_TARGET_TYPE (arg), NULL),
		       REFERENCE_CONVERSION_BADNESS));
  if (TYPE_IS_REFERENCE (parm))
    return (sum_ranks (rank_one_type (TYPE_TARGET_TYPE (parm), arg, NULL),
		       REFERENCE_CONVERSION_BADNESS));
~~~

Here, the result of a recursive call is combined with
REFERENCE_CONVERSION_BADNESS.  This leads to the problem of
over-punishment by combining two ranks.  Consider this:

    void an_overloaded_function (const foo &);
    void an_overloaded_function (const foo &&);
    ...
    foo arg;
    an_overloaded_function(arg);

When ranking 'an_overloaded_function (const foo &)', the badness
values REFERENCE_CONVERSION_BADNESS and CV_CONVERSION_BADNESS are
combined, whereas 'rank_one_type' assigns only the
REFERENCE_CONVERSION_BADNESS value to 'an_overloaded_function (const
foo &&)' (there is a different execution flow for that).  This yields
in GDB picking the latter function as the overload champion instead of
the former.

In fact, the 'rank_one_type' function should have given
'an_overloaded_function (const foo &)' the CV_CONVERSION_BADNESS
value, with the see-through referencing increasing the subrank a
little bit.  This can be achieved by introducing a new badness value,
REFERENCE_SEE_THROUGH_BADNESS, which bumps up the subrank only, and
using it in the two "exceptional" cases of 'sum_ranks'.

gdb/ChangeLog:
2019-12-06  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdbtypes.h: Define the REFERENCE_SEE_THROUGH_BADNESS value.
	* gdbtypes.c (rank_one_type): Use REFERENCE_SEE_THROUGH_BADNESS
	for ranking see-through reference cases.

gdb/testsuite/ChangeLog:
2019-12-06  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.cp/rvalue-ref-overload.cc: Add a case that involves both
	CV and reference conversion for overload resolution.
	* gdb.cp/rvalue-ref-overload.exp: Test it.

Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
---
M gdb/ChangeLog
M gdb/gdbtypes.c
M gdb/gdbtypes.h
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
M gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
6 files changed, 29 insertions(+), 2 deletions(-)



diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7646214..2d60712 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-12-06  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* gdbtypes.h: Define the REFERENCE_SEE_THROUGH_BADNESS value.
+	* gdbtypes.c (rank_one_type): Use REFERENCE_SEE_THROUGH_BADNESS
+	for ranking see-through reference cases.
+
 2019-12-06  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 	* stack.c (faas_command): Check a command is provided.
 	* thread.c (taas_command, tfaas_command): Likewise.
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 4854f49..e226cb7 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -60,6 +60,7 @@
 const struct rank BOOL_CONVERSION_BADNESS = {3,0};
 const struct rank BASE_CONVERSION_BADNESS = {2,0};
 const struct rank REFERENCE_CONVERSION_BADNESS = {2,0};
+const struct rank REFERENCE_SEE_THROUGH_BADNESS = {0,1};
 const struct rank NULL_POINTER_CONVERSION_BADNESS = {2,0};
 const struct rank NS_POINTER_CONVERSION_BADNESS = {10,0};
 const struct rank NS_INTEGER_POINTER_CONVERSION_BADNESS = {3,0};
@@ -4338,10 +4339,10 @@
 
   if (TYPE_IS_REFERENCE (arg))
     return (sum_ranks (rank_one_type (parm, TYPE_TARGET_TYPE (arg), NULL),
-                       REFERENCE_CONVERSION_BADNESS));
+                       REFERENCE_SEE_THROUGH_BADNESS));
   if (TYPE_IS_REFERENCE (parm))
     return (sum_ranks (rank_one_type (TYPE_TARGET_TYPE (parm), arg, NULL),
-                       REFERENCE_CONVERSION_BADNESS));
+                       REFERENCE_SEE_THROUGH_BADNESS));
   if (overload_debug)
   /* Debugging only.  */
     fprintf_filtered (gdb_stderr, 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 0dd7333..a1d95e0 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2105,6 +2105,7 @@
 /* * Badness of converting from non-reference to reference.  Subrank
    is the type of reference conversion being done.  */
 extern const struct rank REFERENCE_CONVERSION_BADNESS;
+extern const struct rank REFERENCE_SEE_THROUGH_BADNESS;
 /* * Conversion to rvalue reference.  */
 #define REFERENCE_CONVERSION_RVALUE 1
 /* * Conversion to const lvalue reference.  */
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 79b124b..adbbd9c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2019-12-06  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* gdb.cp/rvalue-ref-overload.cc: Add a case that involves both
+	CV and reference conversion for overload resolution.
+	* gdb.cp/rvalue-ref-overload.exp: Test it.
+
 2019-12-06  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* gdb.threads/pthreads.exp: Test taas and tfaas without command.
diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc b/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
index fa6cab0..e3111d5 100644
--- a/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
+++ b/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
@@ -35,6 +35,8 @@
 
   int overload1arg (foo_lval_ref);
   int overload1arg (foo_rval_ref);
+  int overloadConst (const foo &);
+  int overloadConst (const foo &&);
 };
 
 void
@@ -71,6 +73,11 @@
   // result = 1 + 2 + 3 + 3 = 9
   int result = f (i) + f (ci) + f (0) + f (std::move (i));
 
+  /* Overload resolution below requires both a CV-conversion
+     and reference conversion.  */
+  int test_const // = 3
+    = foo_rr_instance1.overloadConst (arg);
+
   marker1 (); // marker1-returns-here
   return result;
 }
@@ -84,3 +91,5 @@
 
 int foo::overload1arg (foo_lval_ref arg)           { return 1; }
 int foo::overload1arg (foo_rval_ref arg)           { return 2; }
+int foo::overloadConst (const foo &arg)            { return 3; }
+int foo::overloadConst (const foo &&arg)           { return 4; }
diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp b/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
index 61f81b4..693c7ca 100644
--- a/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
+++ b/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
@@ -49,6 +49,8 @@
 	{ method public "~foo();" }
 	{ method public "int overload1arg(foo_lval_ref);" }
 	{ method public "int overload1arg(foo_rval_ref);" }
+	{ method public "int overloadConst(const foo &);" }
+	{ method public "int overloadConst(const foo &&);" }
     }
 
 gdb_test "print foo_rr_instance1.overload1arg(arg)" \
@@ -59,6 +61,8 @@
     "\\$\[0-9\]+ = 2" \
     "print call overloaded func foo && arg"
 
+gdb_test "print foo_rr_instance1.overloadConst(arg)" "3"
+
 # Test lvalue vs rvalue function overloads
 gdb_test "print f (i)" "= 1" "lvalue reference overload"
 

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
Gerrit-Change-Number: 617
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Keith Seitz <keiths@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newpatchset

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

* [pushed] gdb: fix overload resolution for see-through references
  2019-11-12 15:23 [review] gdb: fix overload resolution for see-through references Tankut Baris Aktemur (Code Review)
                   ` (3 preceding siblings ...)
  2019-12-05 20:13 ` Tom Tromey (Code Review)
@ 2019-12-06  7:25 ` Sourceware to Gerrit sync (Code Review)
  2019-12-06  7:25 ` Sourceware to Gerrit sync (Code Review)
  5 siblings, 0 replies; 7+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2019-12-06  7:25 UTC (permalink / raw)
  To: Tankut Baris Aktemur, gdb-patches; +Cc: Tom Tromey, Simon Marchi, Keith Seitz

Sourceware to Gerrit sync has submitted this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/617
......................................................................

gdb: fix overload resolution for see-through references

The overload resolution mechanism assigns badness values to the
necessary conversions to be made on types to pick a champion.  A
badness value consists of a "rank" that scores the conversion and a
"subrank" to differentiate conversions of the same kind.

An auxiliary function, 'sum_ranks', is used for adding two badness
values.  In all of its uses, except two, 'sum_ranks' is used for
populating the subrank of a badness value.  The two exceptions are in
'rank_one_type':

~~~
  /* See through references, since we can almost make non-references
     references.  */

  if (TYPE_IS_REFERENCE (arg))
    return (sum_ranks (rank_one_type (parm, TYPE_TARGET_TYPE (arg), NULL),
		       REFERENCE_CONVERSION_BADNESS));
  if (TYPE_IS_REFERENCE (parm))
    return (sum_ranks (rank_one_type (TYPE_TARGET_TYPE (parm), arg, NULL),
		       REFERENCE_CONVERSION_BADNESS));
~~~

Here, the result of a recursive call is combined with
REFERENCE_CONVERSION_BADNESS.  This leads to the problem of
over-punishment by combining two ranks.  Consider this:

    void an_overloaded_function (const foo &);
    void an_overloaded_function (const foo &&);
    ...
    foo arg;
    an_overloaded_function(arg);

When ranking 'an_overloaded_function (const foo &)', the badness
values REFERENCE_CONVERSION_BADNESS and CV_CONVERSION_BADNESS are
combined, whereas 'rank_one_type' assigns only the
REFERENCE_CONVERSION_BADNESS value to 'an_overloaded_function (const
foo &&)' (there is a different execution flow for that).  This yields
in GDB picking the latter function as the overload champion instead of
the former.

In fact, the 'rank_one_type' function should have given
'an_overloaded_function (const foo &)' the CV_CONVERSION_BADNESS
value, with the see-through referencing increasing the subrank a
little bit.  This can be achieved by introducing a new badness value,
REFERENCE_SEE_THROUGH_BADNESS, which bumps up the subrank only, and
using it in the two "exceptional" cases of 'sum_ranks'.

gdb/ChangeLog:
2019-12-06  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdbtypes.h: Define the REFERENCE_SEE_THROUGH_BADNESS value.
	* gdbtypes.c (rank_one_type): Use REFERENCE_SEE_THROUGH_BADNESS
	for ranking see-through reference cases.

gdb/testsuite/ChangeLog:
2019-12-06  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* gdb.cp/rvalue-ref-overload.cc: Add a case that involves both
	CV and reference conversion for overload resolution.
	* gdb.cp/rvalue-ref-overload.exp: Test it.

Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
---
M gdb/ChangeLog
M gdb/gdbtypes.c
M gdb/gdbtypes.h
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
M gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
6 files changed, 29 insertions(+), 2 deletions(-)


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7646214..2d60712 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-12-06  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* gdbtypes.h: Define the REFERENCE_SEE_THROUGH_BADNESS value.
+	* gdbtypes.c (rank_one_type): Use REFERENCE_SEE_THROUGH_BADNESS
+	for ranking see-through reference cases.
+
 2019-12-06  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 	* stack.c (faas_command): Check a command is provided.
 	* thread.c (taas_command, tfaas_command): Likewise.
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 4854f49..e226cb7 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -60,6 +60,7 @@
 const struct rank BOOL_CONVERSION_BADNESS = {3,0};
 const struct rank BASE_CONVERSION_BADNESS = {2,0};
 const struct rank REFERENCE_CONVERSION_BADNESS = {2,0};
+const struct rank REFERENCE_SEE_THROUGH_BADNESS = {0,1};
 const struct rank NULL_POINTER_CONVERSION_BADNESS = {2,0};
 const struct rank NS_POINTER_CONVERSION_BADNESS = {10,0};
 const struct rank NS_INTEGER_POINTER_CONVERSION_BADNESS = {3,0};
@@ -4338,10 +4339,10 @@
 
   if (TYPE_IS_REFERENCE (arg))
     return (sum_ranks (rank_one_type (parm, TYPE_TARGET_TYPE (arg), NULL),
-                       REFERENCE_CONVERSION_BADNESS));
+                       REFERENCE_SEE_THROUGH_BADNESS));
   if (TYPE_IS_REFERENCE (parm))
     return (sum_ranks (rank_one_type (TYPE_TARGET_TYPE (parm), arg, NULL),
-                       REFERENCE_CONVERSION_BADNESS));
+                       REFERENCE_SEE_THROUGH_BADNESS));
   if (overload_debug)
   /* Debugging only.  */
     fprintf_filtered (gdb_stderr, 
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 0dd7333..a1d95e0 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -2105,6 +2105,7 @@
 /* * Badness of converting from non-reference to reference.  Subrank
    is the type of reference conversion being done.  */
 extern const struct rank REFERENCE_CONVERSION_BADNESS;
+extern const struct rank REFERENCE_SEE_THROUGH_BADNESS;
 /* * Conversion to rvalue reference.  */
 #define REFERENCE_CONVERSION_RVALUE 1
 /* * Conversion to const lvalue reference.  */
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 79b124b..adbbd9c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2019-12-06  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* gdb.cp/rvalue-ref-overload.cc: Add a case that involves both
+	CV and reference conversion for overload resolution.
+	* gdb.cp/rvalue-ref-overload.exp: Test it.
+
 2019-12-06  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* gdb.threads/pthreads.exp: Test taas and tfaas without command.
diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc b/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
index fa6cab0..e3111d5 100644
--- a/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
+++ b/gdb/testsuite/gdb.cp/rvalue-ref-overload.cc
@@ -35,6 +35,8 @@
 
   int overload1arg (foo_lval_ref);
   int overload1arg (foo_rval_ref);
+  int overloadConst (const foo &);
+  int overloadConst (const foo &&);
 };
 
 void
@@ -71,6 +73,11 @@
   // result = 1 + 2 + 3 + 3 = 9
   int result = f (i) + f (ci) + f (0) + f (std::move (i));
 
+  /* Overload resolution below requires both a CV-conversion
+     and reference conversion.  */
+  int test_const // = 3
+    = foo_rr_instance1.overloadConst (arg);
+
   marker1 (); // marker1-returns-here
   return result;
 }
@@ -84,3 +91,5 @@
 
 int foo::overload1arg (foo_lval_ref arg)           { return 1; }
 int foo::overload1arg (foo_rval_ref arg)           { return 2; }
+int foo::overloadConst (const foo &arg)            { return 3; }
+int foo::overloadConst (const foo &&arg)           { return 4; }
diff --git a/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp b/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
index 61f81b4..693c7ca 100644
--- a/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
+++ b/gdb/testsuite/gdb.cp/rvalue-ref-overload.exp
@@ -49,6 +49,8 @@
 	{ method public "~foo();" }
 	{ method public "int overload1arg(foo_lval_ref);" }
 	{ method public "int overload1arg(foo_rval_ref);" }
+	{ method public "int overloadConst(const foo &);" }
+	{ method public "int overloadConst(const foo &&);" }
     }
 
 gdb_test "print foo_rr_instance1.overload1arg(arg)" \
@@ -59,6 +61,8 @@
     "\\$\[0-9\]+ = 2" \
     "print call overloaded func foo && arg"
 
+gdb_test "print foo_rr_instance1.overloadConst(arg)" "3"
+
 # Test lvalue vs rvalue function overloads
 gdb_test "print f (i)" "= 1" "lvalue reference overload"
 

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I39ae6505ab85ad0bd21915368c82540ceeb3aae9
Gerrit-Change-Number: 617
Gerrit-PatchSet: 2
Gerrit-Owner: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Keith Seitz <keiths@redhat.com>
Gerrit-Reviewer: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: merged

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

end of thread, other threads:[~2019-12-06  7:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12 15:23 [review] gdb: fix overload resolution for see-through references Tankut Baris Aktemur (Code Review)
2019-11-27  7:46 ` Tankut Baris Aktemur (Code Review)
2019-11-29 16:43 ` Simon Marchi (Code Review)
2019-11-29 18:57 ` Tankut Baris Aktemur (Code Review)
2019-12-05 20:13 ` Tom Tromey (Code Review)
2019-12-06  7:25 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2019-12-06  7:25 ` Sourceware to Gerrit sync (Code Review)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).