kadircet created this revision.
kadircet added reviewers: hokein, sammccall.
Herald added a subscriber: mgrang.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Address comments around verbosity of tests.

Depends on D135859 <https://reviews.llvm.org/D135859>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137252

Files:
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -6,19 +6,25 @@
 //
 //===----------------------------------------------------------------------===//
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Types.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/TextDiagnostic.h"
 #include "clang/Testing/TestAST.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cstddef>
+#include <map>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -40,23 +46,16 @@
   llvm_unreachable("Unexpected RefType");
 }
 
-// Specifies a test of which symbols are referenced by a piece of code.
-// If `// c++-header` is present, treats referencing code as a header file.
-// Target should contain points annotated with the reference kind.
-// Example:
-//   Target:      int $explicit^foo();
-//   Referencing: int x = ^foo();
-// There must be exactly one referencing location marked.
-void testWalk(llvm::StringRef TargetCode, llvm::StringRef ReferencingCode) {
-  llvm::Annotations Target(TargetCode);
-  llvm::Annotations Referencing(ReferencingCode);
-
-  TestInputs Inputs(Referencing.code());
-  Inputs.ExtraFiles["target.h"] = Target.code().str();
+// Finds all the references to Target from Referencing.point().
+std::vector<std::pair<size_t, RefType>>
+referencedPoints(llvm::StringRef TargetCode, llvm::StringRef ReferencingCode,
+                 size_t ReferencingOffset) {
+  TestInputs Inputs(ReferencingCode);
+  Inputs.ExtraFiles["target.h"] = TargetCode.str();
   Inputs.ExtraArgs.push_back("-include");
   Inputs.ExtraArgs.push_back("target.h");
   Inputs.ExtraArgs.push_back("-std=c++17");
-  if (Referencing.code().contains("// c++-header\n"))
+  if (ReferencingCode.contains("// c++-header\n"))
     Inputs.ExtraArgs.push_back("-xc++-header");
   TestAST AST(Inputs);
   const auto &SM = AST.sourceManager();
@@ -65,12 +64,12 @@
   // to the target file.
   FileID ReferencingFile = SM.getMainFileID();
   SourceLocation ReferencingLoc =
-      SM.getComposedLoc(ReferencingFile, Referencing.point());
+      SM.getComposedLoc(ReferencingFile, ReferencingOffset);
   FileID TargetFile = SM.translateFile(
       llvm::cantFail(AST.fileManager().getFileRef("target.h")));
 
   // Perform the walk, and capture the offsets of the referenced targets.
-  std::unordered_map<RefType, std::vector<size_t>> ReferencedOffsets;
+  std::map<size_t, RefType> ActualPoints;
   for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) {
     if (ReferencingFile != SM.getDecomposedExpansionLoc(D->getLocation()).first)
       continue;
@@ -80,12 +79,24 @@
       auto NDLoc = SM.getDecomposedLoc(SM.getFileLoc(ND.getLocation()));
       if (NDLoc.first != TargetFile)
         return;
-      ReferencedOffsets[RT].push_back(NDLoc.second);
+      ActualPoints[NDLoc.second] = RT;
     });
   }
-  for (auto &Entry : ReferencedOffsets)
-    llvm::sort(Entry.second);
+  return {ActualPoints.begin(), ActualPoints.end()};
+}
 
+// Runs on the output of referencedPoints. Compares it to offsets n TargetCode
+// and reftype sequence in Types.
+MATCHER_P2(TargetsWithTypes, /*llvm::Annotations*/ Target,
+           /*std::vector<RefType>*/ Types, "") {
+  auto TargetPoints = Target.points();
+  if (TargetPoints.size() != Types.size()) {
+    *result_listener << "Mismatched annotations(" << TargetPoints.size()
+                     << ") vs types(" << Types.size() << ")";
+    return false;
+  }
+  auto ExpectedPoints = llvm::zip(TargetPoints, Types);
+  std::vector<std::pair<size_t, RefType>> ActualPoints = arg;
   // Compare results to the expected points.
   // For each difference, show the target point in context, like a diagnostic.
   std::string DiagBuf;
@@ -93,127 +104,198 @@
   auto *DiagOpts = new DiagnosticOptions();
   DiagOpts->ShowLevel = 0;
   DiagOpts->ShowNoteIncludeStack = 0;
-  TextDiagnostic Diag(DiagOS, AST.context().getLangOpts(), DiagOpts);
-  auto DiagnosePoint = [&](llvm::StringRef Message, unsigned Offset) {
+  TextDiagnostic Diag(DiagOS, LangOptions(), DiagOpts);
+  SourceManagerForFile SM("target-code.h", Target.code());
+  auto DiagnosePoint = [&](llvm::Twine Message, unsigned Offset) {
     Diag.emitDiagnostic(
-        FullSourceLoc(SM.getComposedLoc(TargetFile, Offset), SM),
-        DiagnosticsEngine::Note, Message, {}, {});
+        FullSourceLoc(SM.get().getComposedLoc(SM.get().getMainFileID(), Offset),
+                      SM.get()),
+        DiagnosticsEngine::Note, Message.str(), {}, {});
   };
-  for (auto RT : {RefType::Explicit, RefType::Implicit, RefType::Ambiguous}) {
-    auto RTStr = to_string(RT);
-    for (auto Expected : Target.points(RTStr))
-      if (!llvm::is_contained(ReferencedOffsets[RT], Expected))
-        DiagnosePoint(("location not marked used with type " + RTStr).str(),
-                      Expected);
-    for (auto Actual : ReferencedOffsets[RT])
-      if (!llvm::is_contained(Target.points(RTStr), Actual))
-        DiagnosePoint(("location unexpectedly used with type " + RTStr).str(),
-                      Actual);
+  auto ExpectedIt = ExpectedPoints.begin();
+  auto ActualIt = ActualPoints.begin();
+  while (ExpectedIt != ExpectedPoints.end() && ActualIt != ActualPoints.end()) {
+    auto [ExpectedOff, ExpectedType] = *ExpectedIt;
+    auto [ActualOff, ActualType] = *ActualIt;
+    if (ExpectedOff == ActualOff) {
+      if (ExpectedType != ActualType) {
+        DiagnosePoint("location marked as used with type " +
+                          to_string(ActualType) + " but expected " +
+                          to_string(ExpectedType),
+                      ExpectedOff);
+      }
+      ++ExpectedIt;
+      ++ActualIt;
+    } else if (ExpectedOff < ActualOff) {
+      DiagnosePoint("location not marked used with type " +
+                        to_string(ExpectedType),
+                    ExpectedOff);
+      ++ExpectedIt;
+    } else {
+      DiagnosePoint("location unexpectedly used with type " +
+                        to_string(ActualType),
+                    ActualOff);
+      ++ActualIt;
+    }
+  }
+  while (ExpectedIt != ExpectedPoints.end()) {
+    DiagnosePoint("location not marked used with type " +
+                      to_string(std::get<1>(*ExpectedIt)),
+                  std::get<0>(*ExpectedIt));
+    ++ExpectedIt;
+  }
+  while (ActualIt != ActualPoints.end()) {
+    DiagnosePoint("location unexpectedly used with type " +
+                      to_string(ActualIt->second),
+                  ActualIt->first);
+    ++ActualIt;
   }
 
-  // If there were any differences, we print the entire referencing code once.
   if (!DiagBuf.empty())
-    ADD_FAILURE() << DiagBuf << "\nfrom code:\n" << ReferencingCode;
+    *result_listener << DiagBuf;
+  return DiagBuf.empty();
 }
 
+// Specifies a test of which symbols are referenced by a piece of code.
+// If `// c++-header` is present, treats referencing code as a header file.
+// Target should contain points that are used by the point in ReferencingCode.
+// Example:
+//   Target:      int ^foo();
+//   Referencing: int x = ^foo();
+// There must be exactly one referencing location marked.
+#define EXPECT_REFERENCES_WITH_TYPES(TargetCode, ReferencingCode, ...)         \
+  {                                                                            \
+    llvm::Annotations Target(TargetCode);                                      \
+    llvm::Annotations Referencing(ReferencingCode);                            \
+    EXPECT_THAT(referencedPoints(Target.code(), Referencing.code(),            \
+                                 Referencing.point()),                         \
+                TargetsWithTypes(Target, std::vector<RefType>({__VA_ARGS__}))) \
+        << "from code:\n"                                                      \
+        << ReferencingCode;                                                    \
+  }
+
+// Convenience macros for matching cases where all references are of the same
+// type.
+#define EXPECT_REFERENCES_WITH_TYPE(TargetCode, ReferencingCode, Type)         \
+  EXPECT_REFERENCES_WITH_TYPES(                                                \
+      TargetCode, ReferencingCode,                                             \
+      std::vector<RefType>(llvm::Annotations(TargetCode).points().size(),      \
+                           Type));
+#define EXPECT_EXPLICIT_REFERENCES(TargetCode, ReferencingCode)                \
+  EXPECT_REFERENCES_WITH_TYPE(TargetCode, ReferencingCode, RefType::Explicit)
+#define EXPECT_IMPLICIT_REFERENCES(TargetCode, ReferencingCode)                \
+  EXPECT_REFERENCES_WITH_TYPE(TargetCode, ReferencingCode, RefType::Implicit)
+#define EXPECT_AMBIGUOUS_REFERENCES(TargetCode, ReferencingCode)               \
+  EXPECT_REFERENCES_WITH_TYPE(TargetCode, ReferencingCode, RefType::Ambiguous)
+
 TEST(WalkAST, DeclRef) {
-  testWalk("int $explicit^x;", "int y = ^x;");
-  testWalk("int $explicit^foo();", "int y = ^foo();");
-  testWalk("namespace ns { int $explicit^x; }", "int y = ns::^x;");
-  testWalk("struct S { static int $explicit^x; };", "int y = S::^x;");
+  EXPECT_EXPLICIT_REFERENCES("int ^x;", "int y = ^x;");
+  EXPECT_EXPLICIT_REFERENCES("int ^x;", "int y = ^x;");
+  EXPECT_EXPLICIT_REFERENCES("int ^foo();", "int y = ^foo();");
+  EXPECT_EXPLICIT_REFERENCES("namespace ns { int ^x; }", "int y = ns::^x;");
+  EXPECT_EXPLICIT_REFERENCES("struct S { static int ^x; };", "int y = S::^x;");
   // Canonical declaration only.
-  testWalk("extern int $explicit^x; int x;", "int y = ^x;");
+  EXPECT_EXPLICIT_REFERENCES("extern int ^x; int x;", "int y = ^x;");
   // Return type of `foo` isn't used.
-  testWalk("struct S{}; S $explicit^foo();", "auto bar() { return ^foo(); }");
+  EXPECT_EXPLICIT_REFERENCES("struct S{}; S ^foo();",
+                             "auto bar() { return ^foo(); }");
 }
 
 TEST(WalkAST, TagType) {
-  testWalk("struct $explicit^S {};", "^S *y;");
-  testWalk("enum $explicit^E {};", "^E *y;");
-  testWalk("struct $explicit^S { static int x; };", "int y = ^S::x;");
+  EXPECT_EXPLICIT_REFERENCES("struct ^S {};", "^S *y;");
+  EXPECT_EXPLICIT_REFERENCES("enum ^E {};", "^E *y;");
+  EXPECT_EXPLICIT_REFERENCES("struct ^S { static int x; };", "int y = ^S::x;");
 }
 
 TEST(WalkAST, Alias) {
-  testWalk(R"cpp(
+  EXPECT_EXPLICIT_REFERENCES(R"cpp(
     namespace ns { int x; }
-    using ns::$explicit^x;
+    using ns::^x;
   )cpp",
-           "int y = ^x;");
-  testWalk("using $explicit^foo = int;", "^foo x;");
-  testWalk("struct S {}; using $explicit^foo = S;", "^foo x;");
+                             "int y = ^x;");
+  EXPECT_EXPLICIT_REFERENCES("using ^foo = int;", "^foo x;");
+  EXPECT_EXPLICIT_REFERENCES("struct S {}; using ^foo = S;", "^foo x;");
 }
 
 TEST(WalkAST, Using) {
   // Make sure we ignore unused overloads.
-  testWalk(R"cpp(
+  EXPECT_EXPLICIT_REFERENCES(R"cpp(
     namespace ns {
-      void $explicit^x(); void x(int); void x(char);
+      void ^x(); void x(int); void x(char);
     })cpp",
-           "using ns::^x; void foo() { x(); }");
+                             "using ns::^x; void foo() { x(); }");
   // We should report unused overloads if main file is a header.
-  testWalk(R"cpp(
+  EXPECT_REFERENCES_WITH_TYPES(
+      R"cpp(
     namespace ns {
-      void $ambiguous^x(); void $ambiguous^x(int); void $ambiguous^x(char);
+      void ^x(); void ^x(int); void ^x(char);
     })cpp",
-           "// c++-header\n using ns::^x;");
-  testWalk("namespace ns { struct S; } using ns::$explicit^S;", "^S *s;");
+      "// c++-header\n using ns::^x; void foo() { x(3); }", RefType::Ambiguous,
+      RefType::Explicit, RefType::Ambiguous);
+  EXPECT_EXPLICIT_REFERENCES("namespace ns { struct S; } using ns::^S;",
+                             "^S *s;");
 }
 
 TEST(WalkAST, Namespaces) {
-  testWalk("namespace ns { void x(); }", "using namespace ^ns;");
+  EXPECT_EXPLICIT_REFERENCES("namespace ns { void x(); }",
+                             "using namespace ^ns;");
 }
 
 TEST(WalkAST, TemplateNames) {
-  testWalk("template<typename> struct $explicit^S {};", "^S<int> s;");
+  EXPECT_EXPLICIT_REFERENCES("template<typename> struct ^S {};", "^S<int> s;");
   // FIXME: Template decl has the wrong primary location for type-alias template
   // decls.
-  testWalk(R"cpp(
+  EXPECT_EXPLICIT_REFERENCES(R"cpp(
       template <typename> struct S {};
-      template <typename T> $explicit^using foo = S<T>;)cpp",
-           "^foo<int> x;");
-  testWalk(R"cpp(
+      template <typename T> ^using foo = S<T>;)cpp",
+                             "^foo<int> x;");
+  EXPECT_EXPLICIT_REFERENCES(R"cpp(
       namespace ns {template <typename> struct S {}; }
-      using ns::$explicit^S;)cpp",
-           "^S<int> x;");
-  testWalk("template<typename> struct $explicit^S {};",
-           R"cpp(
+      using ns::^S;)cpp",
+                             "^S<int> x;");
+  EXPECT_EXPLICIT_REFERENCES("template<typename> struct ^S {};",
+                             R"cpp(
       template <template <typename> typename> struct X {};
       X<^S> x;)cpp");
-  testWalk("template<typename T> struct $explicit^S { S(T); };", "^S s(42);");
+  EXPECT_EXPLICIT_REFERENCES("template<typename T> struct ^S { S(T); };",
+                             "^S s(42);");
   // Should we mark the specialization instead?
-  testWalk(
-      "template<typename> struct $explicit^S {}; template <> struct S<int> {};",
+  EXPECT_EXPLICIT_REFERENCES(
+      "template<typename> struct ^S {}; template <> struct S<int> {};",
       "^S<int> s;");
 }
 
 TEST(WalkAST, MemberExprs) {
-  testWalk("struct S { void $explicit^foo(); };", "void foo() { S{}.^foo(); }");
-  testWalk(
-      "struct S { void foo(); }; struct X : S { using S::$explicit^foo; };",
+  EXPECT_EXPLICIT_REFERENCES("struct S { void ^foo(); };",
+                             "void foo() { S{}.^foo(); }");
+  EXPECT_EXPLICIT_REFERENCES(
+      "struct S { void foo(); }; struct X : S { using S::^foo; };",
       "void foo() { X{}.^foo(); }");
 }
 
 TEST(WalkAST, ConstructExprs) {
-  testWalk("struct $implicit^S {};", "S ^t;");
-  testWalk("struct S { $implicit^S(); };", "S ^t;");
-  testWalk("struct S { $explicit^S(int); };", "S ^t(42);");
-  testWalk("struct S { $implicit^S(int); };", "S t = ^42;");
+  EXPECT_IMPLICIT_REFERENCES("struct ^S {};", "S ^t;");
+  EXPECT_IMPLICIT_REFERENCES("struct S { ^S(); };", "S ^t;");
+  EXPECT_EXPLICIT_REFERENCES("struct S { ^S(int); };", "S ^t(42);");
+  EXPECT_IMPLICIT_REFERENCES("struct S { ^S(int); };", "S t = ^42;");
 }
 
 TEST(WalkAST, Functions) {
   // Definition uses declaration, not the other way around.
-  testWalk("void $explicit^foo();", "void ^foo() {}");
-  testWalk("void foo() {}", "void ^foo();");
+  EXPECT_EXPLICIT_REFERENCES("void ^foo();", "void ^foo() {}");
+  EXPECT_EXPLICIT_REFERENCES("void foo() {}", "void ^foo();");
 
   // Unresolved calls marks all the overloads.
-  testWalk("void $ambiguous^foo(int); void $ambiguous^foo(char);",
-           "template <typename T> void bar() { ^foo(T{}); }");
+  EXPECT_REFERENCES_WITH_TYPES(
+      "void ^foo(int); void ^foo(char);",
+      "template <typename T> void bar() { ^foo(T{}); }", RefType::Ambiguous,
+      RefType::Ambiguous);
 }
 
 TEST(WalkAST, Enums) {
-  testWalk("enum E { $explicit^A = 42, B = 43 };", "int e = ^A;");
-  testWalk("enum class $explicit^E : int;", "enum class ^E : int {};");
-  testWalk("enum class E : int {};", "enum class ^E : int ;");
+  EXPECT_EXPLICIT_REFERENCES("enum E { ^A = 42, B = 43 };", "int e = ^A;");
+  EXPECT_EXPLICIT_REFERENCES("enum class ^E : int;", "enum class ^E : int {};");
+  EXPECT_EXPLICIT_REFERENCES("enum class E : int {};", "enum class ^E : int ;");
 }
 
 } // namespace
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to