carlosgalvezp updated this revision to Diff 390708.
carlosgalvezp added a comment.
Switch to public inheritance + mutable cached data. This seems to be a valid
use case <https://arne-mertz.de/2017/10/mutable/> for mutable, even though I'm
not sure if there's anything against it in the LLVM coding guidelines. Looking
forward to your feedback!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113422/new/
https://reviews.llvm.org/D113422
Files:
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
Index: clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
+++ 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 @@
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 @@
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 @@
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 @@
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 @@
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, 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"));
Index: clang-tools-extra/clang-tidy/GlobList.h
===================================================================
--- clang-tools-extra/clang-tidy/GlobList.h
+++ 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 @@
/// 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 @@
/// 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 @@
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
Index: clang-tools-extra/clang-tidy/GlobList.cpp
===================================================================
--- clang-tools-extra/clang-tidy/GlobList.cpp
+++ 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 @@
}
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
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -29,6 +29,7 @@
} // namespace tooling
namespace tidy {
+class CachedGlobList;
/// A detected error complete with information to display diagnostic and
/// automatic fix.
@@ -191,7 +192,7 @@
std::string CurrentFile;
ClangTidyOptions CurrentOptions;
- class CachedGlobList;
+
std::unique_ptr<CachedGlobList> CheckFilter;
std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -155,29 +155,6 @@
: 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)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
