Patchwork RFC: fix std::unique_ptr pretty-printer

login
register
mail settings
Submitter Tom Tromey
Date Aug. 10, 2012, 7:49 p.m.
Message ID <874noaa5o9.fsf@fleche.redhat.com>
Download mbox | patch
Permalink /patch/176613/
State New
Headers show

Comments

Tom Tromey - Aug. 10, 2012, 7:49 p.m.
A user reported on irc that the std::unique_ptr pretty-printer yields
bad results.  For example:

(gdb) p uptr
$1 = std::tuple containing = {
  [1] = ,
  [2] = {
    <std::default_delete<datum>> = {<No data fields>}, <No data fields>}
}

This omits the actual pointer and prints some useless stuff instead.

This patch fixes the printer and adds a test.
The new output looks like this:

$11 = std::unique_ptr containing (datum *) 0x6067d0

Let me know what you think.

Tom

b/libstdc++-v3/ChangeLog:
2012-08-10  Tom Tromey  <tromey@redhat.com>

	* testsuite/libstdc++-prettyprinters/cxx11.cc (struct datum):
	New.
	(global): New global.
	(main): Add test for unique_ptr.
	* python/libstdcxx/v6/printers.py
	(UniquePointerPrinter.to_string): Extract the pointer and also
	print its type.
Jonathan Wakely - Aug. 10, 2012, 11:58 p.m.
On 10 August 2012 20:49, Tom Tromey wrote:
> A user reported on irc that the std::unique_ptr pretty-printer yields
> bad results.  For example:
>
> (gdb) p uptr
> $1 = std::tuple containing = {
>   [1] = ,
>   [2] = {
>     <std::default_delete<datum>> = {<No data fields>}, <No data fields>}
> }
>
> This omits the actual pointer and prints some useless stuff instead.
>
> This patch fixes the printer and adds a test.
> The new output looks like this:
>
> $11 = std::unique_ptr containing (datum *) 0x6067d0
>
> Let me know what you think.

It's inconsistent with the other printers in that it prints the stored
type, unlike e.g. std::vector<int> which just says "std::vector of
length ..." but I think that's an improvement.

Personally I'd prefer the element_type as part of the type, e.g.
"std::unique_ptr<datum> = 0x6067d0" but that would be even more
inconsistent!

OK for trunk, thanks.
Tom Tromey - Aug. 13, 2012, 1:31 p.m.
>>>>> "Jonathan" == Jonathan Wakely <jwakely.gcc@gmail.com> writes:

>> $11 = std::unique_ptr containing (datum *) 0x6067d0

Jonathan> It's inconsistent with the other printers in that it prints
Jonathan> the stored type, unlike e.g. std::vector<int> which just says
Jonathan> "std::vector of length ..." but I think that's an improvement.

Yeah... without this bit it was just printing

$11 = std::unique_ptr containing 0x6067d0

Ordinarily, gdb will print the type here; but it doesn't when called
from Python.  I thought the typical output was easier to read.

Jonathan> Personally I'd prefer the element_type as part of the type, e.g.
Jonathan> "std::unique_ptr<datum> = 0x6067d0" but that would be even more
Jonathan> inconsistent!

I can make that change if you'd prefer.
I don't know why, but I didn't even think of it.

Tom
Jonathan Wakely - Aug. 13, 2012, 5:55 p.m.
On 13 August 2012 14:31, Tom Tromey wrote:
>>>>>> "Jonathan" == Jonathan Wakely <jwakely.gcc@gmail.com> writes:
>
>>> $11 = std::unique_ptr containing (datum *) 0x6067d0
>
> Jonathan> It's inconsistent with the other printers in that it prints
> Jonathan> the stored type, unlike e.g. std::vector<int> which just says
> Jonathan> "std::vector of length ..." but I think that's an improvement.
>
> Yeah... without this bit it was just printing
>
> $11 = std::unique_ptr containing 0x6067d0
>
> Ordinarily, gdb will print the type here; but it doesn't when called
> from Python.  I thought the typical output was easier to read.
>
> Jonathan> Personally I'd prefer the element_type as part of the type, e.g.
> Jonathan> "std::unique_ptr<datum> = 0x6067d0" but that would be even more
> Jonathan> inconsistent!
>
> I can make that change if you'd prefer.
> I don't know why, but I didn't even think of it.

I prefer it as unique_ptr<datum> but I'm probably not your typical
user of the pretty printers, so if anyone else has an opinion please
share it.

Patch

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 4520f32..dcb98de 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -71,7 +71,8 @@  class UniquePointerPrinter:
         self.val = val
 
     def to_string (self):
-        return self.val['_M_t']
+        v = self.val['_M_t']['_M_head_impl']
+        return 'std::unique_ptr containing (' + str(v.type) + ') ' + str(v)
 
 class StdListPrinter:
     "Print a std::list"
diff --git a/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc b/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
index 54b3275..7e2e240 100644
--- a/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
+++ b/libstdc++-v3/testsuite/libstdc++-prettyprinters/cxx11.cc
@@ -48,6 +48,14 @@  use(const T &container)
     placeholder(*i);
 }
 
+struct datum
+{
+  std::string s;
+  int i;
+};
+
+std::unique_ptr<datum> global;
+
 int
 main()
 {
@@ -86,6 +94,11 @@  main()
   uoms.insert(5);
 // { dg-final { note-test uoms {std::unordered_multiset with 1 elements = {[0] = 5}} } }
 
+  std::unique_ptr<datum> uptr (new datum);
+  uptr->s = "hi bob";
+  uptr->i = 23;
+// { dg-final { regexp-test uptr {std::unique_ptr containing .datum .. 0x.*} } }
+
   placeholder(""); // Mark SPOT
   use(efl);
   use(fl);
@@ -93,6 +106,8 @@  main()
   use(eumm);
   use(eus);
   use(eums);
+  use(uoms);
+  use(uptr->s);
 
   return 0;
 }