This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Pretty printers for versioned namespace


On 14/12/16 22:49 +0100, François Dumont wrote:
On 09/12/2016 16:18, Jonathan Wakely wrote:

But I don't know how to fix this so for the moment I just adapt it to correctly handle std::__7::string.

But that's not correct. Please try to understand the point I'm making:
The name "std::__7::string" does not appear in a symbol name.
Ok, the only point I don't get yet is why std::string is a symbol but std::__7::string is not. It seems inconsistent.

The demangler has special handling for std::basic_string<char,
std::char_traits<char>, std::allocator<char>> to treat it as
std::string, because that's much more user-friendly. But when it sees
a different symbol, like foo::basic_string<bar>, or std::__7::... it
doesn't match and doesn't get special handling.

Being consistent doesn't matter, the point is to be user-friendly.

Users want to see std::string, not std::basic_string<char, ..., ...>.

Looking at all the ugly changes needed to the tests, I'm wondering if
we actually want to strip the __7:: namespace out of the typenames
displayed by printers. That would mean we don't need to change the
tests, but more importantly, it would mean the users see "std::vector"
not "std::__7::vector". The versioned namespace is an implementation
detail, not something they write explicitly in their code.

Your change makes the printer tests pass for the versioned namespace,
but does it make the printers useful for users? We want the printers
to be useful, not just pass our testsuite.



This works for me:

@@ -946,9 +950,10 @@ class StdExpAnyPrinter(SingleObjContainerPrinter):
           m = re.match(rx, func.function.name)
           if not m:
raise ValueError("Unknown manager function in %s" % self.typename)
-
- # FIXME need to expand 'std::string' so that gdb.lookup_type works - mgrname = re.sub("std::string(?!\w)", str(gdb.lookup_type('std::string').strip_typedefs()), m.group(1))
+            mgrname = m.group(1)
+            if not typename.startswith('std::' + vers_nsp):
+ # FIXME need to expand 'std::string' so that gdb.lookup_type works + mgrname = re.sub("std::string(?!\w)", str(gdb.lookup_type('std::string').strip_typedefs()), mgrname)

I think it doesn't work in all situations as this code is also used for std::experimental::any so typename doesn't start with std::__7:: but there is still no std::string symbol.

Ah yes, I forgot about the experimental one.

So I propose:

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

as you will see in attach patch.

Looks good.

I eventually use '__7' explicitely in some pretty printers tests because '__\d+' was not working, don't know.

Yes, I also found that wasn't working. Presumably Tcl doesn't support
that syntax.

Ok to commit once tests have completed ?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]