[ASTImporter] Add unit tests for structural equivalence

[martong]

Summary

This patch add new tests for structural equivalence. For that a new common header is created which holds the test related language specific types and functions.

[a.sidorin]

Hello Gabor!

Thank you for this patch! Do you plan to enable this functionality for AST import checking?
Some comments are inline.

unittests/AST/Language.h

2

Header line is too short.

9

Could you please add a brief file description?

14

System includes should follow LLVM includes.

38

I think this function is too big for a header. Same below. Could you create a source file?

unittests/AST/StructuralEquivalenceTest.cpp

3

Do we need ASTImporter.h?

40

s/Import/Search?

48

Nit: comments should end with dot. Could you please check?

49

Can we assert(FoundDecls.size() == 1)?

55

D0, D1 (capital). Same below.

74

Could we just overload testStructuralMatch to accept tuple or pair?

120

Should we assert for spec_begin() != spec_end() instead or within these assertions?

164

There is no Code1, so I think we can call it just "Code". Same below.

175

1. It's better to use more meaningful names than t. DeclTuple?
2. The space after ( is redundant.

194

Redundant space. Could you clang-format?

196

These assertions are always true because there is a strong C assertion in the callee.

[martong]

Do you plan to enable this functionality for AST import checking?

Yes. We'd like to test the structural equivalency independently from ASTImporter, because in certain cases it may have faulty behavior. This can be very handy also when we further extend structural equivalency.

unittests/AST/StructuralEquivalenceTest.cpp

120

I think that is not needed, because the previous cast in line 116 and 118 would assert if spec_begin() == spec_end() was true.

175

Renamed t to Decls.

[martong]

• Address aleksei's comments

[a.sidorin]

Hi Gabor,

LGTM, thank you for addressing the comments! Just a minor nit, it's OK to fix it before committing without a separate review.

unittests/AST/StructuralEquivalenceTest.cpp

68

Not sure we need this using: we can move up the using below or just write std::get twice.

[martong]

Moved using std::get up, before testStructuralMatch.

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 "Language.h"
 #include "gtest/gtest.h"
 #include "llvm/ADT/StringMap.h"
 
 namespace clang {
 namespace ast_matchers {
 
 using internal::Matcher;
 using internal::BindableMatcher;
 using llvm::StringMap;
 
-typedef std::vector<std::string> ArgVector;
-typedef std::vector<ArgVector> RunOptions;
-
-static bool isCXX(Language Lang) {
-  return Lang == Lang_CXX || Lang == Lang_CXX11;
-}
-
-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
 createVirtualFileIfNeeded(ASTUnit *ToAST, StringRef FileName,
                           std::unique_ptr<llvm::MemoryBuffer> &&Buffer) {
   assert(ToAST);
   ASTContext &ToCtx = ToAST->getASTContext();
   auto *OFS = static_cast<vfs::OverlayFileSystem *>(

unittests/AST/CMakeLists.txt

add_clang_unittest(ASTTests
   CommentLexer.cpp
   CommentParser.cpp
   CommentTextTest.cpp
   DataCollectionTest.cpp
   DeclPrinterTest.cpp
   DeclTest.cpp
   EvaluateAsRValueTest.cpp
   ExternalASTSourceTest.cpp
+  Language.cpp
   NamedDeclPrinterTest.cpp
   SourceLocationTest.cpp
   StmtPrinterTest.cpp
+  StructuralEquivalenceTest.cpp
   )
 
 target_link_libraries(ASTTests
   PRIVATE
   clangAST
   clangASTMatchers
   clangBasic
   clangFrontend
   clangTooling
   )

unittests/AST/Language.h

+//===------ unittest/AST/Language.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.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines language options for AST unittests.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_UNITTESTS_AST_LANGUAGE_H
+#define LLVM_CLANG_UNITTESTS_AST_LANGUAGE_H
+
+#include "llvm/Support/ErrorHandling.h"
+#include <vector>
+#include <string>
+
+namespace clang {
+namespace ast_matchers {
+
+typedef std::vector<std::string> ArgVector;
+typedef std::vector<ArgVector> RunOptions;
+
+enum Language {
+    Lang_C,
+    Lang_C89,
+    Lang_CXX,
+    Lang_CXX11,
+    Lang_CXX14,
+    Lang_OpenCL,
+    Lang_OBJCXX
+};
+
+inline bool isCXX(Language Lang) {
+  return Lang == Lang_CXX || Lang == Lang_CXX11 || Lang == Lang_CXX14;
+}
+
+ArgVector getBasicRunOptionsForLanguage(Language Lang);
+RunOptions getRunOptionsForLanguage(Language Lang);
+
+} // end namespace ast_matchers
+} // end namespace clang
+
+#endif

unittests/AST/Language.cpp

+//===------ unittest/AST/Language.cpp - 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.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines language options for AST unittests.
+//
+//===----------------------------------------------------------------------===//
+
+#include "Language.h"
+
+namespace clang {
+namespace ast_matchers {
+
+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_CXX14:
+    BasicArgs = {"-std=c++14", "-frtti"};
+    break;
+  case Lang_OpenCL:
+  case Lang_OBJCXX:
+    llvm_unreachable("Not implemented yet!");
+  }
+  return BasicArgs;
+}
+
+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};
+}
+
+} // end namespace ast_matchers
+} // end namespace clang

unittests/AST/MatchVerifier.h

 
 #ifndef LLVM_CLANG_UNITTESTS_AST_MATCHVERIFIER_H
 #define LLVM_CLANG_UNITTESTS_AST_MATCHVERIFIER_H
 
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+#include "Language.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace ast_matchers {
 
-enum Language { 
-    Lang_C,
-    Lang_C89,
-    Lang_CXX,
-    Lang_CXX11,
-    Lang_OpenCL,
-    Lang_OBJCXX
-};
-
 /// \brief Base class for verifying some property of nodes found by a matcher.
 template <typename NodeType>
 class MatchVerifier : public MatchFinder::MatchCallback {
 public:
   template <typename MatcherType>
   testing::AssertionResult match(const std::string &Code,
                                  const MatcherType &AMatcher) {
     std::vector<std::string> Args;
testing::AssertionResult MatchVerifier<NodeType>::match(
   case Lang_CXX:
     Args.push_back("-std=c++98");
     FileName = "input.cc";
     break;
   case Lang_CXX11:
     Args.push_back("-std=c++11");
     FileName = "input.cc";
     break;
+  case Lang_CXX14:
+    Args.push_back("-std=c++14");
+    FileName = "input.cc";
+    break;
   case Lang_OpenCL:
     FileName = "input.cl";
     break;
   case Lang_OBJCXX:
     FileName = "input.mm";
     break;
   }
 

unittests/AST/StructuralEquivalenceTest.cpp

+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Tooling/Tooling.h"
+
+#include "Language.h"
+#include "DeclMatcher.h"
+
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ast_matchers {
+
+using std::get;
+
+struct StructuralEquivalenceTest : ::testing::Test {
+  std::unique_ptr<ASTUnit> AST0, AST1;
+  std::string Code0, Code1; // Buffers for SourceManager
+
+  // Get a pair of Decl pointers to the synthetised declarations from the given
+  // code snipets. By default we search for the unique Decl with name 'foo' in
+  // both snippets.
+  std::tuple<NamedDecl *, NamedDecl *>
+  makeNamedDecls(const std::string &SrcCode0, const std::string &SrcCode1,
+                 Language Lang, const char *const Identifier = "foo") {
+
+    this->Code0 = SrcCode0;
+    this->Code1 = SrcCode1;
+    ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+
+    const char *const InputFileName = "input.cc";
+
+    AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
+    AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
+
+    ASTContext &Ctx0 = AST0->getASTContext(), &Ctx1 = AST1->getASTContext();
+
+    auto getDecl = [](ASTContext &Ctx, const std::string &Name) -> NamedDecl * {
+      IdentifierInfo *SearchedII = &Ctx.Idents.get(Name);
+      assert(SearchedII && "Declaration with the identifier "
+                           "should be specified in test!");
+      DeclarationName SearchDeclName(SearchedII);
+      SmallVector<NamedDecl *, 4> FoundDecls;
+      Ctx.getTranslationUnitDecl()->localUncachedLookup(SearchDeclName,
+                                                        FoundDecls);
+
+      // We should find one Decl but one only.
+      assert(FoundDecls.size() == 1);
+
+      return FoundDecls[0];
+    };
+
+    NamedDecl *D0 = getDecl(Ctx0, Identifier);
+    NamedDecl *D1 = getDecl(Ctx1, Identifier);
+    assert(D0);
+    assert(D1);
+    return std::make_tuple(D0, D1);
+  }
+
+  bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) {
+    llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
+    StructuralEquivalenceContext Ctx(D0->getASTContext(), D1->getASTContext(),
+                                     NonEquivalentDecls, false, false);
+    return Ctx.IsStructurallyEquivalent(D0, D1);
+  }
+
+  bool testStructuralMatch(std::tuple<NamedDecl *, NamedDecl *> t) {
+    return testStructuralMatch(get<0>(t), get<1>(t));
+  }
+};
+
+TEST_F(StructuralEquivalenceTest, Int) {
+  auto Decls = makeNamedDecls("int foo;", "int foo;", Lang_CXX);
+  EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, IntVsSignedInt) {
+  auto Decls = makeNamedDecls("int foo;", "signed int foo;", Lang_CXX);
+  EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, Char) {
+  auto Decls = makeNamedDecls("char foo;", "char foo;", Lang_CXX);
+  EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+// This test is disabled for now.
+// FIXME Whether this is equivalent is dependendant on the target.
+TEST_F(StructuralEquivalenceTest, DISABLED_CharVsSignedChar) {
+  auto Decls = makeNamedDecls("char foo;", "signed char foo;", Lang_CXX);
+  EXPECT_FALSE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, ForwardRecordDecl) {
+  auto Decls = makeNamedDecls("struct foo;", "struct foo;", Lang_CXX);
+  EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, IntVsSignedIntInStruct) {
+  auto Decls = makeNamedDecls("struct foo { int x; };",
+                              "struct foo { signed int x; };", Lang_CXX);
+  EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, CharVsSignedCharInStruct) {
+  auto Decls = makeNamedDecls("struct foo { char x; };",
+                              "struct foo { signed char x; };", Lang_CXX);
+  EXPECT_FALSE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, IntVsSignedIntTemplateSpec) {
+  auto Decls = makeNamedDecls(
+      "template <class T> struct foo; template<> struct foo<int>{};",
+      "template <class T> struct foo; template<> struct foo<signed int>{};",
+      Lang_CXX);
+  ClassTemplateSpecializationDecl *Spec0 =
+      *cast<ClassTemplateDecl>(get<0>(Decls))->spec_begin();
+  ClassTemplateSpecializationDecl *Spec1 =
+      *cast<ClassTemplateDecl>(get<1>(Decls))->spec_begin();
+  ASSERT_TRUE(Spec0 != nullptr);
+  ASSERT_TRUE(Spec1 != nullptr);
+  EXPECT_TRUE(testStructuralMatch(Spec0, Spec1));
+}
+
+TEST_F(StructuralEquivalenceTest, CharVsSignedCharTemplateSpec) {
+  auto Decls = makeNamedDecls(
+      "template <class T> struct foo; template<> struct foo<char>{};",
+      "template <class T> struct foo; template<> struct foo<signed char>{};",
+      Lang_CXX);
+  ClassTemplateSpecializationDecl *Spec0 =
+      *cast<ClassTemplateDecl>(get<0>(Decls))->spec_begin();
+  ClassTemplateSpecializationDecl *Spec1 =
+      *cast<ClassTemplateDecl>(get<1>(Decls))->spec_begin();
+  ASSERT_TRUE(Spec0 != nullptr);
+  ASSERT_TRUE(Spec1 != nullptr);
+  EXPECT_FALSE(testStructuralMatch(Spec0, Spec1));
+}
+
+TEST_F(StructuralEquivalenceTest, CharVsSignedCharTemplateSpecWithInheritance) {
+  auto Decls = makeNamedDecls(
+      R"(
+      struct true_type{};
+      template <class T> struct foo;
+      template<> struct foo<char> : true_type {};
+      )",
+      R"(
+      struct true_type{};
+      template <class T> struct foo;
+      template<> struct foo<signed char> : true_type {};
+      )",
+      Lang_CXX);
+  ClassTemplateSpecializationDecl *Spec0 =
+      *cast<ClassTemplateDecl>(get<0>(Decls))->spec_begin();
+  ClassTemplateSpecializationDecl *Spec1 =
+      *cast<ClassTemplateDecl>(get<1>(Decls))->spec_begin();
+  ASSERT_TRUE(Spec0 != nullptr);
+  ASSERT_TRUE(Spec1 != nullptr);
+  EXPECT_FALSE(testStructuralMatch(Spec0, Spec1));
+}
+
+// This test is disabled for now.
+// FIXME Enable it, once the check is implemented.
+TEST_F(StructuralEquivalenceTest, DISABLED_WrongOrderInNamespace) {
+  auto Code =
+      R"(
+      namespace NS {
+      template <class T> class Base {
+          int a;
+      };
+      class Derived : Base<Derived> {
+      };
+      }
+      void foo(NS::Derived &);
+      )";
+  auto Decls = makeNamedDecls(Code, Code, Lang_CXX);
+
+  NamespaceDecl *NS =
+      LastDeclMatcher<NamespaceDecl>().match(get<1>(Decls), namespaceDecl());
+  ClassTemplateDecl *TD = LastDeclMatcher<ClassTemplateDecl>().match(
+      get<1>(Decls), classTemplateDecl(hasName("Base")));
+
+  // Reorder the decls, move the TD to the last place in the DC.
+  NS->removeDecl(TD);
+  NS->addDeclInternal(TD);
+
+  EXPECT_FALSE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, WrongOrderOfFieldsInClass) {
+  auto Code = "class X { int a; int b; };";
+  auto Decls = makeNamedDecls(Code, Code, Lang_CXX, "X");
+
+  CXXRecordDecl *RD = FirstDeclMatcher<CXXRecordDecl>().match(
+      get<1>(Decls), cxxRecordDecl(hasName("X")));
+  FieldDecl *FD =
+      FirstDeclMatcher<FieldDecl>().match(get<1>(Decls), fieldDecl(hasName("a")));
+
+  // Reorder the FieldDecls
+  RD->removeDecl(FD);
+  RD->addDeclInternal(FD);
+
+  EXPECT_FALSE(testStructuralMatch(Decls));
+}
+
+} // end namespace ast_matchers
+} // end namespace clang