[patch] Support C++11 rvalue (move constructor)

[patch] Support C++11 rvalue (move constructor)

[Jan Kratochvil]

Hi,

currently one cannot debug C++11 move constructors, one gets:

# in C::C (C &&o)
(gdb) print o
$1 = <unknown type in .../testsuite/gdb.cp/rvalue, CU 0x0, DIE 0x1f6c>

This fix is easy but it handles && rvalue reference like & lvalue reference.
Therefore there is a KFAIL in the testcase for it.  I do not think GDB needs
to handle && differently - except for displaying && instead of &.

Implementing proper && into inferior type system means:

(1) Either add TYPE_CODE_RVALUE_REF besides TYPE_CODE_REF which is pretty
    tedious - TYPE_CODE_REF is referenced in 155 GDB LoCs - due to missing
    types virtualization.  In almost all cases one will do:
-	if (TYPE_CODE (t) == TYPE_CODE_REF)
+	if (TYPE_CODE (t) == TYPE_CODE_REF
+	    || TYPE_CODE (t) == TYPE_CODE_RVALUE_REF)

    or to add new main_type::flag_ref_is_rvalue valid only for TYPE_CODE_REF
    but that is sure not a clean implementation.

(2) One would need to add type.rvalue_reference_type
    (besides type.reference_type) to prevent leaks of types.
    This would mean increasing the current sizeof (struct type) 40 -> 48.
    The proper solution is to rather implement reference counting or garbage
    collecting of types and merge struct type into struct main_type.

Anyway currently I find the patch below as a good enough hack as currently
I find it a bit blocker when using C++11.


Thanks,
Jan


gdb/
2013-10-12  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR c++/16043
	* c-exp.y (ptr_operator): Handle also ANDAND as tp_reference.
	* dwarf2read.c (process_die): Skip also DW_TAG_rvalue_reference_type.
	(read_type_die_1): Handle also DW_TAG_rvalue_reference_type.
	* gdbtypes.h (enum type_code): Extend comment for TYPE_CODE_REF.
	(struct type): Extend comment for reference_type.

gdb/testsuite/
2013-10-12  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.cp/rvalue.cc: New file.
	* gdb.cp/rvalue.exp: New file.

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 77713dd..653a5ad 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -1115,6 +1115,10 @@ ptr_operator:
 			{ insert_type (tp_reference); }
 	|	'&' ptr_operator
 			{ insert_type (tp_reference); }
+	|	ANDAND
+			{ insert_type (tp_reference); }
+	|	ANDAND ptr_operator
+			{ insert_type (tp_reference); }
 	;
 
 ptr_operator_ts: ptr_operator
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index aa109e0..47b1cb5 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -7990,6 +7990,7 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
     case DW_TAG_pointer_type:
     case DW_TAG_ptr_to_member_type:
     case DW_TAG_reference_type:
+    case DW_TAG_rvalue_reference_type:
     case DW_TAG_string_type:
       break;
 
@@ -18010,6 +18011,7 @@ read_type_die_1 (struct die_info *die, struct dwarf2_cu *cu)
       this_type = read_tag_ptr_to_member_type (die, cu);
       break;
     case DW_TAG_reference_type:
+    case DW_TAG_rvalue_reference_type:
       this_type = read_tag_reference_type (die, cu);
       break;
     case DW_TAG_const_type:
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 5e8d1e7..d5e2b7a 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -129,7 +129,8 @@ enum type_code
        by the Itanium C++ ABI (used by GCC on all platforms).  */
     TYPE_CODE_MEMBERPTR,
 
-    TYPE_CODE_REF,		/* C++ Reference types */
+    TYPE_CODE_REF,		/* C++ Reference types
+				   (both lvalue & and rvalue &&) */
 
     TYPE_CODE_CHAR,		/* *real* character type */
 
@@ -657,7 +658,7 @@ struct type
 
   struct type *pointer_type;
 
-  /* C++: also need a reference type.  */
+  /* C++: also need a reference type (both for lvalue & and rvalue &&).  */
 
   struct type *reference_type;
 
diff --git a/gdb/testsuite/gdb.cp/rvalue.cc b/gdb/testsuite/gdb.cp/rvalue.cc
new file mode 100644
index 0000000..7a78b97
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/rvalue.cc
@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+#include <memory>
+
+class C
+{
+public:
+  C () {}
+  C (C &&o);
+  int i = 42;
+};
+
+C::C (C &&o)
+{
+  i = o.i; /* o-is-ready */
+}
+
+int
+main()
+{
+  C a;
+  C b (std::move (a)); /* a-is-ready */
+}
diff --git a/gdb/testsuite/gdb.cp/rvalue.exp b/gdb/testsuite/gdb.cp/rvalue.exp
new file mode 100644
index 0000000..df3536b
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/rvalue.exp
@@ -0,0 +1,53 @@
+# Copyright 2013 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/>.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing $testfile.exp $testfile \
+			 $srcfile {debug c++ additional_flags=-std=c++11}]} {
+    return -1
+}
+
+if ![runto_main] {
+    retur -1
+}
+
+gdb_breakpoint [gdb_get_line_number "a-is-ready"]
+gdb_continue_to_breakpoint "a-is-ready" ".* a-is-ready .*"
+
+set test "print (struct C &&) a"
+gdb_test_multiple $test $test {
+    -re " = \\(C &&\\) @0x\[0-9a-f\]+: {i = 42}\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re " = \\(C &\\) @0x\[0-9a-f\]+: {i = 42}\r\n$gdb_prompt $" {
+	kfail c++/16043 $test
+    }
+}
+
+gdb_breakpoint [gdb_get_line_number "o-is-ready"]
+gdb_continue_to_breakpoint "o-is-ready" ".* o-is-ready .*"
+
+set test "print o"
+gdb_test_multiple $test $test {
+    -re " = \\(C &&\\) @0x\[0-9a-f\]+: {i = 42}\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re " = \\(C &\\) @0x\[0-9a-f\]+: {i = 42}\r\n$gdb_prompt $" {
+	kfail c++/16043 $test
+    }
+}

Re: [patch] Support C++11 rvalue (move constructor)

[Tom Tromey]

CCing Jonathan Wakely.

Jan> currently one cannot debug C++11 move constructors, one gets:

This is PR 14441.  Also see the thread:
https://sourceware.org/ml/gdb/2012-10/msg00108.html

I think Jonathan started a branch for this...

Jan> Therefore there is a KFAIL in the testcase for it.  I do not think
Jan> GDB needs to handle && differently - except for displaying &&
Jan> instead of &.

Is that also true for inferior calls?
I didn't look.

Jan>     or to add new main_type::flag_ref_is_rvalue valid only for
Jan>     TYPE_CODE_REF but that is sure not a clean implementation.

If the semantics of the two kinds of references are sufficiently
similar, then I don't think it is actually so bad.

Jan> (2) One would need to add type.rvalue_reference_type (besides
Jan> type.reference_type) to prevent leaks of types.  This would mean
Jan> increasing the current sizeof (struct type) 40 -> 48.  The proper
Jan> solution is to rather implement reference counting or garbage
Jan> collecting of types and merge struct type into struct main_type.

Maybe the size increase isn't that important.
Alternatively maybe it can be handled like qualifiers.

Jan> Anyway currently I find the patch below as a good enough hack as currently
Jan> I find it a bit blocker when using C++11.

It seems reasonable to me -- an improvement, and not blocking any future
improvements.

Tom

Re: [patch] Support C++11 rvalue (move constructor)

[Jonathan Wakely]

On 14 October 2013 17:10, Tom Tromey wrote:
> CCing Jonathan Wakely.
>
> Jan> currently one cannot debug C++11 move constructors, one gets:
>
> This is PR 14441.  Also see the thread:
> https://sourceware.org/ml/gdb/2012-10/msg00108.html
>
> I think Jonathan started a branch for this...

I have a local Git branch on my home machine, and IIRC I got some uses
of rvalue references displaying correctly, but not all. When I tried
to do something more invasive like Jan's suggestion (1) I got a bit
bogged down learning the GDB code, and never finished the job.

Re: [patch] Support C++11 rvalue (move constructor)

[Jan Kratochvil]

On Mon, 14 Oct 2013 18:10:20 +0200, Tom Tromey wrote:
> Jan> Therefore there is a KFAIL in the testcase for it.  I do not think
> Jan> GDB needs to handle && differently - except for displaying &&
> Jan> instead of &.
> 
> Is that also true for inferior calls?
> I didn't look.

GDB cannot call constructors so this is irrelevant now.
But it looks GCC really passes just a pointer (=like with lvalue reference).


> Maybe the size increase isn't that important.

I always thought the opposite is true.
Due to CU expansion with <tab> after some completions one easily gets to
1GB GDB and more (but IMO this is a bug <tab> should not expand CUs).


> Alternatively maybe it can be handled like qualifiers.

Interesting idea but I would rather go some more clean way.


> It seems reasonable to me -- an improvement, and not blocking any future
> improvements.

I will check it in in some days.


Thanks,
Jan

Re: [patch] Support C++11 rvalue (move constructor)

[Tom Tromey]

>> Is that also true for inferior calls?
>> I didn't look.

Jan> GDB cannot call constructors so this is irrelevant now.

I'm not sure constructors matter.  rvalue references affect overloading,
e.g.:

    #include <stdio.h>

    int ov(int &x) { return 0; }
    int ov(int &&x) { return 1; }

    int main() {
      int z = 23;
      printf ("%d %d\n", ov(z), ov(23));
    }


Tom> Maybe the size increase isn't that important.

Jan> I always thought the opposite is true.
Jan> Due to CU expansion with <tab> after some completions one easily gets to
Jan> 1GB GDB and more (but IMO this is a bug <tab> should not expand CUs).

I think what's missing is an idea of the amount that struct main_type
contributes.

My recollection is that I concluded that shrinking types wasn't
worthwhile.  However, it's worthwhile to redo the experiment, at least
if you plan to completely fix this problem.

Tom

Re: [patch] Support C++11 rvalue (move constructor)

[Jan Kratochvil]

On Mon, 14 Oct 2013 20:18:51 +0200, Tom Tromey wrote:
> >> Is that also true for inferior calls?
> >> I didn't look.
> 
> Jan> GDB cannot call constructors so this is irrelevant now.
> 
> I'm not sure constructors matter.  rvalue references affect overloading,
> e.g.:
> 
>     #include <stdio.h>
> 
>     int ov(int &x) { return 0; }
>     int ov(int &&x) { return 1; }
> 
>     int main() {
>       int z = 23;
>       printf ("%d %d\n", ov(z), ov(23));
>     }

OK, true.  I see && already works fine due to the C++11 libiberty/ demangler.
(gdb) complete p 'ov
p 'ov(int&&)
p 'ov(int&)

Just inferior calls pick randomly the function depending on their order in
source (in DWARF):
(gdb) p ov(z)
$1 = 0
vs.
(gdb) p ov(z)
$1 = 1

This means we should really have two distinct TYPE_CODEs and then properly
select lvalue vs. rvalue function depending on the type of argument used passed
for the inferior call.  Possibly failing to find the function if function only
accepts rvalue but user passes in real variable (lvalue).

The patch is definitely incorrect as is but I would say it is better than
nothing.  The proper rvalue implementation is some work and I would say I have
other work to do.


> Tom> Maybe the size increase isn't that important.
> 
> Jan> I always thought the opposite is true.
> Jan> Due to CU expansion with <tab> after some completions one easily gets to
> Jan> 1GB GDB and more (but IMO this is a bug <tab> should not expand CUs).
> 
> I think what's missing is an idea of the amount that struct main_type
> contributes.
> 
> My recollection is that I concluded that shrinking types wasn't
> worthwhile.  However, it's worthwhile to redo the experiment, at least
> if you plan to completely fix this problem.

I find clear the <tab> CU expansion should be fixed.  Then maybe GDB stops
growing.  Or maybe not (and then one has to look more at main_type...).


Jan