Re: [PATCH] Fix gdb FilteringTypePrinter (again)

[Jonathan Wakely <jwakely@redhat.com>]

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.

>
>-    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?



>+        self.template = template
>         self.name = name
>+        self.targ1 = targ1
>         self.enabled = True
>
>     class _recognizer(object):
>         "The recognizer class for FilteringTypePrinter."
>
>-        def __init__(self, match, name):
>-            self.match = match
>+        def __init__(self, template, name, targ1):
>+            self.template = template
>             self.name = name
>+            self.targ1 = targ1
>             self.type_obj = None
>
>         def recognize(self, type_obj):
>             """
>-            If type_obj starts with self.match and is the same type as
>+            If type_obj starts with self.template and is the same type as
>             self.name then return self.name, otherwise None.
>             """
>             if type_obj.tag is None:
>                 return None
>
>             if self.type_obj is None:
>-                if not type_obj.tag.startswith(self.match):
>+                if self.targ1 is not None:
>+                    if not type_obj.tag.startswith('{}<{}'.format(self.template, self.targ1)):

I wonder if we should make targ1 a gdb.Type object, not just a string.
That seems like it would be better. The add_on_type_printer function
could still accept a string, and then call gdb.lookup_type(targ1) to
get a gdb.Type. We can change that later though.

OK for trunk with the comments fixed to say "specialization" again.