[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG785df616807f: [lldb] Let TypeSystemClang::GetDisplayTypeName 
remove anonymous and inline… (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74478/new/

https://reviews.llvm.org/D74478

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  
lldb/test/API/commands/expression/import-std-module/basic/TestImportStdModule.py
  
lldb/test/API/commands/expression/import-std-module/conflicts/TestStdModuleWithConflicts.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/TestDataFormatterLibccMultiMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/TestDataFormatterLibcxxMultiSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/TestDataFormatterLibcxxQueue.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/TestDataFormatterLibcxxTuple.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp
@@ -102,9 +102,9 @@
 // CHECK: (A::C<-1>::D) ACNeg1D = (ACDMember = 0, CPtr = 0x{{0+}})
 // CHECK: (A::D) AD = {}
 // CHECK: (A::D::E) ADE = (ADDMember = 0)
-// CHECK: ((anonymous namespace)::Anonymous) AnonInt = (AnonymousMember = 0)
-// CHECK: ((anonymous namespace)::Anonymous>) AnonABCVoid = (AnonymousMember = 0)
-// CHECK: ((anonymous namespace)::Anonymous>::D) AnonABCVoidD = (AnonymousDMember = 0)
+// CHECK: (Anonymous) AnonInt = (AnonymousMember = 0)
+// CHECK: (Anonymous>) AnonABCVoid = (AnonymousMember = 0)
+// CHECK: (Anonymous>::D) AnonABCVoidD = (AnonymousDMember = 0)
 // CHECK: Dumping clang ast for 1 modules.
 // CHECK: TranslationUnitDecl {{.*}}
 // CHECK: |-CXXRecordDecl {{.*}} class TrivialC definition
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
@@ -51,7 +51,7 @@
'}'])
 
 self.expect("frame variable v_v1",
-substrs=['v_v1 =  Active Type = std::__1::variant  {',
+substrs=['v_v1 =  Active Type = std::variant  {',
  'Value =  Active Type = int  {',
'Value = 12',
  '}',
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
@@ -16,8 +16,7 @@
 
 def setUp(self):
 TestBase.setUp(self)
-ns = 'ndk' if lldbplatformutil.target_is_android() else ''
-self.namespace = 'std::__' + ns + '1'
+self.namespace = 'std'
 
 @add_test_categories(["libc++"])
 def test_with_run_command(self):
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/TestDataFormatterLibcxxTuple.py
===
--- lldb/test/API/fu

Re: [Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

[Jim Ingham via lldb-commits]

Yes, I don't think you have to solve this problem to make the suggested change. 
 

I think a lot of folks would appreciate the effort it would take to make names 
in backtraces more readable.  Sometimes, particularly when you use function 
objects and iterators in combination, the names are insanely long and pretty 
much impossible to read.  There's only so much we can tell from a mangled name 
- as opposed to having the full decl's, but I bet we could still get a pretty 
good compression from the tree-demangling and some heuristics.  So my main 
point was that we need to address shortening names in backtraces, but keeping 
them too long elsewhere isn't the right direction...

Jim


> On Feb 18, 2020, at 4:29 AM, Raphael Isemann via Phabricator 
>  wrote:
> 
> teemperor added a comment.
> 
> In D74478#1874746 , @jingham wrote:
> 
>> 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, 
> std::__1::allocator >::foo`. However printing the string shows them 
> `std::__1::string`.
> 
> 
> Repository:
>  rLLDB LLDB
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D74478/new/
> 
> https://reviews.llvm.org/D74478
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D74478#1874746 , @jingham wrote:

> 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, 
std::__1::allocator >::foo`. However printing the string shows them 
`std::__1::string`.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74478/new/

https://reviews.llvm.org/D74478



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74478/new/

https://reviews.llvm.org/D74478



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

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


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74478/new/

https://reviews.llvm.org/D74478



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

In D74478#1873581 , @shafik wrote:

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


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74478/new/

https://reviews.llvm.org/D74478



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added a comment.

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.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74478/new/

https://reviews.llvm.org/D74478



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D74478: [lldb] Let TypeSystemClang::GetDisplayTypeName remove anonymous and inline namespaces.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: shafik, jingham.
Herald added subscribers: lldb-commits, JDevlieghere, christof.
Herald added a project: LLDB.

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`):

  (lldb) print my_set
  (std::__1::set, std::__1::allocator >) $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, std::allocator >) $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.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D74478

Files:
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  
lldb/test/API/commands/expression/import-std-module/basic/TestImportStdModule.py
  
lldb/test/API/commands/expression/import-std-module/conflicts/TestStdModuleWithConflicts.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/forward_list/TestDataFormatterLibcxxForwardList.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/iterator/TestDataFormatterLibccIterator.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/map/TestDataFormatterLibccMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multimap/TestDataFormatterLibccMultiMap.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/TestDataFormatterLibcxxMultiSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/multiset/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/queue/TestDataFormatterLibcxxQueue.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/TestDataFormatterLibcxxSet.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/set/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/tuple/TestDataFormatterLibcxxTuple.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataFormatterUnordered.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp

Index: lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/ast-types.cpp
@@ -102,9 +102,9 @@
 // CHECK: (A::C<-1>::D) ACNeg1D = (ACDMember = 0, CPtr = 0x{{0+}})
 // CHECK: (A::D) AD = {}
 // CHECK: (A::D::E) ADE = (ADDMember = 0)
-// CHECK: ((anonymous namespace)::Anonymous) AnonInt = (AnonymousMember = 0)
-// CHECK: ((anonymous namespace)::Anonymous>) AnonABCVoid = (AnonymousMember = 0)
-// CHECK: ((anonymous namespace)::Anonymous>::D) AnonABCVoidD = (AnonymousDMember = 0)
+// CHECK: (Anonymous) AnonInt = (AnonymousMember = 0)
+// CHECK: (Anonymous>) AnonABCVoid = (AnonymousMember = 0)
+// CHECK: (Anonymous>::D) AnonABCVoidD = (AnonymousDMember = 0)
 // CHECK: Dumping clang ast for 1 modules.
 // CHECK: TranslationUnitDecl {{.*}}
 // CHECK: |-CXXRecordDecl {{.*}} class TrivialC definition
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
@@ -51,7 +51,7 @@
'}'])
 
 self.expect("frame variable v_v1",
-substrs=['v_v1 =  Active Type = std::__1::variant  {',
+substrs=['v_v1 =  Active Type = std::variant  {',
  'Value =  Active Type = int  {',
'Value = 12',
  '}',
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/TestDataForma