martong created this revision.
martong added a reviewer: a_sidorin.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.
This is the third step of the full error handling. 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()).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D62376
Files:
clang/include/clang/AST/ASTImporter.h
clang/include/clang/AST/ASTImporterSharedState.h
clang/include/clang/CrossTU/CrossTranslationUnit.h
clang/lib/AST/ASTImporter.cpp
clang/lib/CrossTU/CrossTranslationUnit.cpp
clang/lib/Frontend/ASTMerge.cpp
clang/unittests/AST/ASTImporterFixtures.cpp
clang/unittests/AST/ASTImporterFixtures.h
clang/unittests/AST/ASTImporterTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ 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);
Index: clang/unittests/AST/ASTImporterFixtures.h
===================================================================
--- clang/unittests/AST/ASTImporterFixtures.h
+++ 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,14 +122,14 @@
// 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);
TU *findFromTU(Decl *From);
protected:
- std::unique_ptr<ASTImporterLookupTable> LookupTablePtr;
+ std::shared_ptr<ASTImporterSharedState> SharedStatePtr;
public:
// We may have several From context but only one To context.
Index: clang/unittests/AST/ASTImporterFixtures.cpp
===================================================================
--- clang/unittests/AST/ASTImporterFixtures.cpp
+++ 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() {
Index: clang/lib/Frontend/ASTMerge.cpp
===================================================================
--- clang/lib/Frontend/ASTMerge.cpp
+++ clang/lib/Frontend/ASTMerge.cpp
@@ -9,7 +9,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/ASTImporter.h"
-#include "clang/AST/ASTImporterLookupTable.h"
+#include "clang/AST/ASTImporterSharedState.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
@@ -38,7 +38,7 @@
&CI.getASTContext());
IntrusiveRefCntPtr<DiagnosticIDs>
DiagIDs(CI.getDiagnostics().getDiagnosticIDs());
- ASTImporterLookupTable LookupTable(
+ auto SharedState = std::make_shared<ASTImporterSharedState>(
*CI.getASTContext().getTranslationUnitDecl());
for (unsigned I = 0, N = ASTFiles.size(); I != N; ++I) {
IntrusiveRefCntPtr<DiagnosticsEngine>
@@ -55,7 +55,7 @@
ASTImporter Importer(CI.getASTContext(), CI.getFileManager(),
Unit->getASTContext(), Unit->getFileManager(),
- /*MinimalImport=*/false, &LookupTable);
+ /*MinimalImport=*/false, SharedState);
TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
for (auto *D : TU->decls()) {
Index: clang/lib/CrossTU/CrossTranslationUnit.cpp
===================================================================
--- clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -433,10 +433,10 @@
return importDefinitionImpl(VD);
}
-void CrossTranslationUnitContext::lazyInitLookupTable(
+void CrossTranslationUnitContext::lazyInitImporterSharedSt(
TranslationUnitDecl *ToTU) {
- if (!LookupTable)
- LookupTable = llvm::make_unique<ASTImporterLookupTable>(*ToTU);
+ if (!ImporterSharedSt)
+ ImporterSharedSt = std::make_shared<ASTImporterSharedState>(*ToTU);
}
ASTImporter &
@@ -444,10 +444,10 @@
auto I = ASTUnitImporterMap.find(From.getTranslationUnitDecl());
if (I != ASTUnitImporterMap.end())
return *I->second;
- lazyInitLookupTable(Context.getTranslationUnitDecl());
+ lazyInitImporterSharedSt(Context.getTranslationUnitDecl());
ASTImporter *NewImporter = new ASTImporter(
Context, Context.getSourceManager().getFileManager(), From,
- From.getSourceManager().getFileManager(), false, LookupTable.get());
+ From.getSourceManager().getFileManager(), false, ImporterSharedSt);
ASTUnitImporterMap[From.getTranslationUnitDecl()].reset(NewImporter);
return *NewImporter;
}
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -12,7 +12,7 @@
//===----------------------------------------------------------------------===//
#include "clang/AST/ASTImporter.h"
-#include "clang/AST/ASTImporterLookupTable.h"
+#include "clang/AST/ASTImporterSharedState.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/ASTStructuralEquivalence.h"
@@ -7659,11 +7659,16 @@
ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
ASTContext &FromContext, FileManager &FromFileManager,
bool MinimalImport,
- ASTImporterLookupTable *LookupTable)
- : LookupTable(LookupTable), ToContext(ToContext), FromContext(FromContext),
+ std::shared_ptr<ASTImporterSharedState> SharedState)
+ : SharedState(SharedState), ToContext(ToContext), FromContext(FromContext),
ToFileManager(ToFileManager), FromFileManager(FromFileManager),
Minimal(MinimalImport) {
+ // Create a default state without the lookup table: LLDB case.
+ if (!SharedState) {
+ this->SharedState = std::make_shared<ASTImporterSharedState>();
+ }
+
ImportedDecls[FromContext.getTranslationUnitDecl()] =
ToContext.getTranslationUnitDecl();
}
@@ -7702,9 +7707,9 @@
// then the enum constant 'A' and the variable 'A' violates ODR.
// We can diagnose this only if we search in the redecl context.
DeclContext *ReDC = DC->getRedeclContext();
- if (LookupTable) {
+ if (SharedState->getLookupTable()) {
ASTImporterLookupTable::LookupResult LookupResult =
- LookupTable->lookup(ReDC, Name);
+ SharedState->getLookupTable()->lookup(ReDC, Name);
return FoundDeclsTy(LookupResult.begin(), LookupResult.end());
} else {
// FIXME Can we remove this kind of lookup?
@@ -7716,9 +7721,9 @@
}
void ASTImporter::AddToLookupTable(Decl *ToD) {
- if (LookupTable)
+ if (SharedState->getLookupTable())
if (auto *ToND = dyn_cast<NamedDecl>(ToD))
- LookupTable->add(ToND);
+ SharedState->getLookupTable()->add(ToND);
}
Expected<Decl *> ASTImporter::ImportImpl(Decl *FromD) {
@@ -7822,6 +7827,10 @@
// Check whether we've already imported this declaration.
Decl *ToD = GetAlreadyImportedOrNull(FromD);
if (ToD) {
+ // Already imported (possibly from another TU) and with an error.
+ if (auto Error = SharedState->getImportDeclErrorIfAny(ToD))
+ return make_error<ImportError>(*Error);
+
// If FromD has some updated flags after last import, apply it
updateFlags(FromD, ToD);
// If we encounter a cycle during an import then we save the relevant part
@@ -7855,13 +7864,15 @@
// traverse of the 'to' context).
auto PosF = ImportedFromDecls.find(ToD);
if (PosF != ImportedFromDecls.end()) {
- if (LookupTable)
+ if (SharedState->getLookupTable())
if (auto *ToND = dyn_cast<NamedDecl>(ToD))
- LookupTable->remove(ToND);
+ SharedState->getLookupTable()->remove(ToND);
ImportedFromDecls.erase(PosF);
}
// FIXME: AST may contain remaining references to the failed object.
+ // However, the ImportDeclErrors in the shared state contains all the
+ // failed objects together with their error.
}
if (!getImportDeclErrorIfAny(FromD)) {
@@ -7872,12 +7883,20 @@
handleAllErrors(ToDOrErr.takeError(),
[&ErrOut](const ImportError &E) { ErrOut = E; });
setImportDeclError(FromD, ErrOut);
+ // Set the error for the mapped to Decl, which is in the "to" context.
+ if (Pos != ImportedDecls.end())
+ SharedState->setImportDeclError(Pos->second, ErrOut);
// Set the error for all nodes which have been created before we
// recognized the error.
for (const auto &Path : SavedImportPaths[FromD])
- for (Decl *Di : Path)
- setImportDeclError(Di, ErrOut);
+ for (Decl *FromDi : Path) {
+ setImportDeclError(FromDi, ErrOut);
+ // Set the error for the mapped to Decl, which is in the "to" context.
+ auto Ii = ImportedDecls.find(FromDi);
+ if (Ii != ImportedDecls.end())
+ SharedState->setImportDeclError(Ii->second, ErrOut);
+ }
SavedImportPaths[FromD].clear();
// Do not return ToDOrErr, error was taken out of it.
@@ -7898,8 +7917,15 @@
return make_error<ImportError>(*Err);
}
+ // We could import from the current TU without error. But previously we
+ // already had imported a Decl as `ToD` from another TU (with another
+ // ASTImporter object) and with an error.
+ if (auto Error = SharedState->getImportDeclErrorIfAny(ToD))
+ return make_error<ImportError>(*Error);
+
// Make sure that ImportImpl registered the imported decl.
assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
+
// Notify subclasses.
Imported(FromD, ToD);
Index: clang/include/clang/CrossTU/CrossTranslationUnit.h
===================================================================
--- clang/include/clang/CrossTU/CrossTranslationUnit.h
+++ clang/include/clang/CrossTU/CrossTranslationUnit.h
@@ -14,7 +14,7 @@
#ifndef LLVM_CLANG_CROSSTU_CROSSTRANSLATIONUNIT_H
#define LLVM_CLANG_CROSSTU_CROSSTRANSLATIONUNIT_H
-#include "clang/AST/ASTImporterLookupTable.h"
+#include "clang/AST/ASTImporterSharedState.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallPtrSet.h"
@@ -161,7 +161,7 @@
void emitCrossTUDiagnostics(const IndexError &IE);
private:
- void lazyInitLookupTable(TranslationUnitDecl *ToTU);
+ void lazyInitImporterSharedSt(TranslationUnitDecl *ToTU);
ASTImporter &getOrCreateASTImporter(ASTContext &From);
template <typename T>
llvm::Expected<const T *> getCrossTUDefinitionImpl(const T *D,
@@ -181,7 +181,7 @@
ASTUnitImporterMap;
CompilerInstance &CI;
ASTContext &Context;
- std::unique_ptr<ASTImporterLookupTable> LookupTable;
+ std::shared_ptr<ASTImporterSharedState> ImporterSharedSt;
};
} // namespace cross_tu
Index: clang/include/clang/AST/ASTImporterSharedState.h
===================================================================
--- /dev/null
+++ clang/include/clang/AST/ASTImporterSharedState.h
@@ -0,0 +1,68 @@
+//===- ASTImporterSharedState.h - ASTImporter specific state --*- C++ -*---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines the ASTImporter specific state, which may be shared
+// amongst several ASTImporter objects.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H
+#define LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H
+
+#include "clang/AST/ASTImporterLookupTable.h"
+#include "llvm/ADT/DenseMap.h"
+// FIXME We need this because of ImportError.
+#include "clang/AST/ASTImporter.h"
+
+namespace clang {
+
+class TranslationUnitDecl;
+
+/// Importer specific state, which may be shared amongst several ASTImporter
+/// objects.
+class ASTImporterSharedState {
+
+ /// Pointer to the import specific lookup table.
+ std::unique_ptr<ASTImporterLookupTable> LookupTable;
+
+ /// Mapping from the already-imported declarations in the "to"
+ /// 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
+ /// ImportedFromDecls. This map is updated continuously during imports and
+ /// never cleared (like ImportedFromDecls).
+ llvm::DenseMap<Decl *, ImportError> ImportErrors;
+
+ // FIXME put ImportedFromDecls here!
+ // And from that point we can better encapsulate the lookup table.
+
+public:
+ ASTImporterSharedState() {}
+
+ ASTImporterSharedState(TranslationUnitDecl &ToTU) {
+ LookupTable = llvm::make_unique<ASTImporterLookupTable>(ToTU);
+ }
+
+ ASTImporterLookupTable *getLookupTable() { return LookupTable.get(); }
+
+ llvm::Optional<ImportError> getImportDeclErrorIfAny(Decl *ToD) const {
+ auto Pos = ImportErrors.find(ToD);
+ if (Pos != ImportErrors.end())
+ return Pos->second;
+ else
+ return Optional<ImportError>();
+ }
+
+ void setImportDeclError(Decl *To, ImportError Error) {
+ ImportErrors[To] = Error;
+ }
+};
+
+} // namespace clang
+#endif // LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H
Index: clang/include/clang/AST/ASTImporter.h
===================================================================
--- clang/include/clang/AST/ASTImporter.h
+++ clang/include/clang/AST/ASTImporter.h
@@ -33,8 +33,8 @@
namespace clang {
class ASTContext;
+class ASTImporterSharedState;
class Attr;
-class ASTImporterLookupTable;
class CXXBaseSpecifier;
class CXXCtorInitializer;
class Decl;
@@ -113,13 +113,7 @@
};
private:
-
- /// Pointer to the import specific lookup table, which may be shared
- /// amongst several ASTImporter objects.
- /// This is an externally managed resource (and should exist during the
- /// lifetime of the ASTImporter object)
- /// If not set then the original C/C++ lookup is used.
- ASTImporterLookupTable *LookupTable = nullptr;
+ std::shared_ptr<ASTImporterSharedState> SharedState = nullptr;
/// The path which we go through during the import of a given AST node.
ImportPathTy ImportPath;
@@ -212,7 +206,7 @@
ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
ASTContext &FromContext, FileManager &FromFileManager,
bool MinimalImport,
- ASTImporterLookupTable *LookupTable = nullptr);
+ std::shared_ptr<ASTImporterSharedState> SharedState = nullptr);
virtual ~ASTImporter();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
