diff --git a/clang/include/clang/AST/ASTImporter.h b/clang/include/clang/AST/ASTImporter.h --- a/clang/include/clang/AST/ASTImporter.h +++ b/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 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 SharedState = nullptr); virtual ~ASTImporter(); diff --git a/clang/include/clang/AST/ASTImporterSharedState.h b/clang/include/clang/AST/ASTImporterSharedState.h new file mode 100644 --- /dev/null +++ b/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 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 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(ToTU); + } + + ASTImporterLookupTable *getLookupTable() { return LookupTable.get(); } + + llvm::Optional getImportDeclErrorIfAny(Decl *ToD) const { + auto Pos = ImportErrors.find(ToD); + if (Pos != ImportErrors.end()) + return Pos->second; + else + return Optional(); + } + + void setImportDeclError(Decl *To, ImportError Error) { + ImportErrors[To] = Error; + } +}; + +} // namespace clang +#endif // LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H diff --git a/clang/include/clang/CrossTU/CrossTranslationUnit.h b/clang/include/clang/CrossTU/CrossTranslationUnit.h --- a/clang/include/clang/CrossTU/CrossTranslationUnit.h +++ b/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 llvm::Expected getCrossTUDefinitionImpl(const T *D, @@ -181,7 +181,7 @@ ASTUnitImporterMap; CompilerInstance &CI; ASTContext &Context; - std::unique_ptr LookupTable; + std::shared_ptr ImporterSharedSt; }; } // namespace cross_tu diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp --- a/clang/lib/AST/ASTImporter.cpp +++ b/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 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(); + } + 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(ToD)) - LookupTable->add(ToND); + SharedState->getLookupTable()->add(ToND); } Expected 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(*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(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(*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(*Error); + // Make sure that ImportImpl registered the imported decl. assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?"); + // Notify subclasses. Imported(FromD, ToD); diff --git a/clang/lib/CrossTU/CrossTranslationUnit.cpp b/clang/lib/CrossTU/CrossTranslationUnit.cpp --- a/clang/lib/CrossTU/CrossTranslationUnit.cpp +++ b/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(*ToTU); + if (!ImporterSharedSt) + ImporterSharedSt = std::make_shared(*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; } diff --git a/clang/lib/Frontend/ASTMerge.cpp b/clang/lib/Frontend/ASTMerge.cpp --- a/clang/lib/Frontend/ASTMerge.cpp +++ b/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 DiagIDs(CI.getDiagnostics().getDiagnosticIDs()); - ASTImporterLookupTable LookupTable( + auto SharedState = std::make_shared( *CI.getASTContext().getTranslationUnitDecl()); for (unsigned I = 0, N = ASTFiles.size(); I != N; ++I) { IntrusiveRefCntPtr @@ -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()) { diff --git a/clang/unittests/AST/ASTImporterFixtures.h b/clang/unittests/AST/ASTImporterFixtures.h --- 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 + typedef std::function &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 &SharedState, + ASTUnit *ToAST); + Decl *import(const std::shared_ptr &SharedState, + ASTUnit *ToAST, Decl *FromDecl); + QualType import(const std::shared_ptr &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 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 LookupTablePtr; + std::shared_ptr SharedStatePtr; public: // We may have several From context but only one To context. diff --git a/clang/unittests/AST/ASTImporterFixtures.cpp b/clang/unittests/AST/ASTImporterFixtures.cpp --- 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 &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 &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 &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 &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(*ToTU); + if (!SharedStatePtr) + SharedStatePtr = std::make_shared(*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/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp --- 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 &SharedState) { return new RedirectingImporter(ToContext, ToFileManager, FromContext, FromFileManager, MinimalImport, - LookupTable); + SharedState); }; } }; @@ -2888,7 +2889,7 @@ CXXMethodDecl *Method = FirstDeclMatcher().match(ToClass, MethodMatcher); ToClass->removeDecl(Method); - LookupTablePtr->remove(Method); + SharedStatePtr->getLookupTable()->remove(Method); } ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 0u);