diff options
| -rw-r--r-- | clang/lib/AST/ASTImporter.cpp | 35 | ||||
| -rw-r--r-- | clang/unittests/AST/ASTImporterTest.cpp | 48 | ||||
| -rw-r--r-- | lldb/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py | 14 | ||||
| -rw-r--r-- | lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c | 4 | ||||
| -rw-r--r-- | lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp | 30 |
5 files changed, 110 insertions, 21 deletions
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 8a59c3a7699..9d5dd84161d 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -1707,6 +1707,17 @@ static Error setTypedefNameForAnonDecl(TagDecl *From, TagDecl *To, Error ASTNodeImporter::ImportDefinition( RecordDecl *From, RecordDecl *To, ImportDefinitionKind Kind) { + auto DefinitionCompleter = [To]() { + // There are cases in LLDB when we first import a class without its + // members. The class will have DefinitionData, but no members. Then, + // importDefinition is called from LLDB, which tries to get the members, so + // when we get here, the class already has the DefinitionData set, so we + // must unset the CompleteDefinition here to be able to complete again the + // definition. + To->setCompleteDefinition(false); + To->completeDefinition(); + }; + if (To->getDefinition() || To->isBeingDefined()) { if (Kind == IDK_Everything || // In case of lambdas, the class already has a definition ptr set, but @@ -1717,7 +1728,7 @@ Error ASTNodeImporter::ImportDefinition( Error Result = ImportDeclContext(From, /*ForceImport=*/true); // Finish the definition of the lambda, set isBeingDefined to false. if (To->isLambda()) - To->completeDefinition(); + DefinitionCompleter(); return Result; } @@ -1728,8 +1739,8 @@ Error ASTNodeImporter::ImportDefinition( // Complete the definition even if error is returned. // The RecordDecl may be already part of the AST so it is better to // have it in complete state even if something is wrong with it. - auto DefinitionCompleter = - llvm::make_scope_exit([To]() { To->completeDefinition(); }); + auto DefinitionCompleterScopeExit = + llvm::make_scope_exit(DefinitionCompleter); if (Error Err = setTypedefNameForAnonDecl(From, To, Importer)) return Err; @@ -7757,10 +7768,20 @@ ASTImporter::findDeclsInToCtx(DeclContext *DC, DeclarationName Name) { SharedState->getLookupTable()->lookup(ReDC, Name); return FoundDeclsTy(LookupResult.begin(), LookupResult.end()); } else { - // FIXME Can we remove this kind of lookup? - // Or lldb really needs this C/C++ lookup? - FoundDeclsTy Result; - ReDC->localUncachedLookup(Name, Result); + DeclContext::lookup_result NoloadLookupResult = ReDC->noload_lookup(Name); + FoundDeclsTy Result(NoloadLookupResult.begin(), NoloadLookupResult.end()); + // We must search by the slow case of localUncachedLookup because that is + // working even if there is no LookupPtr for the DC. We could use + // DC::buildLookup() to create the LookupPtr, but that would load external + // decls again, we must avoid that case. + // Also, even if we had the LookupPtr, we must find Decls which are not + // in the LookupPtr, so we need the slow case. + // These cases are handled in ASTImporterLookupTable, but we cannot use + // that with LLDB since that traverses through the AST which initiates the + // load of external decls again via DC::decls(). And again, we must avoid + // loading external decls during the import. + if (Result.empty()) + ReDC->localUncachedLookup(Name, Result); return Result; } } diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 6b8315f2b9e..8b2f7c5b240 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -5122,6 +5122,51 @@ TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionParam) { EXPECT_EQ(ToLSize, FromLSize); } +struct LLDBLookupTest : ASTImporterOptionSpecificTestBase { + LLDBLookupTest() { + Creator = [](ASTContext &ToContext, FileManager &ToFileManager, + ASTContext &FromContext, FileManager &FromFileManager, + bool MinimalImport, + const std::shared_ptr<ASTImporterSharedState> &SharedState) { + return new ASTImporter(ToContext, ToFileManager, FromContext, + FromFileManager, MinimalImport, + // We use the regular lookup. + /*SharedState=*/nullptr); + }; + } +}; + +TEST_P(LLDBLookupTest, ImporterShouldFindInTransparentContext) { + TranslationUnitDecl *ToTU = getToTuDecl( + R"( + extern "C" { + class X{}; + }; + )", + Lang_CXX); + auto *ToX = FirstDeclMatcher<CXXRecordDecl>().match( + ToTU, cxxRecordDecl(hasName("X"))); + + // Set up a stub external storage. + ToTU->setHasExternalLexicalStorage(true); + // Set up DeclContextBits.HasLazyExternalLexicalLookups to true. + ToTU->setMustBuildLookupTable(); + struct TestExternalASTSource : ExternalASTSource {}; + ToTU->getASTContext().setExternalSource(new TestExternalASTSource()); + + Decl *FromTU = getTuDecl( + R"( + class X; + )", + Lang_CXX); + auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match( + FromTU, cxxRecordDecl(hasName("X"))); + auto *ImportedX = Import(FromX, Lang_CXX); + // The lookup must find the existing class definition in the LinkageSpecDecl. + // Then the importer renders the existing and the new decl into one chain. + EXPECT_EQ(ImportedX->getCanonicalDecl(), ToX->getCanonicalDecl()); +} + INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest, DefaultTestValuesForRunOptions, ); @@ -5168,5 +5213,8 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportImplicitMethods, INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportVariables, DefaultTestValuesForRunOptions, ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, LLDBLookupTest, + DefaultTestValuesForRunOptions, ); + } // end namespace ast_matchers } // end namespace clang diff --git a/lldb/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py b/lldb/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py index 455704280d1..857223b5ed1 100644 --- a/lldb/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py +++ b/lldb/packages/Python/lldbsuite/test/lang/c/modules/TestCModules.py @@ -47,6 +47,10 @@ class CModulesTestCase(TestBase): self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE, substrs=[' resolved, hit count = 1']) + # Enable logging of the imported AST. + log_file = os.path.join(self.getBuildDir(), "lldb-ast-log.txt") + self.runCmd("log enable lldb ast -f '%s'" % log_file) + self.expect( "expr -l objc++ -- @import Darwin; 3", VARIABLES_DISPLAYED_CORRECTLY, @@ -54,6 +58,8 @@ class CModulesTestCase(TestBase): "int", "3"]) + # This expr command imports __sFILE with definition + # (FILE is a typedef to __sFILE.) self.expect( "expr *fopen(\"/dev/zero\", \"w\")", VARIABLES_DISPLAYED_CORRECTLY, @@ -61,6 +67,14 @@ class CModulesTestCase(TestBase): "FILE", "_close"]) + # Check that the AST log contains exactly one definition of __sFILE. + f = open(log_file) + log_lines = f.readlines() + f.close() + os.remove(log_file) + self.assertEqual(" ".join(log_lines).count("struct __sFILE definition"), + 1) + self.expect("expr *myFile", VARIABLES_DISPLAYED_CORRECTLY, substrs=["a", "5", "b", "9"]) diff --git a/lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c b/lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c index 2b244bc38d0..df321a75faa 100644 --- a/lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c +++ b/lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c @@ -5,11 +5,11 @@ int printf(const char * __restrict format, ...); typedef struct { int a; int b; -} FILE; +} MYFILE; int main() { - FILE *myFile = malloc(sizeof(FILE)); + MYFILE *myFile = malloc(sizeof(MYFILE)); myFile->a = 5; myFile->b = 9; diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp index 8d29df9dde2..c5778f86bb6 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp @@ -612,10 +612,15 @@ void ClangASTSource::FindExternalLexicalDecls( if (!original_decl_context) return; + // Indicates whether we skipped any Decls of the original DeclContext. + bool SkippedDecls = false; for (TagDecl::decl_iterator iter = original_decl_context->decls_begin(); iter != original_decl_context->decls_end(); ++iter) { Decl *decl = *iter; + // The predicate function returns true if the passed declaration kind is + // the one we are looking for. + // See clang::ExternalASTSource::FindExternalLexicalDecls() if (predicate(decl->getKind())) { if (log) { ASTDumper ast_dumper(decl); @@ -640,21 +645,22 @@ void ClangASTSource::FindExternalLexicalDecls( m_ast_importer_sp->RequireCompleteType(copied_field_type); } - - DeclContext *decl_context_non_const = - const_cast<DeclContext *>(decl_context); - - if (copied_decl->getDeclContext() != decl_context) { - if (copied_decl->getDeclContext()->containsDecl(copied_decl)) - copied_decl->getDeclContext()->removeDecl(copied_decl); - copied_decl->setDeclContext(decl_context_non_const); - } - - if (!decl_context_non_const->containsDecl(copied_decl)) - decl_context_non_const->addDeclInternal(copied_decl); + } else { + SkippedDecls = true; } } + // CopyDecl may build a lookup table which may set up ExternalLexicalStorage + // to false. However, since we skipped some of the external Decls we must + // set it back! + if (SkippedDecls) { + decl_context->setHasExternalLexicalStorage(true); + // This sets HasLazyExternalLexicalLookups to true. By setting this bit we + // ensure that the lookup table is rebuilt, which means the external source + // is consulted again when a clang::DeclContext::lookup is called. + const_cast<DeclContext *>(decl_context)->setMustBuildLookupTable(); + } + return; } |
