Re: Smart pointer pretty printers

[Juraj Oršulić]

Hi Jonathan,

Thanks for looking into this! It’s unfortunate that we can't know if the
pointer in the iterator is legal to dereference. Nevertheless, inspecting
smart pointers is a far more common use case, it's good that it has finally
been sorted out.

Thanks, Juraj


On Tue, 9 Jan 2018 at 23:10, Jonathan Wakely  wrote:

> On 09/01/18 18:50 +, Jonathan Wakely wrote:
> >On 09/01/18 15:02 +, Jonathan Wakely wrote:
> >>On 09/01/18 14:59 +, Jonathan Wakely wrote:
> >>>On 04/01/18 11:22 +0100, Juraj Oršulić wrote:
> Hi Jonathan (and the libstdc++ list). Can we revive this? I sent the
> patches for improving the smart pointer pretty printers in March. They
> haven't been reviewed.
> >>>
> >>>Thanks for the reminder. I'm testing the attached patch, which has
> >>>been rebased on trunk, but I'm getting test failures for the
> >>>shared_ptr printers.
> >>>
> >>>I can't see any difference between the expected output and what GDB
> >>>prints, so I think it's some DejaGnu or Tcl/Tk oddity. The actual
> >>>printer seems to work OK.
> >>
> >>No, the problem is that I'm just misreading the results.
> >>
> >>I'll finish testing and commit this soon.
> >
> >I changed the code slightly, so it still works for GDB versions older
> >than 7.5, which don't have the fix to allow children() to return a
> >list. There's no easy way to tell if the GDB interpreting the code has
> >the fix or not.
> >
> >Thanks very much for the patches, and sorry for the delay reviewing
> >them.
> >
> >Tested x86_64-linux, committed to trunk.
>
> I've just realised I didn't put your name in the ChangeLog, sorry!
> (I semi-automate my changelog entries, so have to remember to do an
> extra step when I'm not the author, or not the sole author).
>
> Fixed with this patch, committed to trunk.
>
>
>
>

Re: Smart pointer pretty printers

[Jonathan Wakely]

On 09/01/18 18:50 +, Jonathan Wakely wrote:

On 09/01/18 15:02 +, Jonathan Wakely wrote:

On 09/01/18 14:59 +, Jonathan Wakely wrote:

On 04/01/18 11:22 +0100, Juraj Oršulić wrote:

Hi Jonathan (and the libstdc++ list). Can we revive this? I sent the
patches for improving the smart pointer pretty printers in March. They
haven't been reviewed.


Thanks for the reminder. I'm testing the attached patch, which has
been rebased on trunk, but I'm getting test failures for the
shared_ptr printers.

I can't see any difference between the expected output and what GDB
prints, so I think it's some DejaGnu or Tcl/Tk oddity. The actual
printer seems to work OK.


No, the problem is that I'm just misreading the results.

I'll finish testing and commit this soon.


I changed the code slightly, so it still works for GDB versions older
than 7.5, which don't have the fix to allow children() to return a
list. There's no easy way to tell if the GDB interpreting the code has
the fix or not.

Thanks very much for the patches, and sorry for the delay reviewing
them.

Tested x86_64-linux, committed to trunk.


I've just realised I didn't put your name in the ChangeLog, sorry!
(I semi-automate my changelog entries, so have to remember to do an
extra step when I'm not the author, or not the sole author).

Fixed with this patch, committed to trunk.



commit b4e59f03b68387c5b556f3d8430ece8927cdca9f
Author: Jonathan Wakely 
Date:   Tue Jan 9 22:01:20 2018 +

Correct earlier ChangeLog entry

Add Juraj Oršulić as original patch author.

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 6c5a2741ba8..88af8ecf37c 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -32,7 +32,8 @@
 	* testsuite/23_containers/unordered_map/insert/83709.cc: New.
 	* testsuite/23_containers/unordered_set/insert/83709.cc: New.
 
-2018-01-09  Jonathan Wakely  
+2018-01-09  Juraj Oršulić  
+	Jonathan Wakely  
 
 	PR libstdc++/59253 (partial)
 	* python/libstdcxx/v6/printers.py (SmartPtrIterator): Common iterator

Re: Smart pointer pretty printers

[Jonathan Wakely]

On 09/01/18 15:02 +, Jonathan Wakely wrote:

On 09/01/18 14:59 +, Jonathan Wakely wrote:

On 04/01/18 11:22 +0100, Juraj Oršulić wrote:

Hi Jonathan (and the libstdc++ list). Can we revive this? I sent the
patches for improving the smart pointer pretty printers in March. They
haven't been reviewed.


Thanks for the reminder. I'm testing the attached patch, which has
been rebased on trunk, but I'm getting test failures for the
shared_ptr printers.

I can't see any difference between the expected output and what GDB
prints, so I think it's some DejaGnu or Tcl/Tk oddity. The actual
printer seems to work OK.


No, the problem is that I'm just misreading the results.

I'll finish testing and commit this soon.


I changed the code slightly, so it still works for GDB versions older
than 7.5, which don't have the fix to allow children() to return a
list. There's no easy way to tell if the GDB interpreting the code has
the fix or not.

Thanks very much for the patches, and sorry for the delay reviewing
them.

Tested x86_64-linux, committed to trunk.


commit 4c65ef4dae877443c5372ec504e3881a3922dc18
Author: Jonathan Wakely 
Date:   Tue Jan 9 15:36:13 2018 +

PR libstdc++/59253 Improve pretty printers for smart pointers

PR libstdc++/59253 (partial)
* python/libstdcxx/v6/printers.py (SmartPtrIterator): Common iterator
type for pointer stored by shared_ptr, weak_ptr and unique_ptr.
(SharedPointerPrinter, UniquePointerPrinter): Treat stored values as
children.
* testsuite/libstdc++-prettyprinters/cxx11.cc: Update expected output
of unique_ptr printer.
* testsuite/libstdc++-prettyprinters/shared_ptr.cc: Update expected
output of shared_ptr printer.

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 6da8508d944..e9f7359d63f 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -114,12 +114,31 @@ def strip_versioned_namespace(typename):
 return typename.replace(_versioned_namespace, '')
 return typename
 
+class SmartPtrIterator(Iterator):
+"An iterator for smart pointer types with a single 'child' value"
+
+def __init__(self, val):
+self.val = val
+
+def __iter__(self):
+return self
+
+def __next__(self):
+if self.val is None:
+raise StopIteration
+self.val, val = None, self.val
+return ('get()', val)
+
 class SharedPointerPrinter:
 "Print a shared_ptr or weak_ptr"
 
 def __init__ (self, typename, val):
 self.typename = strip_versioned_namespace(typename)
 self.val = val
+self.pointer = val['_M_ptr']
+
+def children (self):
+return SmartPtrIterator(self.pointer)
 
 def to_string (self):
 state = 'empty'
@@ -128,27 +147,29 @@ class SharedPointerPrinter:
 usecount = refcounts['_M_use_count']
 weakcount = refcounts['_M_weak_count']
 if usecount == 0:
-state = 'expired, weak %d' % weakcount
+state = 'expired, weak count %d' % weakcount
 else:
-state = 'count %d, weak %d' % (usecount, weakcount - 1)
-return '%s (%s) %s' % (self.typename, state, self.val['_M_ptr'])
+state = 'use count %d, weak count %d' % (usecount, weakcount - 1)
+return '%s<%s> (%s)' % (self.typename, str(self.pointer.type.target().strip_typedefs()), state)
 
 class UniquePointerPrinter:
 "Print a unique_ptr"
 
 def __init__ (self, typename, val):
 self.val = val
+impl_type = val.type.fields()[0].type.tag
+if is_specialization_of(impl_type, '__uniq_ptr_impl'): # New implementation
+self.pointer = val['_M_t']['_M_t']['_M_head_impl']
+elif is_specialization_of(impl_type, 'tuple'):
+self.pointer = val['_M_t']['_M_head_impl']
+else:
+raise ValueError("Unsupported implementation for unique_ptr: %s" % impl_type)
+
+def children (self):
+return SmartPtrIterator(self.pointer)
 
 def to_string (self):
-impl_type = self.val.type.fields()[0].type.tag
-if is_specialization_of(impl_type, '__uniq_ptr_impl'): # New implementation
-v = self.val['_M_t']['_M_t']['_M_head_impl']
-elif is_specialization_of(impl_type, 'tuple'):
-v = self.val['_M_t']['_M_head_impl']
-else:
-raise ValueError("Unsupported implementation for unique_ptr: %s" % self.val.type.fields()[0].type.tag)
-return 'std::unique_ptr<%s> containing %s' % (str(v.type.target()),
-  str(v))
+return ('std::unique_ptr<%s>' % (str(self.pointer.type.target(
 
 def get_value_from_aligned_membuf(buf, valtype):
 """Returns the value held in a 

Re: Smart pointer pretty printers

[Jonathan Wakely]

On 02/03/17 19:10 +0100, Juraj Oršulić wrote:

On Fri, Feb 24, 2017 at 5:36 PM, Jonathan Wakely  wrote:

For a patch this size we do, so I'm not going to look at your patch.
It will probably be simpler to just do it myself, and not worry about
paperwork.

If you want to contribue in future then please do complete the
necessary paperwork anyway:
https://gcc.gnu.org/onlinedocs/libstdc++/manual/appendix_contributing.html#contrib.list


Hi Jonathan,

I have completed the assignment.

I am resubmitting the patch from the last time, this time as an
attachment (since the previous was broken by newlines). As before,
there are two versions of the patch: the one that I obtained by editing
printers.py on Ubuntu 16.04, which ships with gcc 5.4.0; and the
other, for the current printers.py from the gcc repo (I assume it's gcc
6?), which needs a slightly different patch because the unique_ptr
printer has significantly changed.

I have been using the gcc 5 patch without problems.  I have not tested
the gcc 6 patch, but since it is very similar, I don't expect any
problems.

Let me know if you want me to expand this for other iterators.


I don't like the change for std::vector iterators, because it makes it
appear as though the iterator stores a pointer. Although that's true
internally, conceptually it's not how we should think about iterators.

An iterator refers to a value, not a pointer. However, the current
code has a serious flaw that it always dereferences the pointer, even
for the end iterator (which is https://gcc.gnu.org/PR59170 and is
worse than displaying the value as a pointer!)

There's certainly scope for improvement, but I'm not sure this is the
right solution.



@@ -322,9 +328,17 @@ class StdVectorIteratorPrinter:

def __init__(self, typename, val):
self.val = val
+self.pointer = self.val['_M_current']
+
+def children(self):
+if not self.pointer:
+return []
+return [('operator->()', self.pointer)]

def to_string(self):
-return self.val['_M_current'].dereference()
+if not self.pointer:
+ return 'non-dereferenceable iterator for std::vector'
+return ('std::vector<%s>::iterator' % 
(str(self.pointer.type.target(

class StdTuplePrinter:
"Print a std::tuple"

Re: Smart pointer pretty printers

[Jonathan Wakely]

On 09/01/18 14:59 +, Jonathan Wakely wrote:

On 04/01/18 11:22 +0100, Juraj Oršulić wrote:

Hi Jonathan (and the libstdc++ list). Can we revive this? I sent the
patches for improving the smart pointer pretty printers in March. They
haven't been reviewed.


Thanks for the reminder. I'm testing the attached patch, which has
been rebased on trunk, but I'm getting test failures for the
shared_ptr printers.

I can't see any difference between the expected output and what GDB
prints, so I think it's some DejaGnu or Tcl/Tk oddity. The actual
printer seems to work OK.


No, the problem is that I'm just misreading the results.

I'll finish testing and commit this soon.

Re: Smart pointer pretty printers

[Jonathan Wakely]

On 04/01/18 11:22 +0100, Juraj Oršulić wrote:

Hi Jonathan (and the libstdc++ list). Can we revive this? I sent the
patches for improving the smart pointer pretty printers in March. They
haven't been reviewed.


Thanks for the reminder. I'm testing the attached patch, which has
been rebased on trunk, but I'm getting test failures for the
shared_ptr printers.

I can't see any difference between the expected output and what GDB
prints, so I think it's some DejaGnu or Tcl/Tk oddity. The actual
printer seems to work OK.



Regards, and happy 2018,
Juraj

On Mon, Mar 27, 2017 at 6:05 PM, Jonathan Wakely  wrote:

On 27/03/17 17:34 +0200, Juraj Oršulić wrote:

On Mon, Mar 27, 2017 at 5:15 PM, Jonathan Wakely  wrote:

I think we're probably too close to the gcc7 release to make this
change now, sorry.


No problem, I think it's most important that it entered the pipeline,
since it's been completely dead for 4 years :-)

Just a reminder, I added this only for those smart pointers that I
needed at the moment - the unique and shared pointer, and for the
vector iterator. When you (or someone else) will be doing a review of
the patch, let me know if you think this should be expanded for other
iterators as well.


Will do, thanks.

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 6da8508d944..7b999ee6db4 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -120,6 +120,10 @@ class SharedPointerPrinter:
 def __init__ (self, typename, val):
 self.typename = strip_versioned_namespace(typename)
 self.val = val
+self.pointer = val['_M_ptr']
+
+def children (self):
+return [('get()', self.pointer)]
 
 def to_string (self):
 state = 'empty'
@@ -130,25 +134,29 @@ class SharedPointerPrinter:
 if usecount == 0:
 state = 'expired, weak %d' % weakcount
 else:
-state = 'count %d, weak %d' % (usecount, weakcount - 1)
-return '%s (%s) %s' % (self.typename, state, self.val['_M_ptr'])
+state = 'use count %d, weak count %d' % (usecount, weakcount - 1)
+return '%s<%s> (%s)' % (self.typename, str(self.pointer.type.target().strip_typedefs()), state)
 
 class UniquePointerPrinter:
 "Print a unique_ptr"
 
 def __init__ (self, typename, val):
 self.val = val
-
-def to_string (self):
-impl_type = self.val.type.fields()[0].type.tag
+impl_type = val.type.fields()[0].type.tag
 if is_specialization_of(impl_type, '__uniq_ptr_impl'): # New implementation
-v = self.val['_M_t']['_M_t']['_M_head_impl']
+self.pointer = val['_M_t']['_M_t']['_M_head_impl']
 elif is_specialization_of(impl_type, 'tuple'):
-v = self.val['_M_t']['_M_head_impl']
+self.pointer = val['_M_t']['_M_head_impl']
 else:
+self.pointer = None
+
+def children (self):
+return [('get()', self.pointer)]
+
+def to_string (self):
+if not self.pointer:
 raise ValueError("Unsupported implementation for unique_ptr: %s" % self.val.type.fields()[0].type.tag)
-return 'std::unique_ptr<%s> containing %s' % (str(v.type.target()),
-  str(v))
+return ('std::unique_ptr<%s>' % (str(self.pointer.type.target(
 
 def get_value_from_aligned_membuf(buf, valtype):
 """Returns the value held in a __gnu_cxx::__aligned_membuf."""