[ASTImporter] Add test helper Fixture

[martong]

Summary

Add a helper test Fixture, so we can add tests which can check internal attributes of AST nodes like getPreviousDecl(), isVirtual(), etc.
This enables us to check if a redeclaration chain is correctly built during import, if the virtual flag is preserved during import, etc. We cannot check such attributes with the existing testImport.
Also, this fixture makes it possible to import from several "From" contexts.

We also added several test cases here, some of them are disabled.
We plan to pass the disabled tests in other patches.

[martong]

This comment was removed by martong.

[a.sidorin]

Hello Gabor,
Unfortunately, we did same or similar work: D44079. We have to decide what approach is going to be used.
It doesn't mean that we should select only one - probably, we can use both approaches together and refactor or change our patches correspondingly.

[martong]

Hi Aleksei,

I agree that there are some overlapping functionalities in the patches.
As far as I see both make it possible to write tests which import from multiple contexts and I think that is the only redundant part. So, I agree, for now we could use both approaches simultaneously (i.e. merging both patches).

I think the major strength of this patch is that it makes possible to access the whole API of the AST.
However, with testImportSequence in the other patch it is not clear how could we access the internal attributes of the different AST nodes.
In Ericsson, we have several fixes and other tests which depend on this Fixture and without it we can't really make them open source.

[xazax.hun]

I think having both kinds of tests might make sense.
Overall, this looks good to me. Some nits inline.

unittests/AST/ASTImporterTest.cpp

152

I do not like the name of this class. It is like calling a variable as variable.

157

Missing period on the end of the comment and missing space at the beginning.

172

I think it might worth to have a note why do we use a list and not a vector.

179

Repeated type, use auto. Same below.
Are we sure these cast's will not fail? Shouldn't we use dynamic casts and asserts?

193

I wonder if pair or tuple is the better choice here. I have no strong preference just wondering.

196

Maybe StringRef for the Identifier?

199

Maybe declaring in the same line would be better.

236

I prefer to capture FileName by value. Param names should be upper case. Same below.

240

Formatting is off here.

262

Capture From as value.

1026

Redundant new lines, same in most of the tests' beginning.

1036

Repeated type.

1063

Repeated type.

1092

Repeated type.

1121

Missing periods in comments.

1186

Repeated type.

1228

Repeated type.

[martong]

Fix code review comments.

[martong]

unittests/AST/ASTImporterTest.cpp

179

Actually, we set up the static type in buildASTFromCodeWithArgs to OverlayFileSystem. Still you are right it would be better to use dynamic_cast but we can't since clang and the tests are build with -fno-rtti.

193

I chose tuple here, because it is more general then pair, i.e. in the future we might be able to extend this easier. But this is a weak argument, nevertheless I cannot come up anything besides the pair.

[a.sidorin]

Hello Gabor,

Sorry for the delay with review: the patch is pretty big and it was hard to find time for reviewing it.
I like the patch but it requires some cleanup like formatting changes. Could you please clang-format new code?
Also, I'd like to avoid code duplication. I pointed some places where it happens.

unittests/AST/ASTImporterTest.cpp

176

context_s_; two spaces before 'In'.

187

Could you refactor testImport() to use createVirtualFile() to avoid code duplication?

203

Must not be called?

206

This magic constant was factored out in my review.

207

This means that testing under MSVC is omitted. It caused a lot of buildbot headache before, so I strongly recommend to add it.

211

FromTU?

235

FoundDecls.front()

1000

Could you please format inline code?

1112

Sometimes we start raw literal from the beginning of the line, sometimes - with indent. Is there any common style?

1215

Code duplication with upper example. In my patch, I introduced a matcher for this; I think it can be reused here.

1276

This can be written on single line.

1387

It looks like this piece of code duplicates below several times. If so, I think it should be factored into a separate function.

1465

This Pattern is repeated often.

1516

FunctionDecl *DefinitionD (same upper).

1548

This fits 80-char limit, no need to split.

unittests/AST/DeclMatcher.h

34

We can use ASTContext & directly instead of const Decl *.

49

Member names should start with capital: Count.

60

This will cause confusing miscompile if AMatcher is not a BindableMatcher - like matchers defined with AST_MATCHER-like macros, for example. Isn't better to write this call like this:
Finder.addMatcher(BindableMatcher<NodeType>(AMatcher).bind(""), this);

[martong]

Update based on Alexei's comments.

• Use createVirtualFile in testImport
• Add minimal style changes
• Use DeclCounter
• Add hasFieldOrder matcher
• Add parameterized tests

[martong]

unittests/AST/ASTImporterTest.cpp

207

Good point. Changed all tests to be parameterized on the language options.

1112

Yes you are right it is not consistent. I changed everywhere to follow a common style, to start with an indent.

1215

Agree, the new matcher of yours makes this far simpler, changed to use that.

1387

I have refactored the repeating part by using DeclCounter, because what we actually want to ensure is that there is only one decl.

1465

I don't see why is that a problem, could you please elaborate?

unittests/AST/DeclMatcher.h

34

This DeclMatcher ought to be a generic purpose matcher which might be used outside of ASTImporterTest.cpp as well.
The D as a parameter provides the generality.
Perhaps if we used this only inside ASTImporterTestBase then we could add another overload which uses the ASTContext... but which one, the "to" or the "from".

60

I am not sure if this could work. Actually there is no conversion from the BindableMatcher to a DeclarationMatcher.

../../git/llvm/tools/clang/unittests/AST/DeclMatcher.h:37:12: error: no matching member function for call to 'addMatcher'
    Finder.addMatcher(internal::BindableMatcher<NodeType>(AMatcher).bind(""),
    ~~~~~~~^~~~~~~~~~
../../git/llvm/tools/clang/unittests/AST/ASTImporterTest.cpp:1554:57: note: in instantiation of function template specialization 'clang::ast_matchers::DeclMatcher<clang::CXXMethodDecl, clang::ast_matchers::DeclMatcherKind::Last>::match<clang::ast_matchers::internal::BindableMatcher<clang::Decl> >' requested here
  CXXMethodDecl *Def = LastDeclMatcher<CXXMethodDecl>().match(FromTU, Pattern);
                                                        ^

[a.sidorin]

Hi Gabor! This looks much better.

unittests/AST/ASTImporterTest.cpp

1000

In C++, names started with underscore are reserved for implementation so it's undesirable to use them.

1003

I still can see a number of style violations in inline code. Some examples: lack of space between arguments and parameters, no space before '{' in g(){, etc. Could you please fix them?

1169

Tests with has(fieldDecl()) duplicate tests with hasFieldOrder(). Is it intentional?

1171

Should this be match(From, ...) instead of To?

1177

This identifier is very long. How about renaming it to DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder? It is 60 char long instead of 82.

1194

Same as upper.

1211

Please make it a complete sentence.

1258

Please stick * to the name (below too).

[martong]

Add minor syntax changes and rename

unittests/AST/ASTImporterTest.cpp

1169

Ok, I removed the redundant has(fieldDecl()) checks.

1171

That's right, good catch.

1177

Ok, I like the shorter name, changed it.

[a.sidorin]

This looks good to me. Thank you!
@xazax.hun Gabor, could you please take a look?

[xazax.hun]

Overall looks good, some minor comments inline.

unittests/AST/ASTImporterTest.cpp

276

Maybe an IfNeeded suffix or something like that rather than a comment?

289

When can the TU.Unit be nullptr here? Should this be an assert instead?

1045

I wonder why we only assert on the first one and if we should use GTEST macros here. Same questions below.

[martong]

• Add some minor changes based on xazax's comments

unittests/AST/ASTImporterTest.cpp

276

Added the suffix.

289

Good catch, changed it.

1045

Changed to use GTEST macros.
The reason why we assert only on the first is this: In the first check we verify the original "from" AST, if the assumption we made on the original AST is false then there is no point to continue. However, in the second check we do verify the logic of the importer which may fail. But that is not a fatal failure, because if there were any other checks after that we would like to know if those checks pass or not.
The main difference between EXPECT_* and ASSERT_* is that ASSERT_* will terminate the given test if it fails, but EXPECT_* keep on going and subsequent checks will be executed in the same test.

[martong]

Hi Aleksei,
I don't have commit permissions, could you please help and commit?

[martong]

• Fix gcc 5 warnings

[martong]

Ping.

Could someone with commit rights please commit this? @szepet or @xazax.hun ?

unittests/AST/ASTImporterTest.cpp

 //===----------------------------------------------------------------------===//
 
 #include "MatchVerifier.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTImporter.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+
+#include "DeclMatcher.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace ast_matchers {
 
 typedef std::vector<std::string> ArgVector;
 typedef std::vector<ArgVector> RunOptions;
 
 static bool isCXX(Language Lang) {
   return Lang == Lang_CXX || Lang == Lang_CXX11;
 }
 
-static RunOptions getRunOptionsForLanguage(Language Lang) {
+static ArgVector getBasicRunOptionsForLanguage(Language Lang) {
   ArgVector BasicArgs;
   // Test with basic arguments.
   switch (Lang) {
   case Lang_C:
     BasicArgs = {"-x", "c", "-std=c99"};
     break;
   case Lang_C89:
     BasicArgs = {"-x", "c", "-std=c89"};
     break;
   case Lang_CXX:
     BasicArgs = {"-std=c++98", "-frtti"};
     break;
   case Lang_CXX11:
     BasicArgs = {"-std=c++11", "-frtti"};
     break;
   case Lang_OpenCL:
   case Lang_OBJCXX:
     llvm_unreachable("Not implemented yet!");
   }
+  return BasicArgs;
+}
+
+static RunOptions getRunOptionsForLanguage(Language Lang) {
+  ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang);
 
   // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC
   // default behaviour.
   if (isCXX(Lang)) {
     ArgVector ArgsForDelayedTemplateParse = BasicArgs;
     ArgsForDelayedTemplateParse.emplace_back("-fdelayed-template-parsing");
     return {BasicArgs, ArgsForDelayedTemplateParse};
   }
 
   return {BasicArgs};
 }
 
+// Creates a virtual file and assigns that to the context of given AST. If the
+// file already exists then the file will not be created again as a duplicate.
+static void createVirtualFile(ASTUnit *ToAST, StringRef FileName,
+                              const std::string &Code) {
+  assert(ToAST);
+  ASTContext &ToCtx = ToAST->getASTContext();
+  auto *OFS = static_cast<vfs::OverlayFileSystem *>(
+      ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
+  auto *MFS =
+      static_cast<vfs::InMemoryFileSystem *>(OFS->overlays_begin()->get());
+  MFS->addFile(FileName, 0, llvm::MemoryBuffer::getMemBuffer(Code.c_str()));
+}
+
 template<typename NodeType, typename MatcherType>
 testing::AssertionResult
 testImport(const std::string &FromCode, const ArgVector &FromArgs,
            const std::string &ToCode, const ArgVector &ToArgs,
            MatchVerifier<NodeType> &Verifier, const MatcherType &AMatcher) {
   const char *const InputFileName = "input.cc";
   const char *const OutputFileName = "output.cc";
 
   std::unique_ptr<ASTUnit>
       FromAST = tooling::buildASTFromCodeWithArgs(
         FromCode, FromArgs, InputFileName),
       ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
 
   ASTContext &FromCtx = FromAST->getASTContext(),
       &ToCtx = ToAST->getASTContext();
 
   // Add input.cc to virtual file system so importer can 'find' it
   // while importing SourceLocations.
-  vfs::OverlayFileSystem *OFS = static_cast<vfs::OverlayFileSystem *>(
-        ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
-  vfs::InMemoryFileSystem *MFS = static_cast<vfs::InMemoryFileSystem *>(
-        OFS->overlays_begin()->get());
-  MFS->addFile(InputFileName, 0, llvm::MemoryBuffer::getMemBuffer(FromCode));
+  createVirtualFile(ToAST.get(), InputFileName, FromCode);
 
   ASTImporter Importer(ToCtx, ToAST->getFileManager(),
                        FromCtx, FromAST->getFileManager(), false);
 
   IdentifierInfo *ImportedII = &FromCtx.Idents.get("declToImport");
   assert(ImportedII && "Declaration with 'declToImport' name"
                        "should be specified in test!");
   DeclarationName ImportDeclName(ImportedII);
void testImport(const std::string &FromCode, Language FromLang,
   auto RunOptsFrom = getRunOptionsForLanguage(FromLang);
   auto RunOptsTo = getRunOptionsForLanguage(ToLang);
   for (const auto &FromArgs : RunOptsFrom)
     for (const auto &ToArgs : RunOptsTo)
       EXPECT_TRUE(testImport(FromCode, FromArgs, ToCode, ToArgs,
                              Verifier, AMatcher));
 }
 
+const StringRef DeclToImportID = "declToImport";
+
+// This class provides generic methods to write tests which can check internal
+// attributes of AST nodes like getPreviousDecl(), isVirtual(), etc.  Also,
+// this fixture makes it possible to import from several "From" contexts.
+class ASTImporterTestBase : public ::testing::TestWithParam<ArgVector> {
+
+  const char *const InputFileName = "input.cc";
+  const char *const OutputFileName = "output.cc";
+
+  // Buffer for the To context, must live in the test scope.
+  std::string ToCode;
+
+  struct TU {
+    // Buffer for the context, must live in the test scope.
+    std::string Code;
+    std::string FileName;
+    std::unique_ptr<ASTUnit> Unit;
+    TranslationUnitDecl *TUDecl = nullptr;
+    TU(StringRef Code, StringRef FileName, ArgVector Args)
+        : Code(Code), FileName(FileName),
+          Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
+                                                 this->FileName)),
+          TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
+  };
+
+  // We may have several From contexts and related translation units. In each
+  // AST, the buffers for the source are handled via references and are set
+  // during the creation of the AST. These references must point to a valid
+  // buffer until the AST is alive. Thus, we must use a list in order to avoid
+  // moving of the stored objects because that would mean breaking the
+  // references in the AST. By using a vector a move could happen when the
+  // vector is expanding, with the list we won't have these issues.
+  std::list<TU> FromTUs;
+
+public:
+  // We may have several From context but only one To context.
+  std::unique_ptr<ASTUnit> ToAST;
+
+  // Returns the argument vector used for a specific language, this set
+  // can be tweaked by the test parameters.
+  ArgVector getArgVectorForLanguage(Language Lang) {
+    ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+    ArgVector ExtraArgs = GetParam();
+    for (const auto& Arg : ExtraArgs) {
+      Args.push_back(Arg);
+    }
+    return Args;
+  }
+
+  // Creates an AST both for the From and To source code and imports the Decl
+  // of the identifier into the To context.
+  // Must not be called more than once within the same test.
+  std::tuple<Decl *, Decl *>
+  getImportedDecl(StringRef FromSrcCode, Language FromLang, StringRef ToSrcCode,
+                  Language ToLang, StringRef Identifier = DeclToImportID) {
+    ArgVector FromArgs = getArgVectorForLanguage(FromLang),
+              ToArgs = getArgVectorForLanguage(ToLang);
+
+    FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs);
+    TU &FromTU = FromTUs.back();
+
+    ToCode = ToSrcCode;
+    assert(!ToAST);
+    ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+
+    ASTContext &FromCtx = FromTU.Unit->getASTContext(),
+               &ToCtx = ToAST->getASTContext();
+
+    createVirtualFile(ToAST.get(), InputFileName, FromTU.Code);
+
+    ASTImporter Importer(ToCtx, ToAST->getFileManager(), FromCtx,
+                         FromTU.Unit->getFileManager(), false);
+
+    IdentifierInfo *ImportedII = &FromCtx.Idents.get(Identifier);
+    assert(ImportedII && "Declaration with the given identifier "
+                         "should be specified in test!");
+    DeclarationName ImportDeclName(ImportedII);
+    SmallVector<NamedDecl *, 4> FoundDecls;
+    FromCtx.getTranslationUnitDecl()->localUncachedLookup(ImportDeclName,
+                                                          FoundDecls);
+
+    assert(FoundDecls.size() == 1);
+
+    Decl *Imported = Importer.Import(FoundDecls.front());
+    assert(Imported);
+    return std::make_tuple(*FoundDecls.begin(), Imported);
+  }
+
+  // Creates a TU decl for the given source code.
+  // May be called several times in a given test.
+  TranslationUnitDecl *getTuDecl(StringRef SrcCode, Language Lang,
+                                 StringRef FileName = "input.cc") {
+    assert(
+        std::find_if(FromTUs.begin(), FromTUs.end(), [FileName](const TU &E) {
+          return E.FileName == FileName;
+        }) == FromTUs.end());
+
+    ArgVector Args = getArgVectorForLanguage(Lang);
+    FromTUs.emplace_back(SrcCode, FileName, Args);
+    TU &Tu = FromTUs.back();
+
+    return Tu.TUDecl;
+  }
+
+  // Import the given Decl into the ToCtx.
+  // May be called several times in a given test.
+  // The different instances of the param From may have different ASTContext.
+  Decl *Import(Decl *From, Language ToLang) {
+    if (!ToAST) {
+      ArgVector ToArgs = getArgVectorForLanguage(ToLang);
+      // Build the AST from an empty file.
+      ToAST =
+          tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc");
+    }
+
+    // Create a virtual file in the To Ctx which corresponds to the file from
+    // which we want to import the `From` Decl. Without this source locations
+    // will be invalid in the ToCtx.
+    auto It = std::find_if(FromTUs.begin(), FromTUs.end(), [From](const TU &E) {
+      return E.TUDecl == From->getTranslationUnitDecl();
+    });
+    assert(It != FromTUs.end());
+    // This will not create the file more than once.
+    createVirtualFile(ToAST.get(), It->FileName, It->Code);
+
+    ASTContext &FromCtx = From->getASTContext(),
+               &ToCtx = ToAST->getASTContext();
+    ASTImporter Importer(ToCtx, ToAST->getFileManager(), FromCtx,
+                         FromCtx.getSourceManager().getFileManager(), false);
+    return Importer.Import(From);
+  }
+
+  ~ASTImporterTestBase() {
+    if (!::testing::Test::HasFailure()) return;
+
+    for (auto &Tu : FromTUs) {
+      if (Tu.Unit) {
+        llvm::errs() << "FromAST:\n";
+        Tu.Unit->getASTContext().getTranslationUnitDecl()->dump();
+        llvm::errs() << "\n";
+      }
+    }
+    if (ToAST) {
+      llvm::errs() << "ToAST:\n";
+      ToAST->getASTContext().getTranslationUnitDecl()->dump();
+    }
+  }
+};
+
+AST_MATCHER_P(RecordDecl, hasFieldOrder, std::vector<StringRef>, Order) {
+  size_t Index = 0;
+  for (FieldDecl *Field : Node.fields()) {
+    if (Index == Order.size())
+      return false;
+    if (Field->getName() != Order[Index])
+      return false;
+    ++Index;
+  }
+  return Index == Order.size();
+}
 
 TEST(ImportExpr, ImportStringLiteral) {
   MatchVerifier<Decl> Verifier;
   testImport("void declToImport() { \"foo\"; }",
              Lang_CXX, "", Lang_CXX, Verifier,
              functionDecl(
                hasBody(
                  compoundStmt(
TEST(ImportExpr, DependentSizedArrayType) {
   testImport("template<typename T, int Size> class declToImport {"
              "  T data[Size];"
              "};",
              Lang_CXX, "", Lang_CXX, Verifier,
              classTemplateDecl(has(cxxRecordDecl(
                  has(fieldDecl(hasType(dependentSizedArrayType())))))));
 }
 
+TEST_P(ASTImporterTestBase, DISABLED_ImportFunctionWithBackReferringParameter) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+      R"(
+      template <typename T> struct X {};
+
+      void declToImport(int y, X<int> &x) {}
+
+      template <> struct X<int> {
+        void g() {
+          X<int> x;
+          declToImport(0, x);
+        }
+      };
+      )",
+      Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier<Decl> Verifier;
+  auto Matcher = functionDecl(hasName("declToImport"),
+                              parameterCountIs(2),
+                              hasParameter(0, hasName("y")),
+                              hasParameter(1, hasName("x")),
+                              hasParameter(1, hasType(asString("X<int> &"))));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_P(ASTImporterTestBase,
+       TUshouldNotContainTemplatedDeclOfFunctionTemplates) {
+  Decl *From, *To;
+  std::tie(From, To) =
+      getImportedDecl("template <typename T> void declToImport() { T a = 1; }"
+                      "void instantiate() { declToImport<int>(); }",
+                      Lang_CXX, "", Lang_CXX);
+
+  auto Check = [](Decl *D) -> bool {
+    auto TU = D->getTranslationUnitDecl();
+    for (auto Child : TU->decls()) {
+      if (auto *FD = dyn_cast<FunctionDecl>(Child)) {
+        if (FD->getNameAsString() == "declToImport") {
+          GTEST_NONFATAL_FAILURE_(
+              "TU should not contain any FunctionDecl with name declToImport");
+          return false;
+        }
+      }
+    }
+    return true;
+  };
+
+  assert(Check(From));
+  Check(To);
+}
+
+TEST_P(ASTImporterTestBase, TUshouldNotContainTemplatedDeclOfClassTemplates) {
+  Decl *From, *To;
+  std::tie(From, To) =
+      getImportedDecl("template <typename T> struct declToImport { T t; };"
+                      "void instantiate() { declToImport<int>(); }",
+                      Lang_CXX, "", Lang_CXX);
+
+  auto Check = [](Decl *D) -> bool {
+    auto TU = D->getTranslationUnitDecl();
+    for (auto Child : TU->decls()) {
+      if (auto *RD = dyn_cast<CXXRecordDecl>(Child)) {
+        if (RD->getNameAsString() == "declToImport") {
+          GTEST_NONFATAL_FAILURE_(
+              "TU should not contain any CXXRecordDecl with name declToImport");
+          return false;
+        }
+      }
+    }
+    return true;
+  };
+
+  assert(Check(From));
+  Check(To);
+}
+
+TEST_P(ASTImporterTestBase, TUshouldNotContainTemplatedDeclOfTypeAlias) {
+  Decl *From, *To;
+  std::tie(From, To) =
+      getImportedDecl(
+          "template <typename T> struct X {};"
+          "template <typename T> using declToImport = X<T>;"
+          "void instantiate() { declToImport<int> a; }",
+                      Lang_CXX11, "", Lang_CXX11);
+
+  auto Check = [](Decl *D) -> bool {
+    auto TU = D->getTranslationUnitDecl();
+    for (auto Child : TU->decls()) {
+      if (auto *AD = dyn_cast<TypeAliasDecl>(Child)) {
+        if (AD->getNameAsString() == "declToImport") {
+          GTEST_NONFATAL_FAILURE_(
+              "TU should not contain any TypeAliasDecl with name declToImport");
+          return false;
+        }
+      }
+    }
+    return true;
+  };
+
+  assert(Check(From));
+  Check(To);
+}
+
+TEST_P(
+    ASTImporterTestBase,
+    DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) {
+
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+      R"(
+      template<class T>
+      class Base {};
+      class declToImport : public Base<declToImport> {};
+      )",
+      Lang_CXX, "", Lang_CXX);
+
+  // Check that the ClassTemplateSpecializationDecl is NOT the child of the TU.
+  auto Pattern =
+      translationUnitDecl(unless(has(classTemplateSpecializationDecl())));
+  ASSERT_TRUE(
+      MatchVerifier<Decl>{}.match(From->getTranslationUnitDecl(), Pattern));
+  EXPECT_TRUE(
+      MatchVerifier<Decl>{}.match(To->getTranslationUnitDecl(), Pattern));
+
+  // Check that the ClassTemplateSpecializationDecl is the child of the
+  // ClassTemplateDecl.
+  Pattern = translationUnitDecl(has(classTemplateDecl(
+      hasName("Base"), has(classTemplateSpecializationDecl()))));
+  ASSERT_TRUE(
+      MatchVerifier<Decl>{}.match(From->getTranslationUnitDecl(), Pattern));
+  EXPECT_TRUE(
+      MatchVerifier<Decl>{}.match(To->getTranslationUnitDecl(), Pattern));
+}
+
+TEST_P(ASTImporterTestBase,
+       TUshouldContainClassTemplateSpecializationOfExplicitInstantiation) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+      R"(
+      namespace NS {
+        template<class T>
+        class X {};
+        template class X<int>;
+      }
+      )",
+      Lang_CXX, "", Lang_CXX, "NS");
+
+  // Check that the ClassTemplateSpecializationDecl is NOT the child of the
+  // ClassTemplateDecl.
+  auto Pattern = namespaceDecl(has(classTemplateDecl(
+      hasName("X"), unless(has(classTemplateSpecializationDecl())))));
+  ASSERT_TRUE(MatchVerifier<Decl>{}.match(From, Pattern));
+  EXPECT_TRUE(MatchVerifier<Decl>{}.match(To, Pattern));
+
+  // Check that the ClassTemplateSpecializationDecl is the child of the
+  // NamespaceDecl.
+  Pattern = namespaceDecl(has(classTemplateSpecializationDecl(hasName("X"))));
+  ASSERT_TRUE(MatchVerifier<Decl>{}.match(From, Pattern));
+  EXPECT_TRUE(MatchVerifier<Decl>{}.match(To, Pattern));
+}
+
+TEST_P(ASTImporterTestBase, CXXRecordDeclFieldsShouldBeInCorrectOrder) {
+  Decl *From, *To;
+  std::tie(From, To) =
+      getImportedDecl(
+          "struct declToImport { int a; int b; };",
+                      Lang_CXX11, "", Lang_CXX11);
+
+  MatchVerifier<Decl> Verifier;
+  ASSERT_TRUE(Verifier.match(From, cxxRecordDecl(hasFieldOrder({"a", "b"}))));
+  EXPECT_TRUE(Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b"}))));
+}
+
+TEST_P(ASTImporterTestBase,
+       DISABLED_CXXRecordDeclFieldOrderShouldNotDependOnImportOrder) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+      // The original recursive algorithm of ASTImporter first imports 'c' then
+      // 'b' and lastly 'a'.  Therefore we must restore the order somehow.
+      R"s(
+      struct declToImport {
+          int a = c + b;
+          int b = 1;
+          int c = 2;
+      };
+      )s",
+      Lang_CXX11, "", Lang_CXX11);
+
+  MatchVerifier<Decl> Verifier;
+  ASSERT_TRUE(
+      Verifier.match(From, cxxRecordDecl(hasFieldOrder({"a", "b", "c"}))));
+  EXPECT_TRUE(
+      Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"}))));
+}
+
+TEST_P(ASTImporterTestBase, DISABLED_ShouldImportImplicitCXXRecordDecl) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+      R"(
+      template <typename U>
+      struct declToImport {
+      };
+      )",
+      Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier<Decl> Verifier;
+  // Match the implicit Decl.
+  auto Matcher = classTemplateDecl(has(cxxRecordDecl(has(cxxRecordDecl()))));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_P(
+    ASTImporterTestBase,
+    DISABLED_ShouldImportImplicitCXXRecordDeclOfClassTemplateSpecializationDecl) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+      R"(
+      template<class T>
+      class Base {};
+      class declToImport : public Base<declToImport> {};
+      )",
+      Lang_CXX, "", Lang_CXX);
+
+  auto hasImplicitClass = has(cxxRecordDecl());
+  auto Pattern = translationUnitDecl(has(classTemplateDecl(
+      hasName("Base"),
+      has(classTemplateSpecializationDecl(hasImplicitClass)))));
+  ASSERT_TRUE(
+      MatchVerifier<Decl>{}.match(From->getTranslationUnitDecl(), Pattern));
+  EXPECT_TRUE(
+      MatchVerifier<Decl>{}.match(To->getTranslationUnitDecl(), Pattern));
+}
+
+TEST_P(ASTImporterTestBase, IDNSOrdinary) {
+  Decl *From, *To;
+  std::tie(From, To) =
+      getImportedDecl("void declToImport() {}", Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier<Decl> Verifier;
+  auto Matcher = functionDecl();
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+  EXPECT_EQ(From->getIdentifierNamespace(), To->getIdentifierNamespace());
+}
+
+TEST_P(ASTImporterTestBase, DISABLED_IDNSOfNonmemberOperator) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      struct X {};
+      void operator<<(int, X);
+      )",
+      Lang_CXX);
+  Decl *From = LastDeclMatcher<Decl>{}.match(FromTU, functionDecl());
+  const Decl *To = Import(From, Lang_CXX);
+  EXPECT_EQ(From->getIdentifierNamespace(), To->getIdentifierNamespace());
+}
+
+TEST_P(ASTImporterTestBase,
+       ShouldImportMembersOfClassTemplateSpecializationDecl) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+      R"(
+      template<class T>
+      class Base { int a; };
+      class declToImport : Base<declToImport> {};
+      )",
+      Lang_CXX, "", Lang_CXX);
+
+  auto Pattern = translationUnitDecl(has(classTemplateDecl(
+      hasName("Base"),
+      has(classTemplateSpecializationDecl(has(fieldDecl(hasName("a"))))))));
+  ASSERT_TRUE(
+      MatchVerifier<Decl>{}.match(From->getTranslationUnitDecl(), Pattern));
+  EXPECT_TRUE(
+      MatchVerifier<Decl>{}.match(To->getTranslationUnitDecl(), Pattern));
+}
+
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ASTImporterTestBase,
+    ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}));
+
+struct ImportFunctions : ASTImporterTestBase {};
+
+TEST_P(ImportFunctions,
+       PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition) {
+  Decl *FromTU = getTuDecl("void f();", Lang_CXX);
+  auto Pattern = functionDecl(hasName("f"));
+  FunctionDecl *FromD =
+      FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+  Decl *ImportedD = Import(FromD, Lang_CXX);
+  Decl *ToTU = ImportedD->getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 1u);
+  EXPECT_TRUE(!cast<FunctionDecl>(ImportedD)->doesThisDeclarationHaveABody());
+}
+
+TEST_P(ImportFunctions,
+       PrototypeShouldBeImportedAsDefintionWhenThereIsADefinition) {
+  Decl *FromTU = getTuDecl("void f(); void f() {}", Lang_CXX);
+  auto Pattern = functionDecl(hasName("f"));
+  FunctionDecl *FromD = // Prototype
+      FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+  Decl *ImportedD = Import(FromD, Lang_CXX);
+  Decl *ToTU = ImportedD->getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 1u);
+  EXPECT_TRUE(cast<FunctionDecl>(ImportedD)->doesThisDeclarationHaveABody());
+}
+
+TEST_P(ImportFunctions,
+       DefinitionShouldBeImportedAsDefintionWhenThereIsAPrototype) {
+  Decl *FromTU = getTuDecl("void f(); void f() {}", Lang_CXX);
+  auto Pattern = functionDecl(hasName("f"));
+  FunctionDecl *FromD = // Definition
+      LastDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+  Decl *ImportedD = Import(FromD, Lang_CXX);
+  Decl *ToTU = ImportedD->getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 1u);
+  EXPECT_TRUE(cast<FunctionDecl>(ImportedD)->doesThisDeclarationHaveABody());
+}
+
+TEST_P(ImportFunctions, DefinitionShouldBeImportedAsADefinition) {
+  Decl *FromTU = getTuDecl("void f() {}", Lang_CXX);
+  auto Pattern = functionDecl(hasName("f"));
+  FunctionDecl *FromD =
+      FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+  Decl *ImportedD = Import(FromD, Lang_CXX);
+  Decl *ToTU = ImportedD->getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 1u);
+  EXPECT_TRUE(cast<FunctionDecl>(ImportedD)->doesThisDeclarationHaveABody());
+}
+
+TEST_P(ImportFunctions, DISABLED_ImportPrototypeOfRecursiveFunction) {
+  Decl *FromTU = getTuDecl("void f(); void f() { f(); }", Lang_CXX);
+  auto Pattern = functionDecl(hasName("f"));
+  FunctionDecl *PrototypeFD =
+      FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+  Decl *ImportedD = Import(PrototypeFD, Lang_CXX);
+  Decl *ToTU = ImportedD->getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 1u);
+  EXPECT_TRUE(cast<FunctionDecl>(ImportedD)->doesThisDeclarationHaveABody());
+}
+
+TEST_P(ImportFunctions, ImportDefinitionOfRecursiveFunction) {
+  Decl *FromTU = getTuDecl("void f(); void f() { f(); }", Lang_CXX);
+  auto Pattern = functionDecl(hasName("f"));
+  FunctionDecl *DefinitionFD =
+      LastDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+  Decl *ImportedD = Import(DefinitionFD, Lang_CXX);
+  Decl *ToTU = ImportedD->getTranslationUnitDecl();
+
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 1u);
+  EXPECT_TRUE(cast<FunctionDecl>(ImportedD)->doesThisDeclarationHaveABody());
+}
+
+TEST_P(ImportFunctions, ImportPrototypes) {
+  auto Pattern = functionDecl(hasName("f"));
+
+  Decl *ImportedD;
+  {
+    Decl *FromTU = getTuDecl("void f();", Lang_CXX, "input0.cc");
+    FunctionDecl *FromD =
+        FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+    ImportedD = Import(FromD, Lang_CXX);
+  }
+  Decl *ImportedD1;
+  {
+    Decl *FromTU = getTuDecl("void f();", Lang_CXX, "input1.cc");
+    FunctionDecl *FromD =
+        FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+    ImportedD1 = Import(FromD, Lang_CXX);
+  }
+
+  Decl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 1u);
+  EXPECT_TRUE(!cast<FunctionDecl>(ImportedD)->doesThisDeclarationHaveABody());
+}
+
+TEST_P(ImportFunctions, ImportDefinitionThenPrototype) {
+  auto Pattern = functionDecl(hasName("f"));
+
+  Decl *ImportedD;
+  {
+    Decl *FromTU = getTuDecl("void f(){}", Lang_CXX, "input0.cc");
+    FunctionDecl *FromD =
+        FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+    ImportedD = Import(FromD, Lang_CXX);
+  }
+  Decl *ImportedD1;
+  {
+    Decl *FromTU = getTuDecl("void f();", Lang_CXX, "input1.cc");
+    FunctionDecl *FromD =
+        FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+    ImportedD1 = Import(FromD, Lang_CXX);
+  }
+
+  Decl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  EXPECT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 1u);
+  EXPECT_TRUE(cast<FunctionDecl>(ImportedD)->doesThisDeclarationHaveABody());
+}
+
+TEST_P(ImportFunctions, ImportPrototypeThenDefinition) {
+  auto Pattern = functionDecl(hasName("f"));
+
+  {
+    Decl *FromTU = getTuDecl("void f();", Lang_CXX, "input0.cc");
+    FunctionDecl *FromD =
+        FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+    Import(FromD, Lang_CXX);
+  }
+  {
+    Decl *FromTU = getTuDecl("void f(){}", Lang_CXX, "input1.cc");
+    FunctionDecl *FromD =
+        FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+    Import(FromD, Lang_CXX);
+  }
+
+  Decl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  ASSERT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 2u);
+  FunctionDecl *ProtoD = FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(!ProtoD->doesThisDeclarationHaveABody());
+  FunctionDecl *DefinitionD =
+      LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(DefinitionD->doesThisDeclarationHaveABody());
+  EXPECT_EQ(DefinitionD->getPreviousDecl(), ProtoD);
+}
+
+TEST_P(ImportFunctions, DISABLED_ImportPrototypeThenProtoAndDefinition) {
+  auto Pattern = functionDecl(hasName("f"));
+
+  {
+    Decl *FromTU = getTuDecl("void f();", Lang_CXX, "input0.cc");
+    FunctionDecl *FromD =
+        FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+
+    Import(FromD, Lang_CXX);
+  }
+  {
+    Decl *FromTU = getTuDecl("void f(); void f(){}", Lang_CXX, "input1.cc");
+    FunctionDecl *FromD =
+        FirstDeclMatcher<FunctionDecl>().match(FromTU, Pattern);
+    Import(FromD, Lang_CXX);
+  }
+
+  Decl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  ASSERT_EQ(DeclCounter<FunctionDecl>().match(ToTU, Pattern), 2u);
+  FunctionDecl *ProtoD = FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(!ProtoD->doesThisDeclarationHaveABody());
+  FunctionDecl *DefinitionD =
+      LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(DefinitionD->doesThisDeclarationHaveABody());
+  EXPECT_EQ(DefinitionD->getPreviousDecl(), ProtoD);
+}
+
+TEST_P(ImportFunctions, OverriddenMethodsShouldBeImported) {
+  auto Code =
+      R"(
+      struct B { virtual void f(); };
+      void B::f() {}
+      struct D : B { void f(); };
+      )";
+  auto Pattern =
+      cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("D"))));
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  CXXMethodDecl *Proto =
+      FirstDeclMatcher<CXXMethodDecl>().match(FromTU, Pattern);
+
+  ASSERT_EQ(Proto->size_overridden_methods(), 1u);
+  CXXMethodDecl *To = cast<CXXMethodDecl>(Import(Proto, Lang_CXX));
+  EXPECT_EQ(To->size_overridden_methods(), 1u);
+}
+
+TEST_P(ImportFunctions, VirtualFlagShouldBePreservedWhenImportingPrototype) {
+  auto Code =
+      R"(
+      struct B { virtual void f(); };
+      void B::f() {}
+      )";
+  auto Pattern =
+      cxxMethodDecl(hasName("f"), hasParent(cxxRecordDecl(hasName("B"))));
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  CXXMethodDecl *Proto =
+      FirstDeclMatcher<CXXMethodDecl>().match(FromTU, Pattern);
+  CXXMethodDecl *Def = LastDeclMatcher<CXXMethodDecl>().match(FromTU, Pattern);
+
+  ASSERT_TRUE(Proto->isVirtual());
+  ASSERT_TRUE(Def->isVirtual());
+  CXXMethodDecl *To = cast<CXXMethodDecl>(Import(Proto, Lang_CXX));
+  EXPECT_TRUE(To->isVirtual());
+}
+
+INSTANTIATE_TEST_CASE_P(
+    ParameterizedTests, ImportFunctions,
+    ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}));
+
 } // end namespace ast_matchers
 } // end namespace clang

unittests/AST/DeclMatcher.h

+//===- unittest/AST/DeclMatcher.h - AST unit test support ---------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H
+#define LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace ast_matchers {
+
+enum class DeclMatcherKind { First, Last };
+
+// Matcher class to retrieve the first/last matched node under a given AST.
+template <typename NodeType, DeclMatcherKind MatcherKind>
+class DeclMatcher : public MatchFinder::MatchCallback {
+  NodeType *Node = nullptr;
+  void run(const MatchFinder::MatchResult &Result) override {
+    if ((MatcherKind == DeclMatcherKind::First && Node == nullptr) ||
+        MatcherKind == DeclMatcherKind::Last) {
+      Node = const_cast<NodeType *>(Result.Nodes.getNodeAs<NodeType>(""));
+    }
+  }
+public:
+  // Returns the first/last matched node under the tree rooted in `D`.
+  template <typename MatcherType>
+  NodeType *match(const Decl *D, const MatcherType &AMatcher) {
+    MatchFinder Finder;
+    Finder.addMatcher(AMatcher.bind(""), this);
+    Finder.matchAST(D->getASTContext());
+    assert(Node);
+    return Node;
+  }
+};
+template <typename NodeType>
+using LastDeclMatcher = DeclMatcher<NodeType, DeclMatcherKind::Last>;
+template <typename NodeType>
+using FirstDeclMatcher = DeclMatcher<NodeType, DeclMatcherKind::First>;
+
+template <typename NodeType>
+class DeclCounter : public MatchFinder::MatchCallback {
+  unsigned Count = 0;
+  void run(const MatchFinder::MatchResult &Result) override {
+      if(Result.Nodes.getNodeAs<NodeType>("")) {
+        ++Count;
+      }
+  }
+public:
+  // Returns the number of matched nodes under the tree rooted in `D`.
+  template <typename MatcherType>
+  unsigned match(const Decl *D, const MatcherType &AMatcher) {
+    MatchFinder Finder;
+    Finder.addMatcher(AMatcher.bind(""), this);
+    Finder.matchAST(D->getASTContext());
+    return Count;
+  }
+};
+
+} // end namespace ast_matchers
+} // end namespace clang
+
+#endif