shafik updated this revision to Diff 259110. shafik marked 2 inline comments as done. shafik added a comment.
- Simplifying the changes in `ASTImporter::ImportContext` - Removing `DC->setHasExternalLexicalStorage(true);` from unit test since we are checking `getExternalSource()` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78000/new/ https://reviews.llvm.org/D78000 Files: clang/lib/AST/ASTImporter.cpp clang/unittests/AST/ASTImporterTest.cpp lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp
Index: lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp =================================================================== --- /dev/null +++ lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp @@ -0,0 +1,35 @@ +struct B { + int dump() const; +}; + +int B::dump() const { return 42; } + +// Derived class D obtains dump() method from base class B +struct D : public B { + // Introduce a TypedefNameDecl + using Iterator = D *; +}; + +struct C { + // This will cause use to invoke VisitTypedefNameDecl(...) when Importing + // the DeclContext of C. + // We will invoke ImportContext(...) which should force the From Decl to + // be defined if it not already defined. We will then Import the definition + // to the To Decl. + // This will force processing of the base class of D which allows us to see + // base class methods such as dump(). + D::Iterator iter; + + bool f(D *DD) { + return true; //%self.expect_expr("DD->dump()", result_type="int", result_value="42") + } +}; + +int main() { + C c; + D d; + + c.f(&d); + + return 0; +} Index: lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py =================================================================== --- /dev/null +++ lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py @@ -0,0 +1,4 @@ +from lldbsuite.test import lldbinline +from lldbsuite.test import decorators + +lldbinline.MakeInlineTest(__file__, globals()) Index: clang/unittests/AST/ASTImporterTest.cpp =================================================================== --- clang/unittests/AST/ASTImporterTest.cpp +++ clang/unittests/AST/ASTImporterTest.cpp @@ -5922,6 +5922,70 @@ EXPECT_TRUE(ToA); } +struct ImportWithExternalSource : ASTImporterOptionSpecificTestBase { + ImportWithExternalSource() { + 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); + }; + } +}; + +/// An ExternalASTSource that keeps track of the tags is completed. +struct SourceWithCompletedTagList : clang::ExternalASTSource { + std::vector<clang::TagDecl *> &CompletedTags; + SourceWithCompletedTagList(std::vector<clang::TagDecl *> &CompletedTags) + : CompletedTags(CompletedTags) {} + void CompleteType(TagDecl *Tag) override { + auto *Record = cast<CXXRecordDecl>(Tag); + Record->startDefinition(); + Record->completeDefinition(); + CompletedTags.push_back(Tag); + } + void + FindExternalLexicalDecls(const DeclContext *DC, + llvm::function_ref<bool(Decl::Kind)> IsKindWeWant, + SmallVectorImpl<Decl *> &Result) override {} +}; + +TEST_P(ImportWithExternalSource, CompleteRecordBeforeImporting) { + // Create an empty TU. + TranslationUnitDecl *FromTU = getTuDecl("", Lang_CXX, "input.cpp"); + + // Create and add the test ExternalASTSource. + std::vector<clang::TagDecl *> CompletedTags; + IntrusiveRefCntPtr<ExternalASTSource> source = + new SourceWithCompletedTagList(CompletedTags); + clang::ASTContext &context = FromTU->getASTContext(); + context.setExternalSource(std::move(source)); + + // Create a dummy class by hand with external lexical storage. + IdentifierInfo &Ident = context.Idents.get("test_class"); + auto *Record = CXXRecordDecl::Create( + context, TTK_Class, FromTU, SourceLocation(), SourceLocation(), &Ident); + Record->setHasExternalLexicalStorage(); + FromTU->addDecl(Record); + + // Do a minimal import of the created class. + EXPECT_EQ(0U, CompletedTags.size()); + Import(Record, Lang_CXX); + EXPECT_EQ(0U, CompletedTags.size()); + + // Import the definition of the created class. + llvm::Error err = findFromTU(Record)->Importer->ImportDefinition(Record); + EXPECT_FALSE((bool)err); + consumeError(std::move(err)); + + // Make sure the class was completed once. + EXPECT_EQ(1U, CompletedTags.size()); + EXPECT_EQ(Record, CompletedTags.front()); +} + INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest, DefaultTestValuesForRunOptions, ); @@ -5983,5 +6047,8 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportSourceLocations, DefaultTestValuesForRunOptions, ); +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportWithExternalSource, + DefaultTestValuesForRunOptions, ); + } // end namespace ast_matchers } // end namespace clang Index: clang/lib/AST/ASTImporter.cpp =================================================================== --- clang/lib/AST/ASTImporter.cpp +++ clang/lib/AST/ASTImporter.cpp @@ -8159,15 +8159,22 @@ // need it to have a definition. if (auto *ToRecord = dyn_cast<RecordDecl>(ToDC)) { auto *FromRecord = cast<RecordDecl>(FromDC); - if (ToRecord->isCompleteDefinition()) { - // Do nothing. - } else if (FromRecord->isCompleteDefinition()) { + if (ToRecord->isCompleteDefinition()) + return ToDC; + + // If FromRecord is not defined we need to force it to be. + // Simply calling CompleteDecl(...) for a RecordDecl will break some cases + // it will start the definition but we never finish it. + // If there are base classes they won't be imported and we will + // be missing anything that we inherit from those bases. + if (FromRecord->getASTContext().getExternalSource() && + !FromRecord->isCompleteDefinition()) + FromRecord->getASTContext().getExternalSource()->CompleteType(FromRecord); + + if (FromRecord->isCompleteDefinition()) if (Error Err = ASTNodeImporter(*this).ImportDefinition( FromRecord, ToRecord, ASTNodeImporter::IDK_Basic)) return std::move(Err); - } else { - CompleteDecl(ToRecord); - } } else if (auto *ToEnum = dyn_cast<EnumDecl>(ToDC)) { auto *FromEnum = cast<EnumDecl>(FromDC); if (ToEnum->isCompleteDefinition()) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits