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

Reply via email to