[PATCH] D125109: [clangd] Rewrite TweakTesting helpers to avoid reparsing the same code. NFC
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa316a9815a4f: [clangd] Rewrite TweakTesting helpers to avoid
reparsing the same code. NFC (authored by sammccall).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125109/new/
https://reviews.llvm.org/D125109
Files:
clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
@@ -9,9 +9,11 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
+#include "ParsedAST.h"
#include "index/Index.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Annotations.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include
@@ -89,14 +91,13 @@
std::string apply(llvm::StringRef MarkedCode,
llvm::StringMap *EditedFiles = nullptr) const;
- // Accepts a code snippet with many ranges (or points) marked, and returns a
- // list of snippets with one range marked each.
- // Primarily used from EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macro.
- static std::vector expandCases(llvm::StringRef MarkedCode);
-
- // Returns a matcher that accepts marked code snippets where the tweak is
- // available at the marked range.
- ::testing::Matcher isAvailable() const;
+ // Helpers for EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macros.
+ using WrappedAST = std::pair;
+ WrappedAST build(llvm::StringRef) const;
+ bool isAvailable(WrappedAST &, llvm::Annotations::Range) const;
+ // Return code re-decorated with a single point/range.
+ static std::string decorate(llvm::StringRef, unsigned);
+ static std::string decorate(llvm::StringRef, llvm::Annotations::Range);
};
MATCHER_P2(FileWithContents, FileName, Contents, "") {
@@ -109,18 +110,18 @@
TweakID##Test() : TweakTest(#TweakID) {} \
}
-#define EXPECT_AVAILABLE(MarkedCode) \
- do { \
-for (const auto : expandCases(MarkedCode)) \
- EXPECT_THAT(Case, ::clang::clangd::TweakTest::isAvailable());\
- } while (0)
-
-#define EXPECT_UNAVAILABLE(MarkedCode) \
+#define EXPECT_AVAILABLE_(MarkedCode, Available) \
do { \
-for (const auto : expandCases(MarkedCode)) \
- EXPECT_THAT(Case,\
- ::testing::Not(::clang::clangd::TweakTest::isAvailable())); \
+llvm::Annotations A{llvm::StringRef(MarkedCode)}; \
+auto AST = build(A.code());\
+assert(!A.points().empty() || !A.ranges().empty());\
+for (const auto : A.points()) \
+ EXPECT_EQ(Available, isAvailable(AST, {P, P})) << decorate(A.code(), P); \
+for (const auto : A.ranges()) \
+ EXPECT_EQ(Available, isAvailable(AST, R)) << decorate(A.code(), R); \
} while (0)
+#define EXPECT_AVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, true)
+#define EXPECT_UNAVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, false)
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
@@ -8,7 +8,6 @@
#include "TweakTesting.h"
-#include "Annotations.h"
#include "SourceCode.h"
#include "TestTU.h"
#include "refactor/Tweak.h"
@@ -50,31 +49,25 @@
return Outer;
}
-std::pair rangeOrPoint(const Annotations ) {
- Range SelectionRng;
+llvm::Annotations::Range rangeOrPoint(const llvm::Annotations ) {
if (A.points().size() != 0) {
assert(A.ranges().size() == 0 &&
"both a cursor point and a selection range were specified");
-SelectionRng = Range{A.point(), A.point()};
- } else {
-SelectionRng = A.range();
+return {A.point(), A.point()};
}
- return {cantFail(positionToOffset(A.code(), SelectionRng.start)),
- cantFail(positionToOffset(A.code(), SelectionRng.end))};
+ return A.range();
}
// Prepare and apply the specified tweak based
[PATCH] D125109: [clangd] Rewrite TweakTesting helpers to avoid reparsing the same code. NFC
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125109/new/ https://reviews.llvm.org/D125109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D125109: [clangd] Rewrite TweakTesting helpers to avoid reparsing the same code. NFC
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang-tools-extra.
Previously the EXPECT_AVAILABLE macros would rebuild the code at each marked
point, by expanding the cases textually.
There were often lots, and it's nice to have lots!
This reduces total unittest time by ~10% on my machine.
I did have to sacrifice a little apply() coverage in AddUsingTests (was calling
expandCases directly, which was otherwise unused), but we have
EXPECT_AVAILABLE tests covering that, I don't think there's real risk here.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D125109
Files:
clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
@@ -9,9 +9,11 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
+#include "ParsedAST.h"
#include "index/Index.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Annotations.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include
@@ -89,14 +91,13 @@
std::string apply(llvm::StringRef MarkedCode,
llvm::StringMap *EditedFiles = nullptr) const;
- // Accepts a code snippet with many ranges (or points) marked, and returns a
- // list of snippets with one range marked each.
- // Primarily used from EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macro.
- static std::vector expandCases(llvm::StringRef MarkedCode);
-
- // Returns a matcher that accepts marked code snippets where the tweak is
- // available at the marked range.
- ::testing::Matcher isAvailable() const;
+ // Helpers for EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macros.
+ using WrappedAST = std::pair;
+ WrappedAST build(llvm::StringRef) const;
+ bool isAvailable(WrappedAST &, llvm::Annotations::Range) const;
+ // Return code re-decorated with a single point/range.
+ static std::string decorate(llvm::StringRef, unsigned);
+ static std::string decorate(llvm::StringRef, llvm::Annotations::Range);
};
MATCHER_P2(FileWithContents, FileName, Contents, "") {
@@ -109,18 +110,18 @@
TweakID##Test() : TweakTest(#TweakID) {} \
}
-#define EXPECT_AVAILABLE(MarkedCode) \
- do { \
-for (const auto : expandCases(MarkedCode)) \
- EXPECT_THAT(Case, ::clang::clangd::TweakTest::isAvailable());\
- } while (0)
-
-#define EXPECT_UNAVAILABLE(MarkedCode) \
+#define EXPECT_AVAILABLE_(MarkedCode, Available) \
do { \
-for (const auto : expandCases(MarkedCode)) \
- EXPECT_THAT(Case,\
- ::testing::Not(::clang::clangd::TweakTest::isAvailable())); \
+llvm::Annotations A{llvm::StringRef(MarkedCode)}; \
+auto AST = build(A.code());\
+assert(!A.points().empty() || !A.ranges().empty());\
+for (const auto : A.points()) \
+ EXPECT_EQ(Available, isAvailable(AST, {P, P})) << decorate(A.code(), P); \
+for (const auto : A.ranges()) \
+ EXPECT_EQ(Available, isAvailable(AST, R)) << decorate(A.code(), R); \
} while (0)
+#define EXPECT_AVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, true)
+#define EXPECT_UNAVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, false)
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
@@ -8,7 +8,6 @@
#include "TweakTesting.h"
-#include "Annotations.h"
#include "SourceCode.h"
#include "TestTU.h"
#include "refactor/Tweak.h"
@@ -50,31 +49,25 @@
return Outer;
}
-std::pair rangeOrPoint(const Annotations ) {
- Range SelectionRng;
+llvm::Annotations::Range rangeOrPoint(const llvm::Annotations ) {
if
