[Lldb-commits] [PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

@gamesh411 I recreated the setup you listed (thanks for that btw) but for me this works just fine. I assume the crash happens in a class template from one of the external libraries. It probably works for me because I don't have the same library version as you have, but just from the backtrace it's hard to know where the error could come from.

I'm not sure if the XTU analyzer has a way to create reproducers (maybe @martong knows), but if you could apply the patch below, recompile/run the analyzer and post the output that would help a lot with figuring out what code on your system is causing the crash:

  diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
  index 67ee8c0956d6..a471501aa71e 100644
  --- a/clang/lib/AST/ASTContext.cpp
  +++ b/clang/lib/AST/ASTContext.cpp
  @@ -4408,6 +4408,9 @@ static bool NeedsInjectedClassNameType(const RecordDecl *D) {
   /// injected class name type for the specified templated declaration.
   QualType ASTContext::getInjectedClassNameType(CXXRecordDecl *Decl,
                                                 QualType TST) const {
  +  if (!NeedsInjectedClassNameType(Decl)) {
  +    Decl->dumpColor();
  +  }
     assert(NeedsInjectedClassNameType(Decl));
     if (Decl->TypeForDecl) {
       assert(isa<InjectedClassNameType>(Decl->TypeForDecl));

The output from this code is probably colorized in your terminal and looks a bit like this:

  |-ClassTemplateDecl 0x7ff94c04a0a0 <deduction.cpp:1:7, line:3:7> line:1:36 A
  | |-TemplateTypeParmDecl 0x7ff94c049f50 <col:17, col:26> col:26 referenced typename depth 0 index 0 T
  | |-CXXRecordDecl 0x7ff94c04a010 <col:29, line:3:7> line:1:36 struct A definition
  | | |-DefinitionData empty standard_layout trivially_copyable has_user_declared_ctor can_const_default_init
  | | | |-DefaultConstructor defaulted_is_constexpr
  | | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
  | | | |-MoveConstructor exists simple trivial needs_implicit
  [...]

FWIW, I don't think the patch itself introduces this regression, but it just causes more of the AST to be imported (and the import of these AST nodes then runs into an unsupported use case). This would most likely already crash if the analysed source code referenced that class template in some other way.



================
Comment at: lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py:25
 
-        deque_type = "std::deque<int, std::allocator<int> >"
         size_type = deque_type + "::size_type"
----------------
shafik wrote:
> Why do the default arguments not show up in the results anymore?
Because we don't show args that match the default arg in the display type (which is identical to what Clang does). See also rdar://59292534


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103