public inbox for
 help / color / mirror / Atom feed
From: "François Dumont" <>
To: Jonathan Wakely <>
Cc: "" <>,
	gcc-patches <>
Subject: Re: [PATCH] Fix gdb printers for std::string
Date: Sat, 1 Oct 2022 12:43:22 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On 01/10/22 12:06, Jonathan Wakely wrote:
> On Sat, 1 Oct 2022 at 08:20, François Dumont via Libstdc++
> <> wrote:
>> I had forgotten to re-run tests after I removed the #define
>> The comment was misleading, it could also impact output of std::list.
>> I am also restoring the correct std::string alias for
>> std::__cxx11::basic_string, even if with my workaround it doesn't really
>> matter as the one for std::basic_string will be used.
>> I also restored the printer for std::__cxx11::string typedef. Is it
>> intentional to keep this ?
> Yes, I kept that intentionally. There can be programs where some
> objects still use that typedef, if those objects were compiled with
> GCC 8.x or older.
>>       libstdc++: Fix gdb pretty printers when dealing with std::string
>>       Since revision 33b43b0d8cd2de722d177ef823930500948a7487 std::string
>> and other
>>       similar typedef are ambiguous from a gdb point of view because it
>> matches both
>>       std::basic_string<char> and std::__cxx11::basic_string<char>
>> symbols. For those
>>       typedef add a workaround to accept the substitution as long as the
>> same regardless
>>       of __cxx11 namespace.
> Thanks for figuring out what was going wrong here, and how to fix it.
>>       Also avoid to register printers for types in std::__cxx11::__8::
>> namespace, there is
>>       no such symbols.
>>       libstdc++-v3/ChangeLog:
>>               * libstdc++-v3/python/libstdcxx/v6/
>> (Printer.add_version): Do not add
>>               version namespace for __cxx11 symbols.
>>               (add_one_template_type_printer): Likewise.
>>               (add_one_type_printer): Likewise.
>>               (FilteringTypePrinter._recognizer.recognize): Add a
>> workaround for std::string & al
>>               ambiguous typedef matching both std:: and std::__cxx11::
>> symbols.
>>               (register_type_printers): Refine type registration to limit
>> false positive in
>>               FilteringTypePrinter._recognize.recognize requiring to look
>> for the type in gdb.
> I don't really like this part of the change though:

I'll check what you are proposing but I don't think it is necessary to 
fix the problem.

I did this on my path to find out what was going wrong. Once I 
understood it I consider that it was just a good change to keep. If you 
think otherwise I can revert this part.

I also noted that gdb consider the filters as a filo list, not fifo. And 
I think that the 1st filters registered are the most extensively used. I 
can propose to invert all the registration if you think it worth it.

> @@ -2105,29 +2120,29 @@ def register_type_printers(obj):
>           return
>       # Add type printers for typedefs std::string, std::wstring etc.
> -    for ch in ('', 'w', 'u8', 'u16', 'u32'):
> -        add_one_type_printer(obj, 'basic_string', ch + 'string')
> -        add_one_type_printer(obj, '__cxx11::basic_string', ch + 'string')
> +    for ch in (('char', ''), ('wchar_t', 'w'), ('char8_t', 'u8'),
> ('char16_t', 'u16'), ('char32_t', 'u32')):
> +        add_one_type_printer(obj, 'basic_string<' + ch[0], ch[1] + 'string')
> +        add_one_type_printer(obj, '__cxx11::basic_string<' + ch[0],
> ch[1] + 'string')
> As the docs for FilteringTypePrinter says, the first argument is
> supposed to be the class template name:
> class FilteringTypePrinter(object):
>      r"""
>      A type printer that uses typedef names for common template specializations.
>      Args:
>          match (str): The class template to recognize.
>          name (str): The typedef-name that will be used instead.
>      Checks if a specialization of the class template 'match' 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
>      std::istream then print it as std::istream.
>      """
> With this change, the "class template" is sometimes just a string
> prefix of a particular specialization, e.g. "basic_string<char"
> The class template is "basic_string", and that's how the
> FilteringTypePrinter was intended to work.
> How about something like the attached (untested) change instead. which
> keeps the 'match' argument as the class template name, but optionally
> supports passing the first template argument? Then you can use
> match.split('::')[-1] is 'basic_string' to check if we are dealing
> with one of the possibly ambiguous typedefs.

  reply	other threads:[~2022-10-01 10:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-28 20:42 François Dumont
2022-10-01  7:20 ` François Dumont
2022-10-01 10:06   ` Jonathan Wakely
2022-10-01 10:43     ` François Dumont [this message]
2022-10-01 15:30       ` Jonathan Wakely
2022-10-03 16:51         ` François Dumont
2022-10-03 17:14           ` Jonathan Wakely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).