[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

143

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

148

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

163

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

170

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

184

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

187

Maybe StringRef for the Identifier?

190

Maybe declaring in the same line would be better.

227

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

231

Formatting is off here.

253

Capture From as value.

996

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

1006

Repeated type.

1033

Repeated type.

1062

Repeated type.

1091

Missing periods in comments.

1156

Repeated type.

1198

Repeated type.

[martong]

Fix code review comments.

[martong]

unittests/AST/ASTImporterTest.cpp

170

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.

184

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

167

context_s_; two spaces before 'In'.

178

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

194

Must not be called?

197

This magic constant was factored out in my review.

198

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

202

FromTU?

226

FoundDecls.front()

970

Could you please format inline code?

1082

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

1185

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

1246

This can be written on single line.

1357

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

1435

This Pattern is repeated often.

1486

FunctionDecl *DefinitionD (same upper).

1518

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

198

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

1082

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

1185

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

1357

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

1435

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

970

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

973

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?

1139

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

1141

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

1147

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

1164

Same as upper.

1181

Please make it a complete sentence.

1228

Please stick * to the name (below too).

[martong]

Add minor syntax changes and rename

unittests/AST/ASTImporterTest.cpp

1139

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

1141

That's right, good catch.

1147

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

267

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

280

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

1015

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

267

Added the suffix.

280

Good catch, changed it.

1015

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};
   }
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));
 }
 
+class Fixture : public ::testing::Test {
+
+  const char *const InputFileName = "input.cc";
+  const char *const OutputFileName = "output.cc";
+
+  //Buffers for the contexts, must live in test scope
+  std::string ToCode;
+
+  struct TU {
+    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 context and related translation units.
+  std::list<TU> FromTUs;
+
+  // Creates a virtual file and assigns that to the To context.  If the file
+  // already exists then the file will not be created again as a duplicate.
+  void createVirtualFile(StringRef FileName, const std::string &Code) {
+    assert(ToAST);
+    ASTContext &ToCtx = ToAST->getASTContext();
+    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(FileName, 0, llvm::MemoryBuffer::getMemBuffer(Code.c_str()));
+  }
+
+public:
+  // We may have several From context but only one To context.
+  std::unique_ptr<ASTUnit> ToAST;
+
+  // Creates an AST both for the From and To source code and imports the Decl
+  // of the identifier into the To context.
+  // Must not call more than once within the same test.
+  std::tuple<Decl *, Decl *>
+  getImportedDecl(StringRef FromSrcCode, Language FromLang,
+                  StringRef ToSrcCode, Language ToLang,
+                  const char *const Identifier = "declToImport") {
+    ArgVector FromArgs, ToArgs;
+    FromArgs = getBasicRunOptionsForLanguage(FromLang);
+    ToArgs = getBasicRunOptionsForLanguage(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(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.begin());
+    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= getBasicRunOptionsForLanguage(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 = getBasicRunOptionsForLanguage(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(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);
+  }
+
+  ~Fixture() {
+    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();
+    }
+  }
+};
 
 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_F(Fixture, 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_F(Fixture, 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 (FunctionDecl *FD = dyn_cast<FunctionDecl>(Child)) {
+        if (FD->getNameAsString() == "declToImport") {
+          GTEST_NONFATAL_FAILURE_(
+              "TU should not contain any FunctionDecl with name declToImport");
+          TU->dump();
+          return false;
+        }
+      }
+    }
+    return true;
+  };
+
+  assert(Check(From));
+  Check(To);
+}
+
+TEST_F(Fixture, 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 (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(Child)) {
+        if (RD->getNameAsString() == "declToImport") {
+          GTEST_NONFATAL_FAILURE_(
+              "TU should not contain any CXXRecordDecl with name declToImport");
+          TU->dump();
+          return false;
+        }
+      }
+    }
+    return true;
+  };
+
+  assert(Check(From));
+  Check(To);
+}
+
+TEST_F(Fixture, 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 (TypeAliasDecl *AD = dyn_cast<TypeAliasDecl>(Child)) {
+        if (AD->getNameAsString() == "declToImport") {
+          GTEST_NONFATAL_FAILURE_(
+              "TU should not contain any TypeAliasDecl with name declToImport");
+          TU->dump();
+          return false;
+        }
+      }
+    }
+    return true;
+  };
+
+  assert(Check(From));
+  Check(To);
+}
+
+TEST_F(
+    Fixture,
+    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_F(
+    Fixture,
+    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_F(Fixture, 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(has(fieldDecl()))));
+  ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl()))));
+
+  auto Check = [](Decl *D) -> bool {
+    std::array<const char*, 2> FieldNamesInOrder{{"a", "b"}};
+    int i = 0;
+    for (auto Child : cast<DeclContext>(D)->decls()) {
+      if (FieldDecl *FD = dyn_cast<FieldDecl>(Child)) {
+        if (FD->getNameAsString() != FieldNamesInOrder[i++]) {
+          GTEST_NONFATAL_FAILURE_(
+              "Fields are in wrong order");
+          cast<DeclContext>(D)->dumpDeclContext();
+          D->dump();
+          return false;
+        }
+      }
+    }
+    return true;
+  };
+
+  assert(Check(From));
+  Check(To);
+}
+
+TEST_F(
+    Fixture,
+    DISABLED_CXXRecordDeclFieldsShouldBeInCorrectOrderEvenWhenWeImportFirstTheLastDecl) {
+
+  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(has(fieldDecl()))));
+  ASSERT_TRUE(Verifier.match(To, cxxRecordDecl(has(fieldDecl()))));
+
+  auto Check = [](Decl *D) -> bool {
+    std::array<const char*, 3> FieldNamesInOrder{{"a", "b", "c"}};
+    int i = 0;
+    for (auto Child : cast<DeclContext>(D)->decls()) {
+      if (FieldDecl *FD = dyn_cast<FieldDecl>(Child)) {
+        if (FD->getNameAsString() != FieldNamesInOrder[i++]) {
+          GTEST_NONFATAL_FAILURE_(
+              "Fields are in wrong order");
+          cast<DeclContext>(D)->dumpDeclContext();
+          D->dump();
+          return false;
+        }
+      }
+    }
+    return true;
+  };
+
+  assert(Check(From));
+  Check(To);
+}
+
+TEST_F(Fixture, DISABLED_ShouldImportImplicitCXXRecordDecl) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+    R"(
+    template <typename U>
+    struct declToImport {
+    };
+    )",Lang_CXX, "", Lang_CXX);
+
+  MatchVerifier<Decl> Verifier;
+  // matches the implicit decl
+  auto Matcher = classTemplateDecl(has(cxxRecordDecl(has(cxxRecordDecl()))));
+  ASSERT_TRUE(Verifier.match(From, Matcher));
+  EXPECT_TRUE(Verifier.match(To, Matcher));
+}
+
+TEST_F(
+    Fixture,
+    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_F(Fixture, IDNSOrdinary) {
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl(
+    R"(
+    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_F(Fixture, 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_F(
+    Fixture,
+    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));
+}
+
+struct ImportFunctions : Fixture {};
+
+TEST_F(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();
+
+  // There must be only one imported FunctionDecl ...
+  EXPECT_TRUE(FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern) ==
+              LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern));
+  FunctionDecl *ToFD = LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(ImportedD == ToFD);
+  // .. without a body
+  EXPECT_TRUE(!ToFD->doesThisDeclarationHaveABody());
+}
+
+TEST_F(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();
+
+  // There must be only one imported FunctionDecl ...
+  EXPECT_TRUE(FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern) ==
+              LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern));
+  FunctionDecl *ToFD = LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(ImportedD == ToFD);
+  // .. with a body
+  EXPECT_TRUE(ToFD->doesThisDeclarationHaveABody());
+}
+
+TEST_F(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();
+
+  // There must be only one imported FunctionDecl ...
+  EXPECT_TRUE(FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern) ==
+              LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern));
+  FunctionDecl *ToFD = LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(ImportedD == ToFD);
+  // .. with a body
+  EXPECT_TRUE(ToFD->doesThisDeclarationHaveABody());
+}
+
+TEST_F(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();
+
+  // There must be only one imported FunctionDecl ...
+  EXPECT_TRUE(FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern) ==
+              LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern));
+  FunctionDecl *ToFD = LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(ImportedD == ToFD);
+  // .. with a body
+  EXPECT_TRUE(ToFD->doesThisDeclarationHaveABody());
+}
+
+TEST_F(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();
+
+  // There must be only one imported FunctionDecl ...
+  EXPECT_TRUE(FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern) ==
+              LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern));
+  FunctionDecl *ToFD = LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(ImportedD == ToFD);
+  // .. with a body
+  EXPECT_TRUE(ToFD->doesThisDeclarationHaveABody());
+}
+
+TEST_F(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();
+
+  // There must be only one imported FunctionDecl ...
+  EXPECT_TRUE(FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern) ==
+              LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern));
+  FunctionDecl *ToFD = LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_TRUE(ImportedD == ToFD);
+  // .. with a body
+  EXPECT_TRUE(ToFD->doesThisDeclarationHaveABody());
+}
+
+TEST_F(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();
+  // There must be only one imported FunctionDecl ...
+  EXPECT_TRUE(FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern) ==
+              LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern));
+  FunctionDecl *ToFD = LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_EQ(ImportedD, ImportedD1);
+  EXPECT_TRUE(ImportedD == ToFD);
+  // .. without a body
+  EXPECT_TRUE(!ToFD->doesThisDeclarationHaveABody());
+}
+
+TEST_F(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();
+  // There must be only one imported FunctionDecl ...
+  EXPECT_TRUE(FirstDeclMatcher<FunctionDecl>().match(ToTU, Pattern) ==
+              LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern));
+  FunctionDecl *ToFD = LastDeclMatcher<FunctionDecl>().match(ToTU, Pattern);
+  EXPECT_EQ(ImportedD, ImportedD1);
+  EXPECT_TRUE(ImportedD == ToFD);
+  // .. with a body
+  EXPECT_TRUE(ToFD->doesThisDeclarationHaveABody());
+}
+
+TEST_F(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_F(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_F(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_F(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());
+}
+
 } // 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