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()) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
