[clang-tools-extra] 946eb7a - [clang-tidy][NFC] Move CachedGlobList to GlobList.h
Carlos Galvez via cfe-commits
cfe-commits at lists.llvm.org
Sat Dec 4 00:51:11 PST 2021
Author: Carlos Galvez
Date: 2021-12-04T08:50:49Z
New Revision: 946eb7a037d5f83ea9cdc99bac0f939ddd344e09
URL: https://github.com/llvm/llvm-project/commit/946eb7a037d5f83ea9cdc99bac0f939ddd344e09
DIFF: https://github.com/llvm/llvm-project/commit/946eb7a037d5f83ea9cdc99bac0f939ddd344e09.diff
LOG: [clang-tidy][NFC] Move CachedGlobList to GlobList.h
Currently it's hidden inside ClangTidyDiagnosticConsumer,
so it's hard to know it exists.
Given that there are multiple uses of globs in clang-tidy,
it makes sense to have these classes publicly available
for other use cases that might benefit from it.
Also, add unit test by converting the existing tests
for GlobList into typed tests.
Reviewed By: salman-javed-nz
Differential Revision: https://reviews.llvm.org/D113422
Added:
Modified:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/GlobList.cpp
clang-tools-extra/clang-tidy/GlobList.h
clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 66329328757bd..b09079b5e8ba4 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -155,29 +155,6 @@ ClangTidyError::ClangTidyError(StringRef CheckName,
: tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
IsWarningAsError(IsWarningAsError) {}
-class ClangTidyContext::CachedGlobList {
-public:
- CachedGlobList(StringRef Globs) : Globs(Globs) {}
-
- bool contains(StringRef S) {
- switch (auto &Result = Cache[S]) {
- case Yes:
- return true;
- case No:
- return false;
- case None:
- Result = Globs.contains(S) ? Yes : No;
- return Result == Yes;
- }
- llvm_unreachable("invalid enum");
- }
-
-private:
- GlobList Globs;
- enum Tristate { None, Yes, No };
- llvm::StringMap<Tristate> Cache;
-};
-
ClangTidyContext::ClangTidyContext(
std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
bool AllowEnablingAnalyzerAlphaCheckers)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 129c06f60f569..23800f5622f69 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -29,6 +29,7 @@ class CompilationDatabase;
} // namespace tooling
namespace tidy {
+class CachedGlobList;
/// A detected error complete with information to display diagnostic and
/// automatic fix.
@@ -191,7 +192,7 @@ class ClangTidyContext {
std::string CurrentFile;
ClangTidyOptions CurrentOptions;
- class CachedGlobList;
+
std::unique_ptr<CachedGlobList> CheckFilter;
std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
diff --git a/clang-tools-extra/clang-tidy/GlobList.cpp b/clang-tools-extra/clang-tidy/GlobList.cpp
index ccdf856083759..b594d943cc075 100644
--- a/clang-tools-extra/clang-tidy/GlobList.cpp
+++ b/clang-tools-extra/clang-tidy/GlobList.cpp
@@ -9,8 +9,8 @@
#include "GlobList.h"
#include "llvm/ADT/SmallString.h"
-using namespace clang;
-using namespace tidy;
+namespace clang {
+namespace tidy {
// Returns true if GlobList starts with the negative indicator ('-'), removes it
// from the GlobList.
@@ -62,3 +62,19 @@ bool GlobList::contains(StringRef S) const {
}
return false;
}
+
+bool CachedGlobList::contains(StringRef S) const {
+ switch (auto &Result = Cache[S]) {
+ case Yes:
+ return true;
+ case No:
+ return false;
+ case None:
+ Result = GlobList::contains(S) ? Yes : No;
+ return Result == Yes;
+ }
+ llvm_unreachable("invalid enum");
+}
+
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/GlobList.h b/clang-tools-extra/clang-tidy/GlobList.h
index 17730078b6f31..de7020ef3f165 100644
--- a/clang-tools-extra/clang-tidy/GlobList.h
+++ b/clang-tools-extra/clang-tidy/GlobList.h
@@ -11,6 +11,7 @@
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Regex.h"
@@ -24,6 +25,8 @@ namespace tidy {
/// them in the order of appearance in the list.
class GlobList {
public:
+ virtual ~GlobList() = default;
+
/// \p Globs is a comma-separated list of globs (only the '*' metacharacter is
/// supported) with an optional '-' prefix to denote exclusion.
///
@@ -36,10 +39,9 @@ class GlobList {
/// Returns \c true if the pattern matches \p S. The result is the last
/// matching glob's Positive flag.
- bool contains(StringRef S) const;
+ virtual bool contains(StringRef S) const;
private:
-
struct GlobListItem {
bool IsPositive;
llvm::Regex Regex;
@@ -47,7 +49,21 @@ class GlobList {
SmallVector<GlobListItem, 0> Items;
};
-} // end namespace tidy
-} // end namespace clang
+/// A \p GlobList that caches search results, so that search is performed only
+/// once for the same query.
+class CachedGlobList final : public GlobList {
+public:
+ using GlobList::GlobList;
+
+ /// \see GlobList::contains
+ bool contains(StringRef S) const override;
+
+private:
+ enum Tristate { None, Yes, No };
+ mutable llvm::StringMap<Tristate> Cache;
+};
+
+} // namespace tidy
+} // namespace clang
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GLOBLIST_H
diff --git a/clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp b/clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
index f0c3a702442c9..9460a10683fec 100644
--- a/clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
@@ -4,15 +4,20 @@
namespace clang {
namespace tidy {
-TEST(GlobList, Empty) {
- GlobList Filter("");
+template <typename GlobListT> struct GlobListTest : public ::testing::Test {};
+
+using GlobListTypes = ::testing::Types<GlobList, CachedGlobList>;
+TYPED_TEST_SUITE(GlobListTest, GlobListTypes);
+
+TYPED_TEST(GlobListTest, Empty) {
+ TypeParam Filter("");
EXPECT_TRUE(Filter.contains(""));
EXPECT_FALSE(Filter.contains("aaa"));
}
-TEST(GlobList, Nothing) {
- GlobList Filter("-*");
+TYPED_TEST(GlobListTest, Nothing) {
+ TypeParam Filter("-*");
EXPECT_FALSE(Filter.contains(""));
EXPECT_FALSE(Filter.contains("a"));
@@ -21,8 +26,8 @@ TEST(GlobList, Nothing) {
EXPECT_FALSE(Filter.contains("*"));
}
-TEST(GlobList, Everything) {
- GlobList Filter("*");
+TYPED_TEST(GlobListTest, Everything) {
+ TypeParam Filter("*");
EXPECT_TRUE(Filter.contains(""));
EXPECT_TRUE(Filter.contains("aaaa"));
@@ -31,8 +36,8 @@ TEST(GlobList, Everything) {
EXPECT_TRUE(Filter.contains("*"));
}
-TEST(GlobList, OneSimplePattern) {
- GlobList Filter("aaa");
+TYPED_TEST(GlobListTest, OneSimplePattern) {
+ TypeParam Filter("aaa");
EXPECT_TRUE(Filter.contains("aaa"));
EXPECT_FALSE(Filter.contains(""));
@@ -41,8 +46,8 @@ TEST(GlobList, OneSimplePattern) {
EXPECT_FALSE(Filter.contains("bbb"));
}
-TEST(GlobList, TwoSimplePatterns) {
- GlobList Filter("aaa,bbb");
+TYPED_TEST(GlobListTest, TwoSimplePatterns) {
+ TypeParam Filter("aaa,bbb");
EXPECT_TRUE(Filter.contains("aaa"));
EXPECT_TRUE(Filter.contains("bbb"));
@@ -52,11 +57,11 @@ TEST(GlobList, TwoSimplePatterns) {
EXPECT_FALSE(Filter.contains("bbbb"));
}
-TEST(GlobList, PatternPriority) {
+TYPED_TEST(GlobListTest, PatternPriority) {
// The last glob that matches the string decides whether that string is
// included or excluded.
{
- GlobList Filter("a*,-aaa");
+ TypeParam Filter("a*,-aaa");
EXPECT_FALSE(Filter.contains(""));
EXPECT_TRUE(Filter.contains("a"));
@@ -65,7 +70,7 @@ TEST(GlobList, PatternPriority) {
EXPECT_TRUE(Filter.contains("aaaa"));
}
{
- GlobList Filter("-aaa,a*");
+ TypeParam Filter("-aaa,a*");
EXPECT_FALSE(Filter.contains(""));
EXPECT_TRUE(Filter.contains("a"));
@@ -75,15 +80,16 @@ TEST(GlobList, PatternPriority) {
}
}
-TEST(GlobList, WhitespacesAtBegin) {
- GlobList Filter("-*, a.b.*");
+TYPED_TEST(GlobListTest, WhitespacesAtBegin) {
+ TypeParam Filter("-*, a.b.*");
EXPECT_TRUE(Filter.contains("a.b.c"));
EXPECT_FALSE(Filter.contains("b.c"));
}
-TEST(GlobList, Complex) {
- GlobList Filter("*,-a.*, -b.*, \r \n a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
+TYPED_TEST(GlobListTest, Complex) {
+ TypeParam Filter(
+ "*,-a.*, -b.*, \r \n a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
EXPECT_TRUE(Filter.contains("aaa"));
EXPECT_TRUE(Filter.contains("qqq"));
More information about the cfe-commits
mailing list