[Lldb-commits] [PATCH] D54053: [NativePDB] Add the ability to create clang record decls from mangled names.

[Zachary Turner via Phabricator via lldb-commits]

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