[ASTImporter] Mark erroneous nodes in shared st
Summary:
Now we store the errors for the Decls in the "to" context too. For
that, however, we have to put these errors in a shared state (among all
the ASTImporter objects which handle the same "to" context but different
"from" contexts).
After a series of imports from different "from" TUs we have a "to" context
which may have erroneous nodes in it. (Remember, the AST is immutable so
there is no way to delete a node once we had created it and we realized
the error later.) All these erroneous nodes are marked in
ASTImporterSharedState::ImportErrors. Clients of the ASTImporter may
use this as an input. E.g. the static analyzer engine may not try to
analyze a function if that is marked as erroneous (it can be queried via
ASTImporterSharedState::getImportDeclErrorIfAny()).
Reviewers: a_sidorin, a.sidorin, shafik
Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D62376
llvm-svn: 364785
diff --git a/clang/unittests/AST/ASTImporterFixtures.cpp b/clang/unittests/AST/ASTImporterFixtures.cpp
index b2b848f..0be4e78 100644
--- a/clang/unittests/AST/ASTImporterFixtures.cpp
+++ b/clang/unittests/AST/ASTImporterFixtures.cpp
@@ -14,7 +14,7 @@
#include "ASTImporterFixtures.h"
#include "clang/AST/ASTImporter.h"
-#include "clang/AST/ASTImporterLookupTable.h"
+#include "clang/AST/ASTImporterSharedState.h"
#include "clang/Frontend/ASTUnit.h"
#include "clang/Tooling/Tooling.h"
@@ -50,28 +50,31 @@
if (!Creator)
Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
ASTContext &FromContext, FileManager &FromFileManager,
- bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+ bool MinimalImport,
+ const std::shared_ptr<ASTImporterSharedState> &SharedState) {
return new ASTImporter(ToContext, ToFileManager, FromContext,
- FromFileManager, MinimalImport, LookupTable);
+ FromFileManager, MinimalImport, SharedState);
};
}
ASTImporterTestBase::TU::~TU() {}
void ASTImporterTestBase::TU::lazyInitImporter(
- ASTImporterLookupTable &LookupTable, ASTUnit *ToAST) {
+ const std::shared_ptr<ASTImporterSharedState> &SharedState,
+ ASTUnit *ToAST) {
assert(ToAST);
if (!Importer)
Importer.reset(Creator(ToAST->getASTContext(), ToAST->getFileManager(),
Unit->getASTContext(), Unit->getFileManager(), false,
- &LookupTable));
+ SharedState));
assert(&ToAST->getASTContext() == &Importer->getToContext());
createVirtualFileIfNeeded(ToAST, FileName, Code);
}
-Decl *ASTImporterTestBase::TU::import(ASTImporterLookupTable &LookupTable,
- ASTUnit *ToAST, Decl *FromDecl) {
- lazyInitImporter(LookupTable, ToAST);
+Decl *ASTImporterTestBase::TU::import(
+ const std::shared_ptr<ASTImporterSharedState> &SharedState, ASTUnit *ToAST,
+ Decl *FromDecl) {
+ lazyInitImporter(SharedState, ToAST);
if (auto ImportedOrErr = Importer->Import(FromDecl))
return *ImportedOrErr;
else {
@@ -80,9 +83,10 @@
}
}
-QualType ASTImporterTestBase::TU::import(ASTImporterLookupTable &LookupTable,
- ASTUnit *ToAST, QualType FromType) {
- lazyInitImporter(LookupTable, ToAST);
+QualType ASTImporterTestBase::TU::import(
+ const std::shared_ptr<ASTImporterSharedState> &SharedState, ASTUnit *ToAST,
+ QualType FromType) {
+ lazyInitImporter(SharedState, ToAST);
if (auto ImportedOrErr = Importer->Import(FromType))
return *ImportedOrErr;
else {
@@ -91,10 +95,10 @@
}
}
-void ASTImporterTestBase::lazyInitLookupTable(TranslationUnitDecl *ToTU) {
+void ASTImporterTestBase::lazyInitSharedState(TranslationUnitDecl *ToTU) {
assert(ToTU);
- if (!LookupTablePtr)
- LookupTablePtr = llvm::make_unique<ASTImporterLookupTable>(*ToTU);
+ if (!SharedStatePtr)
+ SharedStatePtr = std::make_shared<ASTImporterSharedState>(*ToTU);
}
void ASTImporterTestBase::lazyInitToAST(Language ToLang, StringRef ToSrcCode,
@@ -107,7 +111,7 @@
// Build the AST from an empty file.
ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, FileName);
ToAST->enableSourceFileDiagnostics();
- lazyInitLookupTable(ToAST->getASTContext().getTranslationUnitDecl());
+ lazyInitSharedState(ToAST->getASTContext().getTranslationUnitDecl());
}
ASTImporterTestBase::TU *ASTImporterTestBase::findFromTU(Decl *From) {
@@ -147,7 +151,7 @@
assert(FoundDecls.size() == 1);
Decl *Imported =
- FromTU.import(*LookupTablePtr, ToAST.get(), FoundDecls.front());
+ FromTU.import(SharedStatePtr, ToAST.get(), FoundDecls.front());
assert(Imported);
return std::make_tuple(*FoundDecls.begin(), Imported);
@@ -178,16 +182,17 @@
Decl *ASTImporterTestBase::Import(Decl *From, Language ToLang) {
lazyInitToAST(ToLang, "", OutputFileName);
TU *FromTU = findFromTU(From);
- assert(LookupTablePtr);
- return FromTU->import(*LookupTablePtr, ToAST.get(), From);
+ assert(SharedStatePtr);
+ Decl *To = FromTU->import(SharedStatePtr, ToAST.get(), From);
+ return To;
}
QualType ASTImporterTestBase::ImportType(QualType FromType, Decl *TUDecl,
Language ToLang) {
lazyInitToAST(ToLang, "", OutputFileName);
TU *FromTU = findFromTU(TUDecl);
- assert(LookupTablePtr);
- return FromTU->import(*LookupTablePtr, ToAST.get(), FromType);
+ assert(SharedStatePtr);
+ return FromTU->import(SharedStatePtr, ToAST.get(), FromType);
}
ASTImporterTestBase::~ASTImporterTestBase() {
diff --git a/clang/unittests/AST/ASTImporterFixtures.h b/clang/unittests/AST/ASTImporterFixtures.h
index 4e666e3..4b67a94 100644
--- a/clang/unittests/AST/ASTImporterFixtures.h
+++ b/clang/unittests/AST/ASTImporterFixtures.h
@@ -17,8 +17,8 @@
#include "gmock/gmock.h"
#include "clang/AST/ASTImporter.h"
-#include "clang/AST/ASTImporterLookupTable.h"
#include "clang/Frontend/ASTUnit.h"
+#include "clang/AST/ASTImporterSharedState.h"
#include "DeclMatcher.h"
#include "Language.h"
@@ -26,7 +26,7 @@
namespace clang {
class ASTImporter;
-class ASTImporterLookupTable;
+class ASTImporterSharedState;
class ASTUnit;
namespace ast_matchers {
@@ -77,9 +77,9 @@
public:
/// Allocates an ASTImporter (or one of its subclasses).
- typedef std::function<ASTImporter *(ASTContext &, FileManager &, ASTContext &,
- FileManager &, bool,
- ASTImporterLookupTable *)>
+ typedef std::function<ASTImporter *(
+ ASTContext &, FileManager &, ASTContext &, FileManager &, bool,
+ const std::shared_ptr<ASTImporterSharedState> &SharedState)>
ImporterConstructor;
// The lambda that constructs the ASTImporter we use in this test.
@@ -104,11 +104,13 @@
ImporterConstructor C = ImporterConstructor());
~TU();
- void lazyInitImporter(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST);
- Decl *import(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST,
- Decl *FromDecl);
- QualType import(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST,
- QualType FromType);
+ void
+ lazyInitImporter(const std::shared_ptr<ASTImporterSharedState> &SharedState,
+ ASTUnit *ToAST);
+ Decl *import(const std::shared_ptr<ASTImporterSharedState> &SharedState,
+ ASTUnit *ToAST, Decl *FromDecl);
+ QualType import(const std::shared_ptr<ASTImporterSharedState> &SharedState,
+ ASTUnit *ToAST, QualType FromType);
};
// We may have several From contexts and related translation units. In each
@@ -120,13 +122,13 @@
// vector is expanding, with the list we won't have these issues.
std::list<TU> FromTUs;
- // Initialize the lookup table if not initialized already.
- void lazyInitLookupTable(TranslationUnitDecl *ToTU);
+ // Initialize the shared state if not initialized already.
+ void lazyInitSharedState(TranslationUnitDecl *ToTU);
void lazyInitToAST(Language ToLang, StringRef ToSrcCode, StringRef FileName);
protected:
- std::unique_ptr<ASTImporterLookupTable> LookupTablePtr;
+ std::shared_ptr<ASTImporterSharedState> SharedStatePtr;
public:
// We may have several From context but only one To context.
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index d517d63..165946a 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -316,10 +316,11 @@
RedirectingImporterTest() {
Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
ASTContext &FromContext, FileManager &FromFileManager,
- bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+ bool MinimalImport,
+ const std::shared_ptr<ASTImporterSharedState> &SharedState) {
return new RedirectingImporter(ToContext, ToFileManager, FromContext,
FromFileManager, MinimalImport,
- LookupTable);
+ SharedState);
};
}
};
@@ -2888,7 +2889,7 @@
CXXMethodDecl *Method =
FirstDeclMatcher<CXXMethodDecl>().match(ToClass, MethodMatcher);
ToClass->removeDecl(Method);
- LookupTablePtr->remove(Method);
+ SharedStatePtr->getLookupTable()->remove(Method);
}
ASSERT_EQ(DeclCounter<CXXMethodDecl>().match(ToClass, MethodMatcher), 0u);
@@ -4723,10 +4724,11 @@
ErrorHandlingTest() {
Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
ASTContext &FromContext, FileManager &FromFileManager,
- bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+ bool MinimalImport,
+ const std::shared_ptr<ASTImporterSharedState> &SharedState) {
return new ASTImporterWithFakeErrors(ToContext, ToFileManager,
FromContext, FromFileManager,
- MinimalImport, LookupTable);
+ MinimalImport, SharedState);
};
}
// In this test we purposely report an error (UnsupportedConstruct) when
@@ -4999,6 +5001,79 @@
EXPECT_TRUE(ImportedOK);
}
+// An error should be set for a class if it had a previous import with an error
+// from another TU.
+TEST_P(ErrorHandlingTest,
+ ImportedDeclWithErrorShouldFailTheImportOfDeclWhichMapToIt) {
+ // We already have a fwd decl.
+ TranslationUnitDecl *ToTU = getToTuDecl(
+ "class X;", Lang_CXX);
+ // Then we import a definition.
+ {
+ TranslationUnitDecl *FromTU = getTuDecl(std::string(R"(
+ class X {
+ void f() { )") + ErroneousStmt + R"( }
+ void ok();
+ };
+ )",
+ Lang_CXX);
+ auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("X")));
+ CXXRecordDecl *ImportedX = Import(FromX, Lang_CXX);
+
+ // An error is set for X ...
+ EXPECT_FALSE(ImportedX);
+ ASTImporter *Importer = findFromTU(FromX)->Importer.get();
+ Optional<ImportError> OptErr = Importer->getImportDeclErrorIfAny(FromX);
+ ASSERT_TRUE(OptErr);
+ EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+ }
+ // ... but the node had been created.
+ auto *ToXDef = FirstDeclMatcher<CXXRecordDecl>().match(
+ ToTU, cxxRecordDecl(hasName("X"), isDefinition()));
+ // An error is set for "ToXDef" in the shared state.
+ Optional<ImportError> OptErr =
+ SharedStatePtr->getImportDeclErrorIfAny(ToXDef);
+ ASSERT_TRUE(OptErr);
+ EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+
+ auto *ToXFwd = FirstDeclMatcher<CXXRecordDecl>().match(
+ ToTU, cxxRecordDecl(hasName("X"), unless(isDefinition())));
+ // An error is NOT set for the fwd Decl of X in the shared state.
+ OptErr = SharedStatePtr->getImportDeclErrorIfAny(ToXFwd);
+ ASSERT_FALSE(OptErr);
+
+ // Try to import X again but from another TU.
+ {
+ TranslationUnitDecl *FromTU = getTuDecl(std::string(R"(
+ class X {
+ void f() { )") + ErroneousStmt + R"( }
+ void ok();
+ };
+ )",
+ Lang_CXX, "input1.cc");
+
+ auto *FromX = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("X")));
+ CXXRecordDecl *ImportedX = Import(FromX, Lang_CXX);
+
+ // If we did not save the errors for the "to" context then the below checks
+ // would fail, because the lookup finds the fwd Decl of the existing
+ // definition in the "to" context. We can reach the existing definition via
+ // the found fwd Decl. That existing definition is structurally equivalent
+ // (we check only the fields) with this one we want to import, so we return
+ // with the existing definition, which is erroneous (one method is missing).
+
+ // The import should fail.
+ EXPECT_FALSE(ImportedX);
+ ASTImporter *Importer = findFromTU(FromX)->Importer.get();
+ Optional<ImportError> OptErr = Importer->getImportDeclErrorIfAny(FromX);
+ // And an error is set for this new X in the "from" ctx.
+ ASSERT_TRUE(OptErr);
+ EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct);
+ }
+}
+
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ErrorHandlingTest,
DefaultTestValuesForRunOptions, );