[ASTImporter] Check visibility/linkage of functions and variables

[martong]

Summary

During import of a global variable with external visibility the lookup
will find variables (with the same name) but with static visibility.
Clearly, we cannot put them into the same redecl chain. The same is
true in case of functions. In this fix we filter the lookup results and
consider only those which have the same visibility as the decl we
currently import.

We consider two decls in two anonymous namsepaces to have the same
visibility only if they are imported from the very same translation
unit.

[martong]

• Remove old style import in case of FoundArray

[balazske]

unittests/AST/ASTImporterTest.cpp

2523

Is this dump needed? (The test should not write unnecessary text output. But debug statements can be leaved in the test, possibly in comment.)

[shafik]

lib/AST/ASTImporter.cpp

2956

We don't really need an else here and it would be more like an early exit which is what llvm style guide suggests.

In the same vein I would do:

if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
   return false;

so a second early exit and remove a level of nesting at the same time.

unittests/AST/ASTImporterTest.cpp

65–68

Are these changes directly related to the visibility change? There is a lot of noise that is not obviously related to the description in the PR.

If not maybe it should be a separate PR?

[martong]

• Remove dumpDeclContext() call
• Remove superfluous else

[martong]

lib/AST/ASTImporter.cpp

2956

It's a good catch, thanks.

unittests/AST/ASTImporterTest.cpp

65–68

Actually, it will be more precise to create first a patch which contains the tests related refactor, I agree. I'll do that and this patch will be dependent upon that.

2523

Thanks for catching this, good point.

[martong]

I have created a separate patch for the test related refactor, this patch now
depends on that patch and contains merely the visibility related change and
their tests.

[martong]

• Move hunks into this patch from parent patch

[martong]

Ping

[shafik]

I ran check-lldb locally and it looks good.

[martong]

Rebase to master(trunk)

[martong]

@shafik Thanks for the review!

include/clang/AST/ASTImporter.h

 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Error.h"
 #include <utility>
 
 namespace clang {
 
 class ASTContext;
+class Attr;
 class ASTImporterLookupTable;
 class CXXBaseSpecifier;
 class CXXCtorInitializer;
 class Decl;
 class DeclContext;
 class Expr;
 class FileManager;
 class NamedDecl;
 class Stmt;
 class TagDecl;
+class TranslationUnitDecl;
 class TypeSourceInfo;
-class Attr;
 
   class ImportError : public llvm::ErrorInfo<ImportError> {
   public:
     /// \brief Kind of error when importing an AST component.
     enum ErrorKind {
         NameConflict, /// Naming ambiguity (likely ODR violation).
         UnsupportedConstruct, /// Not supported node or case.
         Unknown /// Other error.
private:
     /// Mapping from the already-imported types in the "from" context
     /// to the corresponding types in the "to" context.
     llvm::DenseMap<const Type *, const Type *> ImportedTypes;
 
     /// Mapping from the already-imported declarations in the "from"
     /// context to the corresponding declarations in the "to" context.
     llvm::DenseMap<Decl *, Decl *> ImportedDecls;
 
+    /// Mapping from the already-imported declarations in the "to"
+    /// context to the corresponding declarations in the "from" context.
+    llvm::DenseMap<Decl *, Decl *> ImportedFromDecls;
+
     /// Mapping from the already-imported statements in the "from"
     /// context to the corresponding statements in the "to" context.
     llvm::DenseMap<Stmt *, Stmt *> ImportedStmts;
 
     /// Mapping from the already-imported FileIDs in the "from" source
     /// manager to the corresponding FileIDs in the "to" source manager.
     llvm::DenseMap<FileID, FileID> ImportedFileIDs;
 
public:
     // FIXME: Remove this version.
     Decl *Import(Decl *FromD);
     Decl *Import(const Decl *FromD) {
       return Import(const_cast<Decl *>(FromD));
     }
 
     /// Return the copy of the given declaration in the "to" context if
     /// it has already been imported from the "from" context.  Otherwise return
-    /// NULL.
+    /// nullptr.
     Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
 
+    /// Return the translation unit from where the declaration was
+    /// imported. If it does not exist nullptr is returned.
+    TranslationUnitDecl *GetFromTU(Decl *ToD);
+
     /// Import the given declaration context from the "from"
     /// AST context into the "to" AST context.
     ///
     /// \returns the equivalent declaration context in the "to"
     /// context, or error value.
     llvm::Expected<DeclContext *> ImportContext(DeclContext *FromDC);
 
     /// Import the given expression from the "from" context into the

lib/AST/ASTImporter.cpp

public:
     Expected<FunctionTemplateAndArgsTy>
     ImportFunctionTemplateWithTemplateArgsFromSpecialization(
         FunctionDecl *FromFD);
 
     Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
     Error ImportFunctionDeclBody(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
+    template <typename T>
+    bool hasSameVisibilityContext(T *Found, T *From);
+
     bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
     bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
                            bool Complain = true);
     bool IsStructuralMatch(VarDecl *FromVar, VarDecl *ToVar,
                            bool Complain = true);
     bool IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToRecord);
     bool IsStructuralMatch(EnumConstantDecl *FromEC, EnumConstantDecl *ToEC);
     bool IsStructuralMatch(FunctionTemplateDecl *From,
ASTNodeImporter::FindFunctionTemplateSpecialization(FunctionDecl *FromFD) {
   auto *FoundSpec = Template->findSpecialization(ToTemplArgs, InsertPos);
   return FoundSpec;
 }
 
 Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD,
                                               FunctionDecl *ToFD) {
   if (Stmt *FromBody = FromFD->getBody()) {
     if (ExpectedStmt ToBodyOrErr = import(FromBody))
       ToFD->setBody(*ToBodyOrErr);
     else
       return ToBodyOrErr.takeError();
   }
   return Error::success();
 }
 
+template <typename T>
+bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
+  if (From->hasExternalFormalLinkage())
+    return Found->hasExternalFormalLinkage();
+  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
+    return false;
+  if (From->isInAnonymousNamespace())
+    return Found->isInAnonymousNamespace();
+  else
+    return !Found->isInAnonymousNamespace() &&
+           !Found->hasExternalFormalLinkage();
+}
+
 ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   SmallVector<Decl *, 2> Redecls = getCanonicalForwardRedeclChain(D);
   auto RedeclIt = Redecls.begin();
   // Import the first part of the decl chain. I.e. import all previous
   // declarations starting from the canonical decl.
   for (; RedeclIt != Redecls.end() && *RedeclIt != D; ++RedeclIt) {
     ExpectedDecl ToRedeclOrErr = import(*RedeclIt);
else if (!LexicalDC->isFunctionOrMethod()) {
     SmallVector<NamedDecl *, 4> ConflictingDecls;
     unsigned IDNS = Decl::IDNS_Ordinary | Decl::IDNS_OrdinaryFriend;
     auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
     for (auto *FoundDecl : FoundDecls) {
       if (!FoundDecl->isInIdentifierNamespace(IDNS))
         continue;
 
       if (auto *FoundFunction = dyn_cast<FunctionDecl>(FoundDecl)) {
-        if (FoundFunction->hasExternalFormalLinkage() &&
-            D->hasExternalFormalLinkage()) {
-          if (IsStructuralMatch(D, FoundFunction)) {
-            const FunctionDecl *Definition = nullptr;
-            if (D->doesThisDeclarationHaveABody() &&
-                FoundFunction->hasBody(Definition)) {
-              return Importer.MapImported(
-                  D, const_cast<FunctionDecl *>(Definition));
-            }
-            FoundByLookup = FoundFunction;
-            break;
-          }
-
-          // FIXME: Check for overloading more carefully, e.g., by boosting
-          // Sema::IsOverload out to the AST library.
+        if (!hasSameVisibilityContext(FoundFunction, D))
+          continue;
+
+              FoundFunction->hasBody(Definition))
+            return Importer.MapImported(D,
+                                        const_cast<FunctionDecl *>(Definition));
 
-          // Function overloading is okay in C++.
-          if (Importer.getToContext().getLangOpts().CPlusPlus)
-            continue;
 
-          // Complain about inconsistent function types.
-          Importer.ToDiag(Loc, diag::err_odr_function_type_inconsistent)
             << Name << D->getType() << FoundFunction->getType();
-          Importer.ToDiag(FoundFunction->getLocation(),
-                          diag::note_odr_value_here)
+        Importer.ToDiag(FoundFunction->getLocation(), diag::note_odr_value_here)
             << FoundFunction->getType();
-        }
-      }
 
       ConflictingDecls.push_back(FoundDecl);
     }
 
     if (!ConflictingDecls.empty()) {
       Name = Importer.HandleNameConflict(Name, DC, IDNS,
                                          ConflictingDecls.data(),
                                          ConflictingDecls.size());
if (D->isFileVarDecl()) {
     SmallVector<NamedDecl *, 4> ConflictingDecls;
     unsigned IDNS = Decl::IDNS_Ordinary;
     auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
     for (auto *FoundDecl : FoundDecls) {
       if (!FoundDecl->isInIdentifierNamespace(IDNS))
         continue;
 
       if (auto *FoundVar = dyn_cast<VarDecl>(FoundDecl)) {
-        // We have found a variable that we may need to merge with. Check it.
-        if (FoundVar->hasExternalFormalLinkage() &&
-            D->hasExternalFormalLinkage()) {
-          if (Importer.IsStructurallyEquivalent(D->getType(),
-                                                FoundVar->getType())) {
+        if (!hasSameVisibilityContext(FoundVar, D))
+          continue;
 
-            // The VarDecl in the "From" context has a definition, but in the
-            // "To" context we already have a definition.
-            VarDecl *FoundDef = FoundVar->getDefinition();
-            if (D->isThisDeclarationADefinition() && FoundDef)
-              // FIXME Check for ODR error if the two definitions have
-              // different initializers?
-              return Importer.MapImported(D, FoundDef);
 
-            // The VarDecl in the "From" context has an initializer, but in the
-            // "To" context we already have an initializer.
-            const VarDecl *FoundDInit = nullptr;
-            if (D->getInit() && FoundVar->getAnyInitializer(FoundDInit))
-              // FIXME Diagnose ODR error if the two initializers are different?
-              return Importer.MapImported(D, const_cast<VarDecl*>(FoundDInit));
 
-            FoundByLookup = FoundVar;
-            break;
-          }
 
-          const ArrayType *FoundArray
-            = Importer.getToContext().getAsArrayType(FoundVar->getType());
-          const ArrayType *TArray
-            = Importer.getToContext().getAsArrayType(D->getType());
-          if (FoundArray && TArray) {
-            if (isa<IncompleteArrayType>(FoundArray) &&
-                isa<ConstantArrayType>(TArray)) {
-              // Import the type.
-              if (auto TyOrErr = import(D->getType()))
-                FoundVar->setType(*TyOrErr);
-              else
-                return TyOrErr.takeError();
 
-              FoundByLookup = FoundVar;
-              break;
-            } else if (isa<IncompleteArrayType>(TArray) &&
-                       isa<ConstantArrayType>(FoundArray)) {
-              FoundByLookup = FoundVar;
-              break;
-            }
-          }
 
-          Importer.ToDiag(Loc, diag::err_odr_variable_type_inconsistent)
-            << Name << D->getType() << FoundVar->getType();
-          Importer.ToDiag(FoundVar->getLocation(), diag::note_odr_value_here)
-            << FoundVar->getType();
-        }
-      }
 
       ConflictingDecls.push_back(FoundDecl);
     }
 
     if (!ConflictingDecls.empty()) {
       Name = Importer.HandleNameConflict(Name, DC, IDNS,
                                          ConflictingDecls.data(),
                                          ConflictingDecls.size());

 Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
   auto Pos = ImportedDecls.find(FromD);
   if (Pos != ImportedDecls.end())
     return Pos->second;
   else
     return nullptr;
 }
 
+TranslationUnitDecl *ASTImporter::GetFromTU(Decl *ToD) {
+  auto FromDPos = ImportedFromDecls.find(ToD);
+  if (FromDPos == ImportedFromDecls.end())
+    return nullptr;
+  return FromDPos->second->getTranslationUnitDecl();
+}
+
 Expected<Decl *> ASTImporter::Import_New(Decl *FromD) {
   Decl *ToD = Import(FromD);
   if (!ToD && FromD)
     return llvm::make_error<ImportError>();
   return ToD;
 }
 Decl *ASTImporter::Import(Decl *FromD) {
   if (!FromD)

 
 Decl *ASTImporter::MapImported(Decl *From, Decl *To) {
   llvm::DenseMap<Decl *, Decl *>::iterator Pos = ImportedDecls.find(From);
   assert((Pos == ImportedDecls.end() || Pos->second == To) &&
       "Try to import an already imported Decl");
   if (Pos != ImportedDecls.end())
     return Pos->second;
   ImportedDecls[From] = To;
+  // This mapping should be maintained only in this function. Therefore do not
+  // check for additional consistency.
+  ImportedFromDecls[To] = From;
   return To;
 }
 
 bool ASTImporter::IsStructurallyEquivalent(QualType From, QualType To,
                                            bool Complain) {
   llvm::DenseMap<const Type *, const Type *>::iterator Pos
    = ImportedTypes.find(From.getTypePtr());
   if (Pos != ImportedTypes.end() && ToContext.hasSameType(Import(From), To))
     return true;
 
   StructuralEquivalenceContext Ctx(FromContext, ToContext, NonEquivalentDecls,
                                    getStructuralEquivalenceKind(*this), false,
                                    Complain);
   return Ctx.IsEquivalent(From, To);
 }

unittests/AST/ASTImporterTest.cpp

 //===- unittest/AST/ASTImporterTest.cpp - AST node import test ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Tests for the correct import of AST nodes from one AST context to another.
 //
 //===----------------------------------------------------------------------===//
 
+// Define this to have ::testing::Combine available.
+// FIXME: Better solution for this?
+#define GTEST_HAS_COMBINE 1
+
 #include "clang/AST/ASTImporter.h"
 #include "MatchVerifier.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclContextInternals.h"
 #include "clang/AST/ASTImporter.h"
 #include "clang/AST/ASTImporterLookupTable.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"

 }
 
 const StringRef DeclToImportID = "declToImport";
 const StringRef DeclToVerifyID = "declToVerify";
 
 // Common base for the different families of ASTImporter tests that are
 // parameterized on the compiler options which may result a different AST. E.g.
 // -fms-compatibility or -fdelayed-template-parsing.
 class CompilerOptionSpecificTest : public ::testing::Test {
 protected:
   // Return the extra arguments appended to runtime options at compilation.
   virtual ArgVector getExtraArgs() const { return ArgVector(); }
 
   // Returns the argument vector used for a specific language option, this set
   // can be tweaked by the test parameters.
   ArgVector getArgVectorForLanguage(Language Lang) const {
     ArgVector Args = getBasicRunOptionsForLanguage(Lang);
     ArgVector ExtraArgs = getExtraArgs();
     for (const auto &Arg : ExtraArgs) {
       Args.push_back(Arg);
public:
   // The different instances of the param From may have different ASTContext.
   Decl *Import(Decl *From, Language ToLang) {
     lazyInitToAST(ToLang, "", OutputFileName);
     TU *FromTU = findFromTU(From);
     assert(LookupTablePtr);
     return FromTU->import(*LookupTablePtr, ToAST.get(), From);
   }
 
+  template <class DeclT> DeclT *Import(DeclT *From, Language Lang) {
+    return cast_or_null<DeclT>(Import(cast<Decl>(From), Lang));
+  }
+
   QualType ImportType(QualType FromType, Decl *TUDecl, Language ToLang) {
     lazyInitToAST(ToLang, "", OutputFileName);
     TU *FromTU = findFromTU(TUDecl);
     assert(LookupTablePtr);
     return FromTU->import(*LookupTablePtr, ToAST.get(), FromType);
    }
 
   ~ASTImporterTestBase() {
TEST_P(ImportFunctions,
   EXPECT_EQ(ToDFOutOfClass->getDeclContext(), ToD);
   EXPECT_EQ(ToDFOutOfClass->getLexicalDeclContext(), ToTU);
 
   // Check that the redecl chain is intact.
   EXPECT_EQ(ToBFOutOfClass->getPreviousDecl(), ToBFInClass);
   EXPECT_EQ(ToDFOutOfClass->getPreviousDecl(), ToDFInClass);
 }
 
+//FIXME Move these tests to a separate test file.
+namespace TypeAndValueParameterizedTests {
+
+// Type parameters for type-parameterized test fixtures.
+struct GetFunPattern {
+  using DeclTy = FunctionDecl;
+  BindableMatcher<Decl> operator()() { return functionDecl(hasName("f")); }
+};
+struct GetVarPattern {
+  using DeclTy = VarDecl;
+  BindableMatcher<Decl> operator()() { return varDecl(hasName("v")); }
+};
+
+// Values for the value-parameterized test fixtures.
+// FunctionDecl:
+auto *ExternF = "void f();";
+auto *StaticF = "static void f();";
+auto *AnonF = "namespace { void f(); }";
+// VarDecl:
+auto *ExternV = "extern int v;";
+auto *StaticV = "static int v;";
+auto *AnonV = "namespace { extern int v; }";
+
+// First value in tuple: Compile options.
+// Second value in tuple: Source code to be used in the test.
+using ImportVisibilityChainParams =
+    ::testing::WithParamInterface<std::tuple<ArgVector, const char *>>;
+// Fixture to test the redecl chain of Decls with the same visibility.  Gtest
+// makes it possible to have either value-parameterized or type-parameterized
+// fixtures.  However, we cannot have both value- and type-parameterized test
+// fixtures.  This is a value-parameterized test fixture in the gtest sense. We
+// intend to mimic gtest's type-parameters via the PatternFactory template
+// parameter.  We manually instantiate the different tests with the each types.
+template <typename PatternFactory>
+class ImportVisibilityChain
+    : public ASTImporterTestBase, public ImportVisibilityChainParams {
+protected:
+  using DeclTy = typename PatternFactory::DeclTy;
+  ArgVector getExtraArgs() const override { return std::get<0>(GetParam()); }
+  std::string getCode() const { return std::get<1>(GetParam()); }
+  BindableMatcher<Decl> getPattern() const { return PatternFactory()(); }
+
+  // Type-parameterized test.
+  void TypedTest_ImportChain() {
+    std::string Code = getCode() + getCode();
+    auto Pattern = getPattern();
+
+    TranslationUnitDecl *FromTu = getTuDecl(Code, Lang_CXX, "input0.cc");
+
+    auto *FromF0 = FirstDeclMatcher<DeclTy>().match(FromTu, Pattern);
+    auto *FromF1 = LastDeclMatcher<DeclTy>().match(FromTu, Pattern);
+
+    auto *ToF0 = Import(FromF0, Lang_CXX);
+    auto *ToF1 = Import(FromF1, Lang_CXX);
+
+    EXPECT_TRUE(ToF0);
+    ASSERT_TRUE(ToF1);
+    EXPECT_NE(ToF0, ToF1);
+    EXPECT_EQ(ToF1->getPreviousDecl(), ToF0);
+  }
+};
+
+// Manual instantiation of the fixture with each type.
+using ImportFunctionsVisibilityChain = ImportVisibilityChain<GetFunPattern>;
+using ImportVariablesVisibilityChain = ImportVisibilityChain<GetVarPattern>;
+// Value-parameterized test for the first type.
+TEST_P(ImportFunctionsVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
+// Value-parameterized test for the second type.
+TEST_P(ImportVariablesVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
+
+// Automatic instantiation of the value-parameterized tests.
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain,
+                        ::testing::Combine(
+                           DefaultTestValuesForRunOptions,
+                           ::testing::Values(ExternF, StaticF, AnonF)), );
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ImportVariablesVisibilityChain,
+    ::testing::Combine(
+        DefaultTestValuesForRunOptions,
+        // There is no point to instantiate with StaticV, because in C++ we can
+        // forward declare a variable only with the 'extern' keyword.
+        // Consequently, each fwd declared variable has external linkage.  This
+        // is different in the C language where any declaration without an
+        // initializer is a tentative definition, subsequent definitions may be
+        // provided but they must have the same linkage.  See also the test
+        // ImportVariableChainInC which test for this special C Lang case.
+        ::testing::Values(ExternV, AnonV)), );
+
+// First value in tuple: Compile options.
+// Second value in tuple: Tuple with informations for the test.
+// Code for first import (or initial code), code to import, whether the `f`
+// functions are expected to be linked in a declaration chain.
+// One value of this tuple is combined with every value of compile options.
+// The test can have a single tuple as parameter only.
+using ImportVisibilityParams = ::testing::WithParamInterface<
+    std::tuple<ArgVector, std::tuple<const char *, const char *, bool>>>;
+
+template <typename PatternFactory>
+class ImportVisibility
+    : public ASTImporterTestBase,
+      public ImportVisibilityParams {
+protected:
+  using DeclTy = typename PatternFactory::DeclTy;
+  ArgVector getExtraArgs() const override { return std::get<0>(GetParam()); }
+  std::string getCode0() const { return std::get<0>(std::get<1>(GetParam())); }
+  std::string getCode1() const { return std::get<1>(std::get<1>(GetParam())); }
+  bool shouldBeLinked() const { return std::get<2>(std::get<1>(GetParam())); }
+  BindableMatcher<Decl> getPattern() const { return PatternFactory()(); }
+
+  void TypedTest_ImportAfter() {
+    TranslationUnitDecl *ToTu = getToTuDecl(getCode0(), Lang_CXX);
+    TranslationUnitDecl *FromTu = getTuDecl(getCode1(), Lang_CXX, "input1.cc");
+
+    auto *ToF0 = FirstDeclMatcher<DeclTy>().match(ToTu, getPattern());
+    auto *FromF1 = FirstDeclMatcher<DeclTy>().match(FromTu, getPattern());
+
+    auto *ToF1 = Import(FromF1, Lang_CXX);
+
+    ASSERT_TRUE(ToF0);
+    ASSERT_TRUE(ToF1);
+    EXPECT_NE(ToF0, ToF1);
+
+    if (shouldBeLinked())
+      EXPECT_EQ(ToF1->getPreviousDecl(), ToF0);
+    else
+      EXPECT_FALSE(ToF1->getPreviousDecl());
+  }
+
+  void TypedTest_ImportAfterImport() {
+    TranslationUnitDecl *FromTu0 = getTuDecl(getCode0(), Lang_CXX, "input0.cc");
+    TranslationUnitDecl *FromTu1 = getTuDecl(getCode1(), Lang_CXX, "input1.cc");
+    auto *FromF0 =
+        FirstDeclMatcher<DeclTy>().match(FromTu0, getPattern());
+    auto *FromF1 =
+        FirstDeclMatcher<DeclTy>().match(FromTu1, getPattern());
+    auto *ToF0 = Import(FromF0, Lang_CXX);
+    auto *ToF1 = Import(FromF1, Lang_CXX);
+    ASSERT_TRUE(ToF0);
+    ASSERT_TRUE(ToF1);
+    EXPECT_NE(ToF0, ToF1);
+    if (shouldBeLinked())
+      EXPECT_EQ(ToF1->getPreviousDecl(), ToF0);
+    else
+      EXPECT_FALSE(ToF1->getPreviousDecl());
+  }
+};
+using ImportFunctionsVisibility = ImportVisibility<GetFunPattern>;
+using ImportVariablesVisibility = ImportVisibility<GetVarPattern>;
+
+// FunctionDecl.
+TEST_P(ImportFunctionsVisibility, ImportAfter) {
+  TypedTest_ImportAfter();
+}
+TEST_P(ImportFunctionsVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImport();
+}
+// VarDecl.
+TEST_P(ImportVariablesVisibility, ImportAfter) {
+  TypedTest_ImportAfter();
+}
+TEST_P(ImportVariablesVisibility, ImportAfterImport) {
+  TypedTest_ImportAfterImport();
+}
+
+bool ExpectLink = true;
+bool ExpectNotLink = false;
+
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ImportFunctionsVisibility,
+    ::testing::Combine(
+        DefaultTestValuesForRunOptions,
+        ::testing::Values(std::make_tuple(ExternF, ExternF, ExpectLink),
+                          std::make_tuple(ExternF, StaticF, ExpectNotLink),
+                          std::make_tuple(ExternF, AnonF, ExpectNotLink),
+                          std::make_tuple(StaticF, ExternF, ExpectNotLink),
+                          std::make_tuple(StaticF, StaticF, ExpectNotLink),
+                          std::make_tuple(StaticF, AnonF, ExpectNotLink),
+                          std::make_tuple(AnonF, ExternF, ExpectNotLink),
+                          std::make_tuple(AnonF, StaticF, ExpectNotLink),
+                          std::make_tuple(AnonF, AnonF, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ImportVariablesVisibility,
+    ::testing::Combine(
+        DefaultTestValuesForRunOptions,
+        ::testing::Values(std::make_tuple(ExternV, ExternV, ExpectLink),
+                          std::make_tuple(ExternV, StaticV, ExpectNotLink),
+                          std::make_tuple(ExternV, AnonV, ExpectNotLink),
+                          std::make_tuple(StaticV, ExternV, ExpectNotLink),
+                          std::make_tuple(StaticV, StaticV, ExpectNotLink),
+                          std::make_tuple(StaticV, AnonV, ExpectNotLink),
+                          std::make_tuple(AnonV, ExternV, ExpectNotLink),
+                          std::make_tuple(AnonV, StaticV, ExpectNotLink),
+                          std::make_tuple(AnonV, AnonV, ExpectNotLink))), );
+
+} // namespace TypeAndValueParameterizedTests
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportVariableChainInC) {
+    std::string Code = "static int v; static int v = 0;";
+    auto Pattern = varDecl(hasName("v"));
+
+    TranslationUnitDecl *FromTu = getTuDecl(Code, Lang_C, "input0.c");
+
+    auto *From0 = FirstDeclMatcher<VarDecl>().match(FromTu, Pattern);
+    auto *From1 = LastDeclMatcher<VarDecl>().match(FromTu, Pattern);
+
+    auto *To0 = Import(From0, Lang_C);
+    auto *To1 = Import(From1, Lang_C);
+
+    EXPECT_TRUE(To0);
+    ASSERT_TRUE(To1);
+    EXPECT_NE(To0, To1);
+    EXPECT_EQ(To1->getPreviousDecl(), To0);
+}
+
+TEST_P(ImportFunctions, ImportFromDifferentScopedAnonNamespace) {
+  TranslationUnitDecl *FromTu = getTuDecl(
+      "namespace NS0 { namespace { void f(); } }"
+      "namespace NS1 { namespace { void f(); } }",
+      Lang_CXX, "input0.cc");
+  auto Pattern = functionDecl(hasName("f"));
+
+  auto *FromF0 = FirstDeclMatcher<FunctionDecl>().match(FromTu, Pattern);
+  auto *FromF1 = LastDeclMatcher<FunctionDecl>().match(FromTu, Pattern);
+
+  auto *ToF0 = Import(FromF0, Lang_CXX);
+  auto *ToF1 = Import(FromF1, Lang_CXX);
+
+  EXPECT_TRUE(ToF0);
+  ASSERT_TRUE(ToF1);
+  EXPECT_NE(ToF0, ToF1);
+  EXPECT_FALSE(ToF1->getPreviousDecl());
+}
+
+TEST_P(ImportFunctions, ImportFunctionFromUnnamedNamespace) {
+  {
+    Decl *FromTU = getTuDecl("namespace { void f() {} } void g0() { f(); }",
+                             Lang_CXX, "input0.cc");
+    auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
+        FromTU, functionDecl(hasName("g0")));
+
+    Import(FromD, Lang_CXX);
+  }
+  {
+    Decl *FromTU =
+        getTuDecl("namespace { void f() { int a; } } void g1() { f(); }",
+                  Lang_CXX, "input1.cc");
+    auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
+        FromTU, functionDecl(hasName("g1")));
+    Import(FromD, Lang_CXX);
+  }
+
+  Decl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  ASSERT_EQ(DeclCounter<FunctionDecl>().match(ToTU, functionDecl(hasName("f"))),
+            2u);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
   auto Pattern = functionDecl(hasName("f"));
 
   Decl *FromTU = getTuDecl("struct X { friend void f(); };"
                            "void f();",
                            Lang_CXX,