Page MenuHomePhabricator

[lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.
ClosedPublic

Authored by teemperor on Feb 12 2020, 4:30 AM.

Details

Summary

Currently when printing data types we include implicit scopes such as inline namespaces or anonymous namespaces.
This leads to command output like this (for std::set<X> with X being in an anonymous namespace):

(lldb) print my_set
(std::__1::set<(anonymous namespace)::X, std::__1::less<(anonymous namespace)::X>, std::__1::allocator<(anonymous namespace)::X> >) $0 = size=0 {}

This patch removes all the implicit scopes when printing type names in TypeSystemClang::GetDisplayTypeName
so that our output now looks like this:

(lldb) print my_set
(std::set<X, std::less<X>, std::allocator<X> >) $0 = size=0 {}

As previously GetDisplayTypeName and GetTypeName had the same output we actually often used the
two as if they are the same method (they were in fact using the same implementation), so this patch also
fixes the places where we actually want the display type name and not the actual type name.

Note that this doesn't touch the GetTypeName class that for example the data formatters use, so this patch
is only changes the way we display types to the user. The full type name can also still be found when passing
'-R' to see the raw output of a variable in case someone is somehow interested in that.

Partly fixes rdar://problem/59292534

Diff Detail

Event Timeline

teemperor created this revision.Feb 12 2020, 4:30 AM

I can see how stripping __1 would be nice but I seeing (anonymous namespace) may be useful to know especially b/c it effects visibility and linkage. It would be nicer if could make this optional and default it off but be able to turn it back on.

teemperor edited the summary of this revision. (Show Details)Feb 13 2020, 5:36 AM
teemperor edited the summary of this revision. (Show Details)Feb 13 2020, 5:48 AM

I can see how stripping __1 would be nice but I seeing (anonymous namespace) may be useful to know especially b/c it effects visibility and linkage. It would be nicer if could make this optional and default it off but be able to turn it back on.

You can always use the 'raw' output on variables (-R) for seeing all information about a variable and its type (including the full non-display name). Also I think the general consensus is that we print the types as close as possible as how the user is using them in the program (which is what for example Clang is doing in its diagnostics).

I updated the review description to better illustrate how verbose the anonymous/inline namespaces are in the type descriptions.

The only hesitation I have about this is if we are still printing this noise in demangled names, then the name of the type you see for a variable will be different from what you see when a method of that type ends up in a backtrace. That might be confusing. OTOH, the correct solution to that problem, IMO, is to remove the noise from backtraces, not keep it in the types...

shafik accepted this revision.Feb 13 2020, 10:42 AM

If we still see the info using -R then I am happy but Jim's concerns are valid, they won't match the bracktrace.

This revision is now accepted and ready to land.Feb 13 2020, 10:42 AM

The only hesitation I have about this is if we are still printing this noise in demangled names, then the name of the type you see for a variable will be different from what you see when a method of that type ends up in a backtrace. That might be confusing. OTOH, the correct solution to that problem, IMO, is to remove the noise from backtraces, not keep it in the types...

I'm not entirely sure how/if we can make the backtraces nicer as we only have the function type (but not the specific function declaration with the name) and mangled/demangled name for them. Removing the anonymous namespace from the backtrace is doable with that, but inline namespaces and default template arguments (which are part of the radar) aren't possible unless we start doing much more work for the backtraces and figure out the actual FunctionDecls behind each function.

FWIW, the current way we express backtraces is anyway not identical to what we show in variables. Any typedefs and so on are lost in the mangled name, so stepping into std::string member functions brings users to a backtrace with in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::foo. However printing the string shows them std::__1::string.

This revision was automatically updated to reflect the committed changes.