[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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@364785 91177308-0d34-0410-b5e6-96231b3b80d8

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<ASTImporterSharedState> 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<ASTImporterSharedState> SharedState = nullptr);
 
     virtual ~ASTImporter();
 

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<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() = default;
+
+  ASTImporterSharedState(TranslationUnitDecl &ToTU) {
+    LookupTable = llvm::make_unique<ASTImporterLookupTable>(ToTU);
+  }
+
+  ASTImporterLookupTable *getLookupTable() { return LookupTable.get(); }
+
+  void addDeclToLookup(Decl *D) {
+    if (LookupTable)
+      if (auto *ND = dyn_cast<NamedDecl>(D))
+        LookupTable->add(ND);
+  }
+
+  void removeDeclFromLookup(Decl *D) {
+    if (LookupTable)
+      if (auto *ND = dyn_cast<NamedDecl>(D))
+        LookupTable->remove(ND);
+  }
+
+  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

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 <typename T>
   llvm::Expected<const T *> getCrossTUDefinitionImpl(const T *D,
@@ -181,7 +181,7 @@ private:
       ASTUnitImporterMap;
   CompilerInstance &CI;
   ASTContext &Context;
-  std::unique_ptr<ASTImporterLookupTable> LookupTable;
+  std::shared_ptr<ASTImporterSharedState> ImporterSharedSt;
 };
 
 } // namespace cross_tu

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<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();
 }
@@ -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<NamedDecl>(ToD))
-      LookupTable->add(ToND);
+  SharedState->addDeclToLookup(ToD);
 }
 
 Expected<Decl *> ASTImporter::ImportImpl(Decl *FromD) {
@@ -7855,6 +7858,12 @@ Expected<Decl *> 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<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
@@ -7888,27 +7897,39 @@ Expected<Decl *> 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<NamedDecl>(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<Decl *> ASTImporter::Import(Decl *FromD) {
     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)) {
+    setImportDeclError(FromD, *Error);
+    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);
 

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<ASTImporterLookupTable>(*ToTU);
+  if (!ImporterSharedSt)
+    ImporterSharedSt = std::make_shared<ASTImporterSharedState>(*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;
 }

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<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 @@ 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()) {

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<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 @@ 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<ASTImporterSharedState> &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<ASTImporterLookupTable>(*ToTU);
+  if (!SharedStatePtr)
+    SharedStatePtr = std::make_shared<ASTImporterSharedState>(*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() {

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<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 @@ 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<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 @@ private:
   // 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.

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<ASTImporterSharedState> &SharedState) {
       return new RedirectingImporter(ToContext, ToFileManager, FromContext,
                                      FromFileManager, MinimalImport,
-                                     LookupTable);
+                                     SharedState);
     };
   }
 };
@@ -2888,7 +2889,7 @@ private:
       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 @@ struct ErrorHandlingTest : ASTImporterOptionSpecificTestBase {
   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 @@ 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<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, );