On Wed, 16 Nov 2022 at 11:54, Jonathan Wakely wrote: > > On Wed, 16 Nov 2022 at 11:35, Jonathan Wakely wrote: > > > > On Wed, 16 Nov 2022 at 06:04, François Dumont 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 > > >> > > > >> - 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 is the same > > > >> type as > > > >> + e.g. for template='basic_istream', name='istream', if any > > > >> instantiation of > > > >> + std::basic_istream 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 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 struct foo { }; > > > > using F = foo; // #1 > > > > template struct foo { }; // #2 > > > > template<> struct foo { }; // #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) > > > > > > 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 : No type named > > std::experimental::fundamentals_v1::any::_Manager_internal > std::char_traits, std::allocator >>. > > got: $12 = Python Exception : No type named > > std::experimental::fundamentals_v1::any::_Manager_internal > std::char_traits, std::allocator >>. > > 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 which is not the > correct type for this specialization of the any manager function. It > contains a std::basic_string. And here's the rather disgusting "fix". I'll push to trunk.