From 2afbfb6b2268b8ba7fa9584784e8b11ef97c064a Mon Sep 17 00:00:00 2001 From: Gabor Marton Date: Mon, 1 Jul 2019 15:37:07 +0000 Subject: [PATCH] [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 --- clang/include/clang/AST/ASTImporter.h | 12 +-- clang/include/clang/AST/ASTImporterSharedState.h | 80 ++++++++++++++++++++ clang/include/clang/CrossTU/CrossTranslationUnit.h | 6 +- clang/lib/AST/ASTImporter.cpp | 58 +++++++++++---- clang/lib/CrossTU/CrossTranslationUnit.cpp | 10 +-- clang/lib/Frontend/ASTMerge.cpp | 6 +- clang/unittests/AST/ASTImporterFixtures.cpp | 45 +++++++----- clang/unittests/AST/ASTImporterFixtures.h | 28 +++---- clang/unittests/AST/ASTImporterTest.cpp | 85 ++++++++++++++++++++-- 9 files changed, 258 insertions(+), 72 deletions(-) create mode 100644 clang/include/clang/AST/ASTImporterSharedState.h diff --git a/clang/include/clang/AST/ASTImporter.h b/clang/include/clang/AST/ASTImporter.h index e0089bb..21b1bbb 100644 --- a/clang/include/clang/AST/ASTImporter.h +++ b/clang/include/clang/AST/ASTImporter.h @@ -32,8 +32,8 @@ namespace clang { class ASTContext; +class ASTImporterSharedState; class Attr; -class ASTImporterLookupTable; class CXXBaseSpecifier; class CXXCtorInitializer; class Decl; @@ -209,13 +209,7 @@ class TypeSourceInfo; }; 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; @@ -311,7 +305,7 @@ class TypeSourceInfo; 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 index 0000000..e384950 --- /dev/null +++ b/clang/include/clang/AST/ASTImporterSharedState.h @@ -0,0 +1,80 @@ +//===- 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() = default; + + ASTImporterSharedState(TranslationUnitDecl &ToTU) { + LookupTable = llvm::make_unique(ToTU); + } + + ASTImporterLookupTable *getLookupTable() { return LookupTable.get(); } + + void addDeclToLookup(Decl *D) { + if (LookupTable) + if (auto *ND = dyn_cast(D)) + LookupTable->add(ND); + } + + void removeDeclFromLookup(Decl *D) { + if (LookupTable) + if (auto *ND = dyn_cast(D)) + LookupTable->remove(ND); + } + + 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 index 9270f7f..d1d38e0 100644 --- 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 @@ public: 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 @@ private: 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 index 7d69ed9..cc4cb16 100644 --- 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" @@ -7701,11 +7701,16 @@ void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod, 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(); } @@ -7744,9 +7749,9 @@ ASTImporter::findDeclsInToCtx(DeclContext *DC, DeclarationName Name) { // 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? @@ -7758,9 +7763,7 @@ ASTImporter::findDeclsInToCtx(DeclContext *DC, DeclarationName Name) { } void ASTImporter::AddToLookupTable(Decl *ToD) { - if (LookupTable) - if (auto *ToND = dyn_cast(ToD)) - LookupTable->add(ToND); + SharedState->addDeclToLookup(ToD); } Expected ASTImporter::ImportImpl(Decl *FromD) { @@ -7855,6 +7858,12 @@ Expected ASTImporter::Import(Decl *FromD) { // 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)) { + setImportDeclError(FromD, *Error); + 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 @@ -7888,27 +7897,39 @@ Expected ASTImporter::Import(Decl *FromD) { // traverse of the 'to' context). auto PosF = ImportedFromDecls.find(ToD); if (PosF != ImportedFromDecls.end()) { - if (LookupTable) - if (auto *ToND = dyn_cast(ToD)) - LookupTable->remove(ToND); + SharedState->removeDeclFromLookup(ToD); 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. } - // After takeError the error is not usable anymore in ToDOrErr. + // Error encountered for the first time. + // 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); + // 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); + //FIXME Should we remove these Decls from ImportedDecls? + // 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); + // FIXME Should we remove these Decls from the LookupTable, + // and from ImportedFromDecls? + } SavedImportPaths[FromD].clear(); // Do not return ToDOrErr, error was taken out of it. @@ -7927,8 +7948,17 @@ Expected ASTImporter::Import(Decl *FromD) { 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)) { + setImportDeclError(FromD, *Error); + 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 index a532189..166bdc0 100644 --- a/clang/lib/CrossTU/CrossTranslationUnit.cpp +++ b/clang/lib/CrossTU/CrossTranslationUnit.cpp @@ -437,10 +437,10 @@ CrossTranslationUnitContext::importDefinition(const VarDecl *VD) { 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 & @@ -448,10 +448,10 @@ CrossTranslationUnitContext::getOrCreateASTImporter(ASTContext &From) { 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 index f270975..14d781c 100644 --- 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 @@ void ASTMergeAction::ExecuteAction() { &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 @@ void ASTMergeAction::ExecuteAction() { 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.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 @@ ASTImporterTestBase::TU::TU(StringRef Code, StringRef FileName, ArgVector Args, 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 @@ Decl *ASTImporterTestBase::TU::import(ASTImporterLookupTable &LookupTable, } } -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 @@ QualType ASTImporterTestBase::TU::import(ASTImporterLookupTable &LookupTable, } } -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 @@ void ASTImporterTestBase::lazyInitToAST(Language ToLang, StringRef ToSrcCode, // 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 @@ ASTImporterTestBase::getImportedDecl(StringRef FromSrcCode, Language FromLang, 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 @@ TranslationUnitDecl *ASTImporterTestBase::getToTuDecl(StringRef ToSrcCode, 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 @@ class ASTImporterTestBase : public CompilerOptionSpecificTest { 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 @@ private: 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,13 +122,13 @@ private: // 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); 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/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 @@ struct RedirectingImporterTest : ASTImporterOptionSpecificTestBase { 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 @@ private: CXXMethodDecl *Method = FirstDeclMatcher().match(ToClass, MethodMatcher); ToClass->removeDecl(Method); - LookupTablePtr->remove(Method); + SharedStatePtr->getLookupTable()->remove(Method); } ASSERT_EQ(DeclCounter().match(ToClass, MethodMatcher), 0u); @@ -4723,10 +4724,11 @@ struct ErrorHandlingTest : ASTImporterOptionSpecificTestBase { ErrorHandlingTest() { Creator = [](ASTContext &ToContext, FileManager &ToFileManager, ASTContext &FromContext, FileManager &FromFileManager, - bool MinimalImport, ASTImporterLookupTable *LookupTable) { + bool MinimalImport, + const std::shared_ptr &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 @@ TEST_P(ErrorHandlingTest, ErrorIsNotPropagatedFromMemberToNamespace) { 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().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 OptErr = Importer->getImportDeclErrorIfAny(FromX); + ASSERT_TRUE(OptErr); + EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct); + } + // ... but the node had been created. + auto *ToXDef = FirstDeclMatcher().match( + ToTU, cxxRecordDecl(hasName("X"), isDefinition())); + // An error is set for "ToXDef" in the shared state. + Optional OptErr = + SharedStatePtr->getImportDeclErrorIfAny(ToXDef); + ASSERT_TRUE(OptErr); + EXPECT_EQ(OptErr->Error, ImportError::UnsupportedConstruct); + + auto *ToXFwd = FirstDeclMatcher().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().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 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, ); -- 2.7.4