summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--clang/include/clang/AST/ASTImporter.h22
-rw-r--r--clang/lib/AST/ASTImporter.cpp98
-rw-r--r--clang/test/ASTMerge/class-template-partial-spec/test.cpp2
-rw-r--r--clang/unittests/AST/ASTImporterFixtures.cpp2
-rw-r--r--clang/unittests/AST/ASTImporterFixtures.h4
-rw-r--r--clang/unittests/AST/ASTImporterTest.cpp121
6 files changed, 226 insertions, 23 deletions
diff --git a/clang/include/clang/AST/ASTImporter.h b/clang/include/clang/AST/ASTImporter.h
index b0dcece5ce9..14fb93d1a83 100644
--- a/clang/include/clang/AST/ASTImporter.h
+++ b/clang/include/clang/AST/ASTImporter.h
@@ -116,6 +116,14 @@ class TypeSourceInfo;
/// context to the corresponding declarations in the "to" context.
llvm::DenseMap<Decl *, Decl *> ImportedDecls;
+ /// Mapping from the already-imported declarations in the "from"
+ /// context to the error status of the import of that declaration.
+ /// This map contains only the declarations that were not correctly
+ /// imported. The same declaration may or may not be included in
+ /// ImportedDecls. This map is updated continuously during imports and never
+ /// cleared (like ImportedDecls).
+ llvm::DenseMap<Decl *, ImportError> ImportDeclErrors;
+
/// Mapping from the already-imported declarations in the "to"
/// context to the corresponding declarations in the "from" context.
llvm::DenseMap<Decl *, Decl *> ImportedFromDecls;
@@ -148,6 +156,9 @@ class TypeSourceInfo;
/// decl on its own.
virtual Expected<Decl *> ImportImpl(Decl *From);
+ /// Used only in unittests to verify the behaviour of the error handling.
+ virtual bool returnWithErrorInTest() { return false; };
+
public:
/// \param ToContext The context we'll be importing into.
@@ -394,6 +405,8 @@ class TypeSourceInfo;
void RegisterImportedDecl(Decl *FromD, Decl *ToD);
/// Store and assign the imported declaration to its counterpart.
+ /// It may happen that several decls from the 'from' context are mapped to
+ /// the same decl in the 'to' context.
Decl *MapImported(Decl *From, Decl *To);
/// Called by StructuralEquivalenceContext. If a RecordDecl is
@@ -404,6 +417,14 @@ class TypeSourceInfo;
/// importation, eliminating this loop.
virtual Decl *GetOriginalDecl(Decl *To) { return nullptr; }
+ /// Return if import of the given declaration has failed and if yes
+ /// the kind of the problem. This gives the first error encountered with
+ /// the node.
+ llvm::Optional<ImportError> getImportDeclErrorIfAny(Decl *FromD) const;
+
+ /// Mark (newly) imported declaration with error.
+ void setImportDeclError(Decl *From, ImportError Error);
+
/// Determine whether the given types are structurally
/// equivalent.
bool IsStructurallyEquivalent(QualType From, QualType To,
@@ -414,7 +435,6 @@ class TypeSourceInfo;
/// \returns The index of the field in its parent context (starting from 0).
/// On error `None` is returned (parent context is non-record).
static llvm::Optional<unsigned> getFieldIndex(Decl *F);
-
};
} // namespace clang
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index d3c79eac903..edefa45fb56 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -253,11 +253,10 @@ namespace clang {
LLVM_NODISCARD bool
GetImportedOrCreateSpecialDecl(ToDeclT *&ToD, CreateFunT CreateFun,
FromDeclT *FromD, Args &&... args) {
- // FIXME: This code is needed later.
- //if (Importer.getImportDeclErrorIfAny(FromD)) {
- // ToD = nullptr;
- // return true; // Already imported but with error.
- //}
+ if (Importer.getImportDeclErrorIfAny(FromD)) {
+ ToD = nullptr;
+ return true; // Already imported but with error.
+ }
ToD = cast_or_null<ToDeclT>(Importer.GetAlreadyImportedOrNull(FromD));
if (ToD)
return true; // Already imported.
@@ -5113,7 +5112,7 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl(
}
} else { // ODR violation.
// FIXME HandleNameConflict
- return nullptr;
+ return make_error<ImportError>(ImportError::NameConflict);
}
}
@@ -5555,6 +5554,8 @@ ExpectedStmt ASTNodeImporter::VisitStmt(Stmt *S) {
ExpectedStmt ASTNodeImporter::VisitGCCAsmStmt(GCCAsmStmt *S) {
+ if (Importer.returnWithErrorInTest())
+ return make_error<ImportError>(ImportError::UnsupportedConstruct);
SmallVector<IdentifierInfo *, 4> Names;
for (unsigned I = 0, E = S->getNumOutputs(); I != E; I++) {
IdentifierInfo *ToII = Importer.Import(S->getOutputIdentifier(I));
@@ -7749,7 +7750,6 @@ Expected<Decl *> ASTImporter::ImportImpl(Decl *FromD) {
void ASTImporter::RegisterImportedDecl(Decl *FromD, Decl *ToD) {
MapImported(FromD, ToD);
- AddToLookupTable(ToD);
}
Expected<QualType> ASTImporter::Import(QualType FromT) {
@@ -7822,6 +7822,11 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) {
return nullptr;
+ // Check whether there was a previous failed import.
+ // If yes return the existing error.
+ if (auto Error = getImportDeclErrorIfAny(FromD))
+ return make_error<ImportError>(*Error);
+
// Check whether we've already imported this declaration.
Decl *ToD = GetAlreadyImportedOrNull(FromD);
if (ToD) {
@@ -7832,24 +7837,65 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) {
// Import the declaration.
ExpectedDecl ToDOrErr = ImportImpl(FromD);
- if (!ToDOrErr)
- return ToDOrErr;
+ if (!ToDOrErr) {
+ // Failed to import.
+
+ auto Pos = ImportedDecls.find(FromD);
+ if (Pos != ImportedDecls.end()) {
+ // Import failed after the object was created.
+ // Remove all references to it.
+ auto *ToD = Pos->second;
+ ImportedDecls.erase(Pos);
+
+ // ImportedDecls and ImportedFromDecls are not symmetric. It may happen
+ // (e.g. with namespaces) that several decls from the 'from' context are
+ // mapped to the same decl in the 'to' context. If we removed entries
+ // from the LookupTable here then we may end up removing them multiple
+ // times.
+
+ // The Lookuptable contains decls only which are in the 'to' context.
+ // Remove from the Lookuptable only if it is *imported* into the 'to'
+ // context (and do not remove it if it was added during the initial
+ // traverse of the 'to' context).
+ auto PosF = ImportedFromDecls.find(ToD);
+ if (PosF != ImportedFromDecls.end()) {
+ if (LookupTable)
+ if (auto *ToND = dyn_cast<NamedDecl>(ToD))
+ LookupTable->remove(ToND);
+ ImportedFromDecls.erase(PosF);
+ }
+
+ // FIXME: AST may contain remaining references to the failed object.
+ }
+
+ // Error encountered for the first time.
+ assert(!getImportDeclErrorIfAny(FromD) &&
+ "Import error already set for Decl.");
+
+ // After takeError the error is not usable any more in ToDOrErr.
+ // Get a copy of the error object (any more simple solution for this?).
+ ImportError ErrOut;
+ handleAllErrors(ToDOrErr.takeError(),
+ [&ErrOut](const ImportError &E) { ErrOut = E; });
+ setImportDeclError(FromD, ErrOut);
+ // Do not return ToDOrErr, error was taken out of it.
+ return make_error<ImportError>(ErrOut);
+ }
+
ToD = *ToDOrErr;
- // FIXME Use getImportDeclErrorIfAny() here (and return with the error) once
- // the error handling is finished in GetImportedOrCreateSpecialDecl().
+ // FIXME: Handle the "already imported with error" case. We can get here
+ // nullptr only if GetImportedOrCreateDecl returned nullptr (after a
+ // previously failed create was requested).
+ // Later GetImportedOrCreateDecl can be updated to return the error.
if (!ToD) {
- return nullptr;
+ auto Err = getImportDeclErrorIfAny(FromD);
+ assert(Err);
+ return make_error<ImportError>(*Err);
}
// Make sure that ImportImpl registered the imported decl.
assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
-
- // Once the decl is connected to the existing declarations, i.e. when the
- // redecl chain is properly set then we populate the lookup again.
- // This way the primary context will be able to find all decls.
- AddToLookupTable(ToD);
-
// Notify subclasses.
Imported(FromD, ToD);
@@ -8560,9 +8606,25 @@ Decl *ASTImporter::MapImported(Decl *From, Decl *To) {
// This mapping should be maintained only in this function. Therefore do not
// check for additional consistency.
ImportedFromDecls[To] = From;
+ AddToLookupTable(To);
return To;
}
+llvm::Optional<ImportError>
+ASTImporter::getImportDeclErrorIfAny(Decl *FromD) const {
+ auto Pos = ImportDeclErrors.find(FromD);
+ if (Pos != ImportDeclErrors.end())
+ return Pos->second;
+ else
+ return Optional<ImportError>();
+}
+
+void ASTImporter::setImportDeclError(Decl *From, ImportError Error) {
+ assert(ImportDeclErrors.find(From) == ImportDeclErrors.end() &&
+ "Setting import error allowed only once for a Decl.");
+ ImportDeclErrors[From] = Error;
+}
+
bool ASTImporter::IsStructurallyEquivalent(QualType From, QualType To,
bool Complain) {
llvm::DenseMap<const Type *, const Type *>::iterator Pos =
diff --git a/clang/test/ASTMerge/class-template-partial-spec/test.cpp b/clang/test/ASTMerge/class-template-partial-spec/test.cpp
index fdd0ebd4884..6ab5ed5a7d5 100644
--- a/clang/test/ASTMerge/class-template-partial-spec/test.cpp
+++ b/clang/test/ASTMerge/class-template-partial-spec/test.cpp
@@ -1,5 +1,3 @@
-// FIXME: Crashes after r357394
-// XFAIL: *
// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.1.ast %S/Inputs/class-template-partial-spec1.cpp
// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.2.ast %S/Inputs/class-template-partial-spec2.cpp
// RUN: %clang_cc1 -std=c++1z -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
diff --git a/clang/unittests/AST/ASTImporterFixtures.cpp b/clang/unittests/AST/ASTImporterFixtures.cpp
index a0273b617e5..b2b848f5e57 100644
--- a/clang/unittests/AST/ASTImporterFixtures.cpp
+++ b/clang/unittests/AST/ASTImporterFixtures.cpp
@@ -161,7 +161,7 @@ TranslationUnitDecl *ASTImporterTestBase::getTuDecl(StringRef SrcCode,
}) == FromTUs.end());
ArgVector Args = getArgVectorForLanguage(Lang);
- FromTUs.emplace_back(SrcCode, FileName, Args);
+ FromTUs.emplace_back(SrcCode, FileName, Args, Creator);
TU &Tu = FromTUs.back();
return Tu.TUDecl;
diff --git a/clang/unittests/AST/ASTImporterFixtures.h b/clang/unittests/AST/ASTImporterFixtures.h
index cfe302124ff..4e666e37aa3 100644
--- a/clang/unittests/AST/ASTImporterFixtures.h
+++ b/clang/unittests/AST/ASTImporterFixtures.h
@@ -124,7 +124,6 @@ private:
void lazyInitLookupTable(TranslationUnitDecl *ToTU);
void lazyInitToAST(Language ToLang, StringRef ToSrcCode, StringRef FileName);
- TU *findFromTU(Decl *From);
protected:
std::unique_ptr<ASTImporterLookupTable> LookupTablePtr;
@@ -133,6 +132,9 @@ public:
// We may have several From context but only one To context.
std::unique_ptr<ASTUnit> ToAST;
+ // Returns with the TU associated with the given Decl.
+ TU *findFromTU(Decl *From);
+
// Creates an AST both for the From and To source code and imports the Decl
// of the identifier into the To context.
// Must not be called more than once within the same test.
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 63bcc609f06..41a041f101e 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -4620,6 +4620,127 @@ TEST_P(ImportFriendFunctionTemplates, LookupShouldFindPreviousFriend) {
EXPECT_EQ(Imported->getPreviousDecl(), Friend);
}
+struct ASTImporterWithFakeErrors : ASTImporter {
+ using ASTImporter::ASTImporter;
+ bool returnWithErrorInTest() override { return true; }
+};
+
+struct ErrorHandlingTest : ASTImporterOptionSpecificTestBase {
+ ErrorHandlingTest() {
+ Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+ return new ASTImporterWithFakeErrors(ToContext, ToFileManager,
+ FromContext, FromFileManager,
+ MinimalImport, LookupTable);
+ };
+ }
+ // In this test we purposely report an error (UnsupportedConstruct) when
+ // importing the below stmt.
+ static constexpr auto* ErroneousStmt = R"( asm(""); )";
+};
+
+// Check a case when no new AST node is created in the AST before encountering
+// the error.
+TEST_P(ErrorHandlingTest, ErrorHappensBeforeCreatingANewNode) {
+ TranslationUnitDecl *ToTU = getToTuDecl(
+ R"(
+ template <typename T>
+ class X {};
+ template <>
+ class X<int> { int a; };
+ )",
+ Lang_CXX);
+ TranslationUnitDecl *FromTU = getTuDecl(
+ R"(
+ template <typename T>
+ class X {};
+ template <>
+ class X<int> { double b; };
+ )",
+ Lang_CXX);
+ auto *FromSpec = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match(
+ FromTU, classTemplateSpecializationDecl(hasName("X")));
+ ClassTemplateSpecializationDecl *ImportedSpec = Import(FromSpec, Lang_CXX);
+ EXPECT_FALSE(ImportedSpec);
+
+ // The original Decl is kept, no new decl is created.
+ EXPECT_EQ(DeclCounter<ClassTemplateSpecializationDecl>().match(
+ ToTU, classTemplateSpecializationDecl(hasName("X"))),
+ 1u);
+
+ // But an error is set to the counterpart in the "from" context.
+ ASTImporter *Importer = findFromTU(FromSpec)->Importer.get();
+ Optional<ImportError> OptErr = Importer->getImportDeclErrorIfAny(FromSpec);
+ ASSERT_TRUE(OptErr);
+ EXPECT_EQ(OptErr->Error, ImportError::NameConflict);
+}
+
+// Check a case when a new AST node is created but not linked to the AST before
+// encountering the error.
+TEST_P(ErrorHandlingTest,
+ ErrorHappensAfterCreatingTheNodeButBeforeLinkingThatToTheAST) {
+ TranslationUnitDecl *FromTU = getTuDecl(
+ std::string("void foo() { ") + ErroneousStmt + " }",
+ Lang_CXX);
+ auto *FromFoo = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("foo")));
+
+ FunctionDecl *ImportedFoo = Import(FromFoo, Lang_CXX);
+ EXPECT_FALSE(ImportedFoo);
+
+ TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+ // Created, but not linked.
+ EXPECT_EQ(
+ DeclCounter<FunctionDecl>().match(ToTU, functionDecl(hasName("foo"))),
+ 0u);
+
+ ASTImporter *Importer = findFromTU(FromFoo)->Importer.get();
+ Optional<ImportError> OptErr = Importer->getImportDeclErrorIfAny(FromFoo);
+ ASSERT_TRUE(OptErr);
+ EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+}
+
+// Check a case when a new AST node is created and linked to the AST before
+// encountering the error. The error is set for the counterpart of the nodes in
+// the "from" context.
+TEST_P(ErrorHandlingTest, ErrorHappensAfterNodeIsCreatedAndLinked) {
+ TranslationUnitDecl *FromTU = getTuDecl(
+ std::string(R"(
+ void f();
+ void f() { )") + ErroneousStmt + R"( }
+ )",
+ Lang_CXX);
+ auto *FromProto = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f")));
+ auto *FromDef =
+ LastDeclMatcher<FunctionDecl>().match(FromTU, functionDecl(hasName("f")));
+ FunctionDecl *ImportedProto = Import(FromProto, Lang_CXX);
+ EXPECT_FALSE(ImportedProto); // Could not import.
+ // However, we created two nodes in the AST. 1) the fwd decl 2) the
+ // definition. The definition is not added to its DC, but the fwd decl is
+ // there.
+ TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+ EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, functionDecl(hasName("f"))),
+ 1u);
+ // Match the fwd decl.
+ auto *ToProto =
+ FirstDeclMatcher<FunctionDecl>().match(ToTU, functionDecl(hasName("f")));
+ EXPECT_TRUE(ToProto);
+ // An error is set to the counterpart in the "from" context both for the fwd
+ // decl and the definition.
+ ASTImporter *Importer = findFromTU(FromProto)->Importer.get();
+ Optional<ImportError> OptErr = Importer->getImportDeclErrorIfAny(FromProto);
+ ASSERT_TRUE(OptErr);
+ EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+ OptErr = Importer->getImportDeclErrorIfAny(FromDef);
+ ASSERT_TRUE(OptErr);
+ EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+}
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ErrorHandlingTest,
+ DefaultTestValuesForRunOptions, );
+
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions, );
OpenPOWER on IntegriCloud