Lang, I've done some more research on this and here is what I've found so far.
LLDB uses the ExternalASTSource interface and it provides its own implementation for it (lldb_private::ClangASTSource). ClangASTSource is being used in at least 3 modes (all of these uses the ASTImporter): (1) to do a minimal import for a Type/Decl, this is used for lookup (2) to do a minimal import for a Type/Decl, this is used for computing the record layout (3) to complete the Type which has been imported in a minimal mode previously. The completion is done via ASTImporter::ImportDefinition, which does a full import of a TagDecl even in minimal mode. Besides, there is a fourth method which exercises the ASTImporter, but it does not use the ExternalASTSource interface (4). [[ Actually, (4) happens before than (3) in program runtime. ]] So, it is kind of weird to me, that we use a simple minimal import in (2) for computing the record layout, perhaps we could make a change in LLDB to call `lldb_private::ClangASTSource::CompleteType` to achieve a full import in this case? Instead of changing the logic in clang::ASTImporter? By doing so, I think the logic would be aligned with Richard's idea. Gabor (1) === "To" ASTContext A, "From" ASTContext X, MINIMAL import through ExternalASTSource inteface clang::ASTImporter::Import (this=0x13b3c50, FromT=...) at ../../git/llvm/tools/clang/lib/AST/ASTImporter.cpp:6983 lldb_private::ClangASTImporter::CopyType (this=0x12a3480, dst_ast=0x1363420, src_ast=0x11f7980, type=...) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTImporter.cpp:61 lldb_private::ClangASTSource::GuardedCopyType (this=0x12394b0, src_type=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:2038 lldb_private::ClangExpressionDeclMap::GetVariableValue (this=0x12394b0, var=warning: RTTI symbol not found for class 'std::_Sp_counted_ptr<lldb_private::Variable*, (__gnu_cxx::_Lock_policy)2>' lldb_private::ClangExpressionDeclMap::AddOneVariable (this=0x12394b0, context=..., var=warning: RTTI symbol not found for class 'std::_Sp_counted_ptr<lldb_private::Variable*, (__gnu_cxx::_Lock_policy)2>' lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls (this=0x12394b0, context=..., module_sp=std::shared_ptr (empty) 0x0, namespace_decl=..., current_id=3) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1235 lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls (this=0x12394b0, context=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:842 lldb_private::ClangASTSource::FindExternalVisibleDeclsByName (this=0x12394b0, decl_ctx=0x1371298, clang_decl_name=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:261 lldb_private::ClangASTSource::ClangASTSourceProxy::FindExternalVisibleDeclsByName (this=0x105e230, DC=0x1371298, Name=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h:244 clang::DeclContext::lookup (this=0x1371298, Name=...) at ../../git/llvm/tools/clang/lib/AST/DeclBase.cpp:1599 LookupDirect (S=..., R=..., DC=0x1371298) at ../../git/llvm/tools/clang/lib/Sema/SemaLookup.cpp:84 CppNamespaceLookup (S=..., R=..., Context=..., NS=0x1d69dd8, UDirs=...) at ../../git/llvm/tools/clang/lib/Sema/SemaLookup.cpp:942 clang::Sema::CppLookupName (this=0x1d74cd0, R=..., S=0x1d80510) at ../../git/llvm/tools/clang/lib/Sema/SemaLookup.cpp:1322 ... clang::Sema::ActOnUsingDeclaration (this=0x1d74cd0, S=0x1d93820, AS=clang::AS_none, UsingLoc=..., TypenameLoc=..., SS=..., Name=..., EllipsisLoc=..., AttrList=...) at ../../git/llvm/tools/clang/lib/Sema/SemaDeclCXX.cpp:9418 clang::Parser::ParseUsingDeclaration (this=0x1d7bd90, Context=clang::DeclaratorContext::BlockContext, TemplateInfo=..., UsingLoc=..., DeclEnd=..., AS=clang::AS_none) at ../../git/llvm/tools/clang/lib/Parse/ParseDeclCXX.cpp:710 ... clang::Parser::ParseTopLevelDecl (this=0x1d7bd90, Result=...) at ../../git/llvm/tools/clang/lib/Parse/Parser.cpp:610 clang::ParseAST (S=..., PrintStats=false, SkipFunctionBodies=false) at ../../git/llvm/tools/clang/lib/Parse/ParseAST.cpp:158 clang::ParseAST (PP=..., Consumer=0x1a2dfc0, Ctx=..., PrintStats=false, TUKind=clang::TU_Complete, CompletionConsumer=0x0, SkipFunctionBodies=false) at ../../git/llvm/tools/clang/lib/Parse/ParseAST.cpp:111 lldb_private::ClangExpressionParser::Parse (this=0x7fff14cdfe90, diagnostic_manager=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:608 lldb_private::ClangUserExpression::Parse (this=0x1d1d530, diagnostic_manager=..., exe_ctx=..., execution_policy=lldb_private::eExecutionPolicyOnlyWhenNeeded, keep_result_in_memory=true, generate_debug_info=false) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp:495 lldb_private::UserExpression::Evaluate (exe_ctx=..., options=..., expr="tpi", prefix="", result_valobj_sp=..., error=..., line_offset=0, fixed_expression=0x1a95738, jit_module_sp_ptr=0x0) at ../../git/llvm/tools/lldb/source/Expression/UserExpression.cpp:237 lldb_private::Target::EvaluateExpression (this=0x1b32570, expr="tpi", exe_scope=0x7fe73ca42d90, result_valobj_sp=..., options=..., fixed_expression=0x1a95738) at ../../git/llvm/tools/lldb/source/Target/Target.cpp:2308 lldb_private::CommandObjectExpression::EvaluateExpression (this=0x1a953b0, expr=0x7fff14ce0d58 "tpi", output_stream=0x7fff14ce16e8, error_stream=0x7fff14ce1740, result=0x7fff14ce16e8) at ../../git/llvm/tools/lldb/source/Commands/CommandObjectExpression.cpp:377 (2) === ASTContext A, "From" ASTContext X, MINIMAL import through ExternalASTSource inteface clang::ASTImporter::Import (this=0x13b3c50, FromD=0x13bba18) at ../../git/llvm/tools/clang/lib/AST/ASTImporter.cpp:7040 lldb_private::ClangASTImporter::CopyDecl (this=0x12a3480, dst_ast=0x1363420, src_ast=0x11f7980, decl=0x13bba18) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTImporter.cpp:102 lldb_private::ClangASTSource::CopyDecl (this=0x12394b0, src_decl=0x13bba18) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:1991 lldb_private::ClangASTSource::FindExternalLexicalDecls(clang::DeclContext const*, llvm::function_ref<bool (clang::Decl::Kind)>, llvm::SmallVectorImpl<clang::Decl*>&) (this=0x12394b0, decl_context=0x13b2048, predicate=..., decls=llvm::SmallVector of Size 0, Capacity 64) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:635 lldb_private::ClangASTSource::ClangASTSourceProxy::FindExternalLexicalDecls(clang::DeclContext const*, llvm::function_ref<bool (clang::Decl::Kind)>, llvm::SmallVectorImpl<clang::Decl*>&) (this=0x105e230, DC=0x13b2048, IsKindWeWant=..., Decls=llvm::SmallVector of Size 0, Capacity 64) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h:251 clang::RecordDecl::LoadFieldsFromExternalStorage (this=0x13b2010) at ../../git/llvm/tools/clang/lib/AST/Decl.cpp:4137 clang::RecordDecl::field_begin (this=0x13b2010) at ../../git/llvm/tools/clang/lib/AST/Decl.cpp:4109 clang::RecordDecl::fields (this=0x13b2010) at ../../git/llvm/tools/clang/include/clang/AST/Decl.h:3751 (anonymous namespace)::EmptySubobjectMap::ComputeEmptySubobjectSizes (this=0x7fffd5c774d0) at ../../git/llvm/tools/clang/lib/AST/RecordLayoutBuilder.cpp:206 ... clang::CodeGen::CodeGenTypes::ComputeRecordLayout (this=0x1d6e2d8, D=0x1daab50, Ty=0x1d70550) at ../../git/llvm/tools/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp:725 ... clang::CodeGen::CodeGenModule::EmitTopLevelDecl (this=0x1d6e200, D=0x1daaa00) at ../../git/llvm/tools/clang/lib/CodeGen/CodeGenModule.cpp:4571 (anonymous namespace)::CodeGeneratorImpl::HandleTopLevelDecl (this=0x1d6c620, DG=...) at ../../git/llvm/tools/clang/lib/CodeGen/ModuleBuilder.cpp:160 lldb_private::ASTResultSynthesizer::HandleTopLevelDecl (this=0x1a2dfc0, D=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:109 clang::ParseAST (S=..., PrintStats=false, SkipFunctionBodies=false) at ../../git/llvm/tools/clang/lib/Parse/ParseAST.cpp:162 clang::ParseAST (PP=..., Consumer=0x1a2dfc0, Ctx=..., PrintStats=false, TUKind=clang::TU_Complete, CompletionConsumer=0x0, SkipFunctionBodies=false) at ../../git/llvm/tools/clang/lib/Parse/ParseAST.cpp:111 lldb_private::ClangExpressionParser::Parse (this=0x7fff14cdfe90, diagnostic_manager=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:608 lldb_private::ClangUserExpression::Parse (this=0x1d1d530, diagnostic_manager=..., exe_ctx=..., execution_policy=lldb_private::eExecutionPolicyOnlyWhenNeeded, keep_result_in_memory=true, generate_debug_info=false) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp:495 lldb_private::UserExpression::Evaluate (exe_ctx=..., options=..., expr="tpi", prefix="", result_valobj_sp=..., error=..., line_offset=0, fixed_expression=0x1a95738, jit_module_sp_ptr=0x0) at ../../git/llvm/tools/lldb/source/Expression/UserExpression.cpp:237 lldb_private::Target::EvaluateExpression (this=0x1b32570, expr="tpi", exe_scope=0x7fe73ca42d90, result_valobj_sp=..., options=..., fixed_expression=0x1a95738) at ../../git/llvm/tools/lldb/source/Target/Target.cpp:2308 lldb_private::CommandObjectExpression::EvaluateExpression (this=0x1a953b0, expr=0x7fff14ce0d58 "tpi", output_stream=0x7fff14ce16e8, error_stream=0x7fff14ce1740, result=0x7fff14ce16e8) at ../../git/llvm/tools/lldb/source/Commands/CommandObjectExpression.cpp:377 (3) === ASTContext B, "From" ASTContext Z, ASTImporter is in MINIMAL mode, but ImportDefinition forces a full import clang::ASTImporter::ImportDefinition (this=0x1d75200, From=0x1c01048) at ../../git/llvm/tools/clang/lib/AST/ASTImporter.cpp:7519 lldb_private::ClangASTImporter::Minion::ImportDefinitionTo (this=0x1d75200, to=0x1d1b8d8, from=0x1c01048) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTImporter.cpp:889 lldb_private::ClangASTImporter::CompleteTagDecl (this=0x1c96360, decl=0x1d1b8d8) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTImporter.cpp:560 lldb_private::ClangASTSource::CompleteType (this=0x1a66560, tag_decl=0x1d1b8d8) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:299 lldb_private::ClangASTSource::ClangASTSourceProxy::CompleteType (this=0x1a24d60, Tag=0x1d1b8d8) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h:255 GetCompleteQualType (ast=0x1cde470, qual_type=..., allow_completion=true) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTContext.cpp:2515 lldb_private::ClangASTContext::GetNumChildren (this=0x1c456f0, type=0x1d1b980, omit_empty_base_classes=true) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTContext.cpp:5341 lldb_private::CompilerType::GetNumChildren (this=0x7fff14cdfe18, omit_empty_base_classes=true) at ../../git/llvm/tools/lldb/source/Symbol/CompilerType.cpp:541 lldb_private::ClangASTContext::GetNumChildren (this=0x1c456f0, type=0x1d1ba10, omit_empty_base_classes=true) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTContext.cpp:5443 lldb_private::CompilerType::GetNumChildren (this=0x7fff14ce0018, omit_empty_base_classes=true) at ../../git/llvm/tools/lldb/source/Symbol/CompilerType.cpp:541 lldb_private::ClangASTContext::GetNumChildren (this=0x1c456f0, type=0x1d1baa0, omit_empty_base_classes=true) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTContext.cpp:5472 lldb_private::CompilerType::GetNumChildren (this=0x7fff14ce01d0, omit_empty_base_classes=true) at ../../git/llvm/tools/lldb/source/Symbol/CompilerType.cpp:541 lldb_private::ValueObjectConstResult::CalculateNumChildren (this=0x1d6c620, max=4294967295) at ../../git/llvm/tools/lldb/source/Core/ValueObjectConstResult.cpp:211 lldb_private::ValueObject::GetNumChildren (this=0x1d6c620, max=4294967295) at ../../git/llvm/tools/lldb/source/Core/ValueObject.cpp:601 lldb_private::FormatManager::ShouldPrintAsOneLiner (this=0x7fe7676031d8 <GetFormatManager()::g_format_manager>, valobj=...) at ../../git/llvm/tools/lldb/source/DataFormatters/FormatManager.cpp:494 lldb_private::DataVisualization::ShouldPrintAsOneLiner (valobj=...) at ../../git/llvm/tools/lldb/source/DataFormatters/DataVisualization.cpp:33 lldb_private::ValueObjectPrinter::PrintChildrenIfNeeded (this=0x7fff14ce0560, value_printed=true, summary_printed=false) at ../../git/llvm/tools/lldb/source/DataFormatters/ValueObjectPrinter.cpp:771 lldb_private::ValueObjectPrinter::PrintValueObject ... lldb_private::ValueObject::Dump (this=0x1d6c620, s=..., options=...) at ../../git/llvm/tools/lldb/source/Core/ValueObject.cpp:2742 lldb_private::CommandObjectExpression::EvaluateExpression (4) ==== ASTContext B, "From" ASTContext Y, MINIMAL import, skips ExternalASTSource interface clang::ASTImporter::Import (this=0x1d74cd0, FromT=...) at ../../git/llvm/tools/clang/lib/AST/ASTImporter.cpp:6983 lldb_private::ClangASTImporter::CopyType (this=0x1c96360, dst_ast=0x1cde470, src_ast=0x1d5bf60, type=...) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTImporter.cpp:61 lldb_private::ClangASTImporter::CopyType (this=0x1c96360, dst_ast=0x1cde470, src_ast=0x1d5bf60, type=0x1daad10) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTImporter.cpp:70 lldb_private::ClangASTImporter::DeportType (this=0x1c96360, dst_ctx=0x1cde470, src_ctx=0x1d5bf60, type=0x1daad10) at ../../git/llvm/tools/lldb/source/Symbol/ClangASTImporter.cpp:268 lldb_private::ClangExpressionDeclMap::DeportType (this=0x1c98e00, target=..., source=..., parser_type=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:288 lldb_private::ClangExpressionDeclMap::AddPersistentVariable (this=0x1c98e00, decl=0x1daaff8, name=..., parser_type=..., is_result=true, is_lvalue=true) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:332 IRForTarget::CreateResultVariable (this=0x7fff14cdf8c0, llvm_function=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp:407 IRForTarget::runOnModule (this=0x7fff14cdf8c0, llvm_module=...) at ../../git/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp:2102 lldb_private::ClangExpressionParser::PrepareForExecution ... lldb_private::CommandObjectExpression::EvaluateExpression (this=0x1a953b0, expr=0x7fff14ce0d58 "tpi", output_stream=0x7fff14ce16e8, error_stream=0x7fff14ce1740, result=0x7fff14ce16e8) at ../../git/llvm/tools/lldb/source/Commands/CommandObjectExpression.cpp:377 On Wed, Aug 15, 2018 at 2:52 AM Lang Hames <lha...@gmail.com> wrote: > > Hi Gábor, Richard, > >> Hi Richard, maybe my understanding is not correct, but I believe the >> situation is somewhat twisted and reversed... > > > Is this relationship recursive? (Should it be?) I.e. When ClangASTSource uses > ASTImporter to minimally import a Decl, should that Decl have another > ExternalASTSource attached (possibly the original ClangASTSource) to complete > the minimally imported Decl on demand? > > -- Lang. > > On Tue, Aug 14, 2018 at 6:49 AM Gábor Márton <martongab...@gmail.com> wrote: >> >> > It might be interesting to consider how this is addressed by the >> > ExternalASTSource interface. There, we have separate "import the lexical >> > contents of this AST node (including populating the lexical declarations >> > list with all members, in the right order)", "import the lookup results >> > for this name in this context (and cache them but don't add them to the >> > list of lexical members)" and "import this specific declaration member >> > (but don't add it to the list of lexical members)" queries. One could >> > imagine doing something similar for the AST importer: when you're >> > performing a minimal import but you want to import a method declaration, >> > don't insert it into the list of lexical members of the enclosing record. >> > Instead, defer doing that until a complete type is required (at that >> > point, you'd need to import all the relevant members anyway, including the >> > base classes and virtual functions in the right order). >> >> Hi Richard, maybe my understanding is not correct, but I believe the >> situation is somewhat twisted and reversed. >> Seems like LLDB already exercises the ExternalASTSource interface. The >> purpose of using it is exactly to load the external lexical contents >> of an AST node (as you suggested). However, the load is implemented by >> the means of the ASTImporter (in minimal mode). Consider the attached >> function call trace: >> When the user instructs LLDB to evaluate an expression then LLDB >> exercises the parser `clang::ParseAST`, which in turn does a lookup >> `clang::DeclContext::lookup`. During the lookup, >> `lldb_private::ClangASTSource::FindExternalVisibleDeclsByName` is >> called (this function overloads >> `ExternalASTSource::FindExternalVisibleDeclsByName`). >> And finally, through `lldb_private::ClangASTImporter::CopyType` we end >> up in `clang::ASTImporter::Import`. >> >> Gabor >> >> ``` >> #4 0x00007ffff41a72ed in clang::ASTNodeImporter::ImportDeclParts >> (this=this@entry=0x7fffffff8be0, D=D@entry=0x632038, >> DC=@0x7fffffff8ac8: 0x0, >> LexicalDC=@0x7fffffff8ad0: 0x0, Name=..., ToD=@0x7fffffff8ad8: 0x0, >> Loc=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/AST/ASTImporter.cpp:1062 >> #5 0x00007ffff41a8d55 in clang::ASTNodeImporter::VisitRecordDecl >> (this=0x7fffffff8be0, D=0x632038) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/AST/ASTImporter.cpp:2126 >> #6 0x00007ffff419377b in clang::ASTImporter::Import (this=0x70cc90, >> FromD=0x632038) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/AST/ASTImporter.cpp:7054 >> #7 0x00007ffff41939f7 in clang::ASTNodeImporter::VisitRecordType >> (this=0x7fffffff8c40, T=<optimized out>) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/AST/ASTImporter.cpp:851 >> #8 0x00007ffff4196f65 in clang::TypeVisitor<clang::ASTNodeImporter, >> clang::QualType>::Visit (this=this@entry=0x7fffffff8c40, T=<optimized >> out>) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/include/clang/AST/TypeNodes.def:92 >> #9 0x00007ffff4197187 in clang::ASTImporter::Import (this=0x70cc90, >> FromT=FromT@entry=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/AST/ASTImporter.cpp:6983 >> #10 0x00007ffff7351c92 in lldb_private::ClangASTImporter::CopyType >> (this=<optimized out>, dst_ast=<optimized out>, src_ast=<optimized >> out>, type=type@entry=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Symbol/ClangASTImporter.cpp:61 >> #11 0x00007ffff74bdb95 in >> lldb_private::ClangASTSource::GuardedCopyType >> (this=this@entry=0x611e20, src_type=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:2040 >> #12 0x00007ffff74e9ffe in >> lldb_private::ClangExpressionDeclMap::GetVariableValue >> (this=this@entry=0x611e20, var=std::shared_ptr (count 6, weak 1) >> 0x7fffe80020f0, >> var_location=..., user_type=user_type@entry=0x7fffffff8fc0, >> parser_type=parser_type@entry=0x7fffffff8fe0) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1627 >> #13 0x00007ffff74ea3a7 in >> lldb_private::ClangExpressionDeclMap::AddOneVariable >> (this=this@entry=0x611e20, context=..., >> var=std::shared_ptr (count 6, weak 1) 0x7fffe80020f0, valobj=..., >> current_id=current_id@entry=2) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1679 >> #14 0x00007ffff74ec50b in >> lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls >> (this=this@entry=0x611e20, context=..., module_sp= >> std::shared_ptr (empty) 0x0, namespace_decl=..., >> current_id=current_id@entry=2) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1240 >> #15 0x00007ffff74ef4ce in >> lldb_private::ClangExpressionDeclMap::FindExternalVisibleDecls >> (this=0x611e20, context=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:843 >> #16 0x00007ffff74c71cc in >> lldb_private::ClangASTSource::FindExternalVisibleDeclsByName >> (this=0x611e20, decl_ctx=0x71d8b8, clang_decl_name=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:261 >> #17 0x00007ffff421d92d in clang::DeclContext::lookup >> (this=this@entry=0x71d8b8, Name=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/AST/DeclBase.cpp:1577 >> #18 0x00007ffff4d4bb42 in LookupDirect (S=..., R=..., DC=DC@entry=0x71d8b8) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Sema/SemaLookup.cpp:843 >> #19 0x00007ffff4d52f50 in CppNamespaceLookup (S=..., R=..., >> NS=NS@entry=0x71d8b8, UDirs=..., Context=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Sema/SemaLookup.cpp:942 >> #20 0x00007ffff4d53925 in clang::Sema::CppLookupName >> (this=this@entry=0x7245b0, R=..., S=0x72cad0) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Sema/SemaLookup.cpp:1322 >> #21 0x00007ffff4d53c8a in clang::Sema::LookupName (this=0x7245b0, >> R=..., S=0x7314f0, AllowBuiltinCreation=<optimized out>) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Sema/SemaLookup.cpp:1827 >> #22 0x00007ffff4b6b945 in clang::Sema::ClassifyName (this=0x7245b0, >> S=0x7314f0, SS=..., Name=@0x7fffffffa3a8: 0x750dc8, NameLoc=..., >> NameLoc@entry=..., >> NextToken=..., IsAddressOfOperand=false, >> CCC=std::unique_ptr<clang::CorrectionCandidateCallback> containing >> 0x63c250) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Sema/SemaDecl.cpp:868 >> #23 0x00007ffff52c6a9f in clang::Parser::TryAnnotateName >> (this=this@entry=0x728c60, >> IsAddressOfOperand=IsAddressOfOperand@entry=false, >> CCC=std::unique_ptr<clang::CorrectionCandidateCallback> containing >> 0x0) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/Parser.cpp:1531 >> #24 0x00007ffff52aa56b in >> clang::Parser::ParseStatementOrDeclarationAfterAttributes >> (this=this@entry=0x728c60, Stmts=..., >> Allowed=Allowed@entry=clang::Parser::ACK_Any, >> TrailingElseLoc=<optimized out>, Attrs=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/ParseStmt.cpp:189 >> #25 0x00007ffff52aa843 in clang::Parser::ParseStatementOrDeclaration >> (this=this@entry=0x728c60, Stmts=..., >> Allowed=Allowed@entry=clang::Parser::ACK_Any, >> TrailingElseLoc=TrailingElseLoc@entry=0x0) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/ParseStmt.cpp:111 >> #26 0x00007ffff52ae577 in clang::Parser::ParseCompoundStatementBody >> (this=this@entry=0x728c60, isStmtExpr=isStmtExpr@entry=false) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/ParseStmt.cpp:1003 >> #27 0x00007ffff52b0a49 in clang::Parser::ParseFunctionStatementBody >> (this=this@entry=0x728c60, Decl=Decl@entry=0x72dbd0, BodyScope=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/ParseStmt.cpp:1972 >> #28 0x00007ffff52cc6e0 in clang::Parser::ParseFunctionDefinition >> (this=this@entry=0x728c60, D=..., TemplateInfo=..., >> LateParsedAttrs=LateParsedAttrs@entry=0x7fffffffabe0) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/Parser.cpp:1248 >> #29 0x00007ffff524aa56 in clang::Parser::ParseDeclGroup >> (this=this@entry=0x728c60, DS=..., >> Context=Context@entry=clang::DeclaratorContext::FileContext, >> DeclEnd=DeclEnd@entry=0x0, FRI=FRI@entry=0x0) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/ParseDecl.cpp:1968 >> #30 0x00007ffff52c7f90 in >> clang::Parser::ParseDeclOrFunctionDefInternal >> (this=this@entry=0x728c60, attrs=..., DS=..., >> AS=AS@entry=clang::AS_none) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/Parser.cpp:1012 >> #31 0x00007ffff52c8761 in >> clang::Parser::ParseDeclarationOrFunctionDefinition (this=0x728c60, >> attrs=..., AS=clang::AS_none, DS=0x0) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/Parser.cpp:1028 >> #32 0x00007ffff52c87bf in >> clang::Parser::ParseDeclarationOrFunctionDefinition (this=<optimized >> out>, attrs=..., DS=<optimized out>, AS=<optimized out>) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/Parser.cpp:1030 >> #33 0x00007ffff52cd05d in clang::Parser::ParseExternalDeclaration >> (this=this@entry=0x728c60, attrs=..., DS=DS@entry=0x0) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/Parser.cpp:853 >> #34 0x00007ffff52ce05c in clang::Parser::ParseTopLevelDecl >> (this=this@entry=0x728c60, Result=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/Parser.cpp:609 >> #35 0x00007ffff522ce1b in clang::ParseAST (S=..., >> PrintStats=PrintStats@entry=false, >> SkipFunctionBodies=SkipFunctionBodies@entry=false) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/ParseAST.cpp:146 >> #36 0x00007ffff522d0e0 in clang::ParseAST (PP=..., Consumer=0x535f70, >> Ctx=..., PrintStats=<optimized out>, TUKind=clang::TU_Complete, >> CompletionConsumer=0x0, >> SkipFunctionBodies=false) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/clang/lib/Parse/ParseAST.cpp:110 >> #37 0x00007ffff74f0059 in lldb_private::ClangExpressionParser::Parse >> (this=this@entry=0x7fffffffcae0, diagnostic_manager=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:619 >> #38 0x00007ffff74d23ad in lldb_private::ClangUserExpression::Parse >> (this=0x6d23c0, diagnostic_manager=..., exe_ctx=..., >> execution_policy=lldb_private::eExecutionPolicyOnlyWhenNeeded, >> keep_result_in_memory=<optimized out>, generate_debug_info=<optimized >> out>) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp:472 >> ---Type <return> to continue, or q <return> to quit--- >> #39 0x00007ffff72cea9e in lldb_private::UserExpression::Evaluate >> (exe_ctx=..., options=..., expr=..., prefix=..., result_valobj_sp=..., >> error=..., line_offset=0, >> fixed_expression=0x4bced8, jit_module_sp_ptr=0x0) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Expression/UserExpression.cpp:239 >> #40 0x00007ffff7401996 in lldb_private::Target::EvaluateExpression >> (this=this@entry=0x5b8d10, expr=..., exe_scope=<optimized out>, >> result_valobj_sp=..., >> options=..., fixed_expression=0x4bced8) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Target/Target.cpp:2321 >> #41 0x00007ffff77912dd in >> lldb_private::CommandObjectExpression::EvaluateExpression >> (this=this@entry=0x4bcb50, expr=expr@entry=0x7fffffffd4c0 "val.v", >> output_stream=output_stream@entry=0x7fffffffd5f0, >> error_stream=error_stream@entry=0x7fffffffd648, >> result=result@entry=0x7fffffffd5f0) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Commands/CommandObjectExpression.cpp:377 >> #42 0x00007ffff7792018 in >> lldb_private::CommandObjectExpression::DoExecute (this=0x4bcb50, >> command=0x7fffffffd4c0 "val.v", result=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Commands/CommandObjectExpression.cpp:625 >> #43 0x00007ffff7305baa in lldb_private::CommandObjectRaw::Execute >> (this=0x4bcb50, args_string=0x7fffffffd4c0 "val.v", result=...) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Interpreter/CommandObject.cpp:1013 >> #44 0x00007ffff7302e1b in >> lldb_private::CommandInterpreter::HandleCommand >> (this=this@entry=0x4a8740, command_line=<optimized out>, >> >> lazy_add_to_history=lazy_add_to_history@entry=lldb_private::eLazyBoolCalculate, >> result=..., override_context=override_context@entry=0x0, >> repeat_on_empty_command=repeat_on_empty_command@entry=true, >> no_context_switching=false) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Interpreter/CommandInterpreter.cpp:1683 >> #45 0x00007ffff73047c8 in >> lldb_private::CommandInterpreter::IOHandlerInputComplete >> (this=0x4a8740, io_handler=..., line="expr val.v") >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Interpreter/CommandInterpreter.cpp:2771 >> #46 0x00007ffff723e241 in lldb_private::IOHandlerEditline::Run >> (this=0x5b4da0) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Core/IOHandler.cpp:573 >> #47 0x00007ffff720a820 in lldb_private::Debugger::ExecuteIOHandlers >> (this=0x4a6f90) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Core/Debugger.cpp:961 >> #48 0x00007ffff72f731f in >> lldb_private::CommandInterpreter::RunCommandInterpreter >> (this=0x4a8740, auto_handle_events=auto_handle_events@entry=true, >> spawn_thread=spawn_thread@entry=false, options=...) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/Interpreter/CommandInterpreter.cpp:2971 >> #49 0x00007ffff7094776 in lldb::SBDebugger::RunCommandInterpreter >> (this=this@entry=0x7fffffffda60, >> auto_handle_events=auto_handle_events@entry=true, >> spawn_thread=spawn_thread@entry=false) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/source/API/SBDebugger.cpp:892 >> #50 0x0000000000404a76 in Driver::MainLoop (this=this@entry=0x7fffffffda40) >> at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/tools/driver/Driver.cpp:1156 >> #51 0x0000000000403724 in main (argc=2, argv=0x7fffffffdcb8) at >> /home/gerazo/repos/codechecker_dev_env/CTU/llvm/tools/lldb/tools/driver/Driver.cpp:1253 >> >> ``` >> On Tue, Aug 14, 2018 at 2:34 AM Richard Smith via cfe-dev >> <cfe-...@lists.llvm.org> wrote: >> > >> > On Mon, 13 Aug 2018 at 17:08, Lang Hames via cfe-dev >> > <cfe-...@lists.llvm.org> wrote: >> >> >> >> Hi Richard, >> >> >> >>> Perhaps a better approach would be to make the "minimal" mode for the >> >>> ASTImporter provide an ExternalASTSource to lazily complete the AST as >> >>> needed (thereby avoiding violating the invariant, because you would >> >>> populate the lexical declarations list whenever anyone actually asks for >> >>> it). >> >> >> >> >> >> This seems worth investigating. I wonder if it might also fix another bug >> >> that I know of involving virtual methods with covariant return types. >> >> (You and I actually discussed it at one of the socials a while back, but >> >> I have not had time to dig into it further.) >> >> >> >> The reproducer for the bug is: >> >> >> >> class Foo {}; >> >> class Bar : public Foo {}; >> >> >> >> class Base { >> >> public: >> >> virtual Foo* foo() { return nullptr; } >> >> }; >> >> >> >> class Derived : public Base { >> >> public: >> >> virtual Bar* foo() { return nullptr; } >> >> }; >> >> >> >> int main() { >> >> Derived D; >> >> D.foo(); // Evaluate 'D.foo()' here, crash LLDB. >> >> } >> >> >> >> The issue is that since Bar's definition is not used its bases are never >> >> imported, and so the call to Bar::bases() crashes in CodeGen. If we >> >> provided an ExternalASTSource, would that be able to lazily complete the >> >> bases? >> > >> > >> > Yes, such an approach should be able to solve that problem too: when >> > CodeGen (or indeed anything) queries any property of the definition of Foo >> > / Bar (including whether a definition exists), you'll get a callback and a >> > chance to provide a complete type. >> > >> >> >> >> Cheers, >> >> Lang. >> >> >> >> >> >> On Mon, Aug 13, 2018 at 3:30 PM Richard Smith <rich...@metafoo.co.uk> >> >> wrote: >> >>> >> >>> On Thu, 9 Aug 2018 at 10:47, Lang Hames via cfe-dev >> >>> <cfe-...@lists.llvm.org> wrote: >> >>>> >> >>>> Hi clang-dev, lldb-dev, >> >>>> >> >>>> It looks like my clang commit r305850, which modified ASTImporter to >> >>>> import method override tables from an external context, introduced a >> >>>> new bug which manifests as incorrect vtable layouts for LLDB expression >> >>>> code. >> >>>> >> >>>> The bug itself is fairly straightforward. In r305850 I introduced the >> >>>> following method, which is called from >> >>>> ASTNodeImporter::VisitFunctionDecl: >> >>>> >> >>>> void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod, >> >>>> CXXMethodDecl *FromMethod) { >> >>>> for (auto *FromOverriddenMethod : FromMethod->overridden_methods()) >> >>>> ToMethod->addOverriddenMethod( >> >>>> cast<CXXMethodDecl>(Importer.Import(const_cast<CXXMethodDecl*>( >> >>>> FromOverriddenMethod)))); >> >>>> } >> >>>> >> >>>> This will produce the correct override table, but can also cause >> >>>> methods in the Base class to be visited in the wrong order. Consider: >> >>>> >> >>>> class Base { >> >>>> public: >> >>>> virtual void bar() {} >> >>>> virtual void foo() {} >> >>>> }; >> >>>> >> >>>> class Derived : public Base { >> >>>> public: >> >>>> void foo() override {} >> >>>> }; >> >>>> >> >>>> If Derived is imported into a new context before Base, then the >> >>>> importer will visit Derived::foo, and (via ImportOverrides) immediately >> >>>> import Base::foo, but this happens before Base::bar is imported. As a >> >>>> consequence, the decl order on the imported Base class will end up >> >>>> being [ foo, bar ], instead of [ bar, foo ]. In LLDB expression >> >>>> evaluation this manifests as an incorrect vtable layout for Base, with >> >>>> foo occupying the first slot. >> >>>> >> >>>> I am looking for suggestions on the right way to fix this. A brute >> >>>> force solution might be to always have ASTNodeImporter::VisitRecordDecl >> >>>> visit all base classes, then all virtual methods, which would ensure >> >>>> they are visited in the original decl order. However I do not know >> >>>> whether this covers all paths by which a CXXRecordDecl might be >> >>>> imported, nor whether the performance of this solution would be >> >>>> acceptable (it seems like it would preclude a lot of laziness). >> >>>> >> >>>> Alternatively we might be able to associate an index with each imported >> >>>> decl and sort on that when we complete the type, but that would leave >> >>>> imported decls in the wrong order until the type was complete, and >> >>>> since I do not know all the use cases for the importer I'm concerned >> >>>> people may rely on the decl order before type is complete. >> >>>> >> >>>> Any insight from ASTImporter experts would be greatly appreciated. :) >> >>> >> >>> >> >>> Hi Lang, >> >>> >> >>> It might be interesting to consider how this is addressed by the >> >>> ExternalASTSource interface. There, we have separate "import the lexical >> >>> contents of this AST node (including populating the lexical declarations >> >>> list with all members, in the right order)", "import the lookup results >> >>> for this name in this context (and cache them but don't add them to the >> >>> list of lexical members)" and "import this specific declaration member >> >>> (but don't add it to the list of lexical members)" queries. One could >> >>> imagine doing something similar for the AST importer: when you're >> >>> performing a minimal import but you want to import a method declaration, >> >>> don't insert it into the list of lexical members of the enclosing >> >>> record. Instead, defer doing that until a complete type is required (at >> >>> that point, you'd need to import all the relevant members anyway, >> >>> including the base classes and virtual functions in the right order). >> >>> >> >>> The above approach would violate AST invariants (you'd have a >> >>> declaration whose lexical parent doesn't believe it lexically contains >> >>> the child), but I don't know off-hand whether that would be problematic. >> >>> Perhaps a better approach would be to make the "minimal" mode for the >> >>> ASTImporter provide an ExternalASTSource to lazily complete the AST as >> >>> needed (thereby avoiding violating the invariant, because you would >> >>> populate the lexical declarations list whenever anyone actually asks for >> >>> it). >> >> >> >> _______________________________________________ >> >> cfe-dev mailing list >> >> cfe-...@lists.llvm.org >> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev >> > >> > _______________________________________________ >> > cfe-dev mailing list >> > cfe-...@lists.llvm.org >> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev _______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev