On 16/11/22 12:54, Jonathan Wakely wrote:
On Wed, 16 Nov 2022 at 11:35, Jonathan Wakely <jwak...@redhat.com> wrote:
On Wed, 16 Nov 2022 at 06:04, François Dumont <frs.dum...@gmail.com> wrote:
On 15/11/22 17:17, Jonathan Wakely wrote:
On 06/10/22 19:38 +0200, François Dumont wrote:
Hi

Looks like the previous patch was not enough. When using it in the
context of a build without dual abi and versioned namespace I started
having failures again. I guess I hadn't rebuild everything properly.

This time I think the problem was in those lines:

             if self.type_obj == type_obj:
                 return strip_inline_namespaces(self.name)

I've added a call to gdb.types.get_basic_type so that we do not compare
a type with its typedef.

Thanks for the pointer to the doc !

Doing so I eventually use your code Jonathan to make FilteringTypeFilter
more specific to a given instantiation.

     libstdc++: Fix gdb FilteringTypePrinter

     Once we found a matching FilteringTypePrinter instance we look for
the associated
     typedef and check that the returned Python Type is equal to the
Type to recognize.
     But gdb Python Type includes properties to distinguish a typedef
from the actual
     type. So use gdb.types.get_basic_type to check if we are indeed on
the same type.

     Additionnaly enhance FilteringTypePrinter matching mecanism by
introducing targ1 that,
     if not None, will be used as the 1st template parameter.

     libstdc++-v3/ChangeLog:

             * python/libstdcxx/v6/printers.py (FilteringTypePrinter):
Rename 'match' field
             'template'. Add self.targ1 to specify the first template
parameter of the instantiation
             to match.
             (add_one_type_printer): Add targ1 optional parameter,
default to None.
             Use gdb.types.get_basic_type to compare the type to
recognize and the type
             returned from the typedef lookup.
             (register_type_printers): Adapt calls to
add_one_type_printers.

Tested under Linux x86_64 normal, version namespace with or without dual
abi.

François

diff --git a/libstdc++-v3/python/libstdcxx/v6/printers.py
b/libstdc++-v3/python/libstdcxx/v6/printers.py
index 0fa7805183e..52339b247d8 100644
--- a/libstdc++-v3/python/libstdcxx/v6/printers.py
+++ b/libstdc++-v3/python/libstdcxx/v6/printers.py
@@ -2040,62 +2040,72 @@ def add_one_template_type_printer(obj, name,
defargs):

class FilteringTypePrinter(object):
     r"""
-    A type printer that uses typedef names for common template
specializations.
+    A type printer that uses typedef names for common template
instantiations.

     Args:
-        match (str): The class template to recognize.
+        template (str): The class template to recognize.
         name (str): The typedef-name that will be used instead.
+        targ1 (str): The first template argument.
+            If arg1 is provided (not None), only template
instantiations with this type
+            as the first template argument, e.g. if
template='basic_string<targ1'

-    Checks if a specialization of the class template 'match' is the
same type
+    Checks if an instantiation of the class template 'template' is
the same type
     as the typedef 'name', and prints it as 'name' instead.

-    e.g. if an instantiation of std::basic_istream<C, T> is the same
type as
+    e.g. for template='basic_istream', name='istream', if any
instantiation of
+    std::basic_istream<C, T> is the same type as std::istream then
print it as
+    std::istream.
+
+    e.g. for template='basic_istream', name='istream', targ1='char',
if any
+    instantiation of std::basic_istream<char, T> is the same type as
     std::istream then print it as std::istream.
     """
These are template specializations, not instantiations. Please undo
the changes to the comments, because the comments are 100% correct
now, and would become wrong with this patch.

template<class T, class U> struct foo { };
using F = foo<int, int>; // #1
template<class T> struct foo<T, void> { }; // #2
template<> struct foo<void, void> { }; // #3

#1 is a *specialization* of the class template foo. It is
*instantiated* when you construct one or depend on its size, or its
members.
#2 is a *partial specialization* and #3 is an explicit specialization.
But #1 is a speclialization, not an instantiation.

Instantiation is a process that happens during compilation. A
specialization is a type (or function, or variable) generated from a
template by substituting arguments for the template parameters. The
python type printer matches specializations.
Lesson learned, thanks.

Maybe comment on line 169 is wrong then. I think there is a clue in the
function name 'is_specialization_of' :-)
Good point! Thanks, I'll fix it.

-    def __init__(self, match, name):
-        self.match = match
+    def __init__(self, template, name, targ1):
Is there a reason to require targ1 here, instead of making it
optional, by using =None as the default?
In your original, and I know untested, proposal it was not working.

The function add_one_type_printer was missing to pass its targ1
parameter to the FilteringTypePrinter ctor but thanks to the default
value it was un-noticed by the interpreter.
My untested patch had this, which adds it, doesn't it?

-def add_one_type_printer(obj, match, name):
-    printer = FilteringTypePrinter('std::' + match, 'std::' + name)
+def add_one_type_printer(obj, match, name, targ1 = None):
+    printer = FilteringTypePrinter('std::' + match, 'std::' + name, targ1)
      gdb.types.register_type_printer(obj, printer)
      if _versioned_namespace:
          ns = 'std::' + _versioned_namespace
-        printer = FilteringTypePrinter(ns + match, ns + name)
+        printer = FilteringTypePrinter(ns + match, ns + name, targ1)
          gdb.types.register_type_printer(obj, printer)
Indeed, I guess I mess it up myself when doing all my tests.


I think FilteringTypePrinter should be usable without specifying None
explicitly as the argument. Even if we don't actually use it that way
today, it seems like a better API. If the argument is optional, then
the idiomatic way to express that is to give it a default, not require
None to be passed.

I'll add that default argument, but first I need to figure out why I'm
seeing new failures for libfundts.cc with -D_GLIBCXX_USE_CXX11_ABI=0.
Your patch has introduced this new error:

$12 = Python Exception <class 'gdb.error'>: No type named
std::experimental::fundamentals_v1::any::_Manager_internal<std::__cxx11::basic_string<char,
std::char_traits<char>, std::allocator<char> >>.
got: $12 = Python Exception <class 'gdb.error'>: No type named
std::experimental::fundamentals_v1::any::_Manager_internal<std::__cxx11::basic_string<char,
std::char_traits<char>, std::allocator<char> >>.
FAIL: libstdc++-prettyprinters/libfundts.cc print as
The problem happens here in StdExpAnyPrinter:

             mgrname = m.group(1)
             # FIXME need to expand 'std::string' so that gdb.lookup_type works
             if 'std::string' in mgrname:
                 mgrname = re.sub("std::string(?!\w)",
str(gdb.lookup_type('std::string').strip_typedefs()), m.group(1))

             mgrtype = gdb.lookup_type(mgrname)

After your patch, gdb.lookup_type('std::string').strip_typedefs() is
returning std::__cxx11::basic_string<char,...> which is not the
correct type for this specialization of the any manager function. It
contains a std::basic_string<char,...>.

I had noticed this usage of std::string but surprisingly had never managed to have a test failure because of that so I left it untouch.

Note that I had not re-tested the patch after your approval, maybe something changed in the month gap.

Thanks for having sorted that out.

Reply via email to