zturner added a comment.
Bleh, I think I found a problem with this approach. Since we assume everything
is a namespace, it can lead to ambiguities. I think we need to actually
consult the debug info while parsing components of the mangled name. For
example, suppose you have this code:
namespace NS1 {
struct Tag2 {
class TagRecord {};
};
}
NS1::Tag2::TagRecord ATR;
NS1::Tag2 T2;
And in LLDB you run the following commands:
(lldb) target variable ATR
(lldb) target variable T2
We demangle the first name and create an AST from it, but we don't know what
`Tag2` is so we assume it's a namespace and we create a `NamespaceDecl`. Then
we get to the second one we actually know what it is because we have a precise
debug info record for it so we know it's a struct so we create a
`CXXRecordDecl` for it. So now we end up creating an invalid AST. Clang will
accept it, but it's going to lead to ambiguities on name lookup and create
problems down the line. So, I think what we should do instead is ask the debug
info "Do you know what the type named `NS1` is in the global scope?" If no,
it's a namespace. Otherwise we create (or lookup) the appropriate decl. Then
we can repeat this at each step along the way.
BTW, when I run those above 2 commands with the patch as it is above, here is
the output I get. And you can see that there are two conflicting decls.
D:\src\llvmbuild\ninja-x64>"bin\lldb" "-f"
"D:\src\llvmbuild\ninja-x64\tools\lldb\lit\SymbolFile\NativePDB\Output\ast-reconstruction.cpp.tmp.exe"
(lldb) target create
"D:\\src\\llvmbuild\\ninja-x64\\tools\\lldb\\lit\\SymbolFile\\NativePDB\\Output\\ast-reconstruction.cpp.tmp.exe"
Current executable set to
'D:\src\llvmbuild\ninja-x64\tools\lldb\lit\SymbolFile\NativePDB\Output\ast-reconstruction.cpp.tmp.exe'
(x86_64).
(lldb) target variable ATR
(NS1::Tag2::TagRecord) ATR = {}
(lldb) target variable T2
(NS1::Tag2) T2 = {}
(lldb) target modules dump ast
TranslationUnitDecl 0x2247dff6fd8 <<invalid sloc>> <invalid sloc>
|-NamespaceDecl 0x2247dff7898 <<invalid sloc>> <invalid sloc> NS1
| |-NamespaceDecl 0x2247dff7918 <<invalid sloc>> <invalid sloc> Tag2
| | `-CXXRecordDecl 0x2247dff7998 <<invalid sloc>> <invalid sloc> class
TagRecord definition
| | |-DefinitionData pass_in_registers empty aggregate standard_layout
trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor
can_const_default_init
| | | |-DefaultConstructor exists trivial constexpr needs_implicit
defaulted_is_constexpr
| | | |-CopyConstructor simple trivial has_const_param needs_implicit
implicit_has_const_param
| | | |-MoveConstructor exists simple trivial needs_implicit
| | | |-CopyAssignment trivial has_const_param needs_implicit
implicit_has_const_param
| | | |-MoveAssignment exists simple trivial needs_implicit
| | | `-Destructor simple irrelevant trivial needs_implicit
| | `-MSInheritanceAttr 0x2247dff7a60 <<invalid sloc>> Implicit
__single_inheritance
| `-CXXRecordDecl 0x2247dff7b08 <<invalid sloc>> <invalid sloc> struct Tag2
definition
| |-DefinitionData pass_in_registers empty aggregate standard_layout
trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor
can_const_default_init
| | |-DefaultConstructor exists trivial constexpr needs_implicit
defaulted_is_constexpr
| | |-CopyConstructor simple trivial has_const_param needs_implicit
implicit_has_const_param
| | |-MoveConstructor exists simple trivial needs_implicit
| | |-CopyAssignment trivial has_const_param needs_implicit
implicit_has_const_param
| | |-MoveAssignment exists simple trivial needs_implicit
| | `-Destructor simple irrelevant trivial needs_implicit
| `-MSInheritanceAttr 0x2247dff7bd0 <<invalid sloc>> Implicit
__single_inheritance
`-<undeserialized declarations>
Dumping clang ast for 1 modules.
https://reviews.llvm.org/D54053
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
