[clang-tools-extra] r340411 - [clang-tidy] Abseil: faster strsplit delimiter check
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 22 06:58:25 PDT 2018
Author: hokein
Date: Wed Aug 22 06:58:25 2018
New Revision: 340411
URL: http://llvm.org/viewvc/llvm-project?rev=340411&view=rev
Log:
[clang-tidy] Abseil: faster strsplit delimiter check
This check is an abseil specific check that checks for code using single character string literals as delimiters and transforms the code into characters.
The check was developed internally and has been running at google, this is just
a move to open source the check. It was originally written by @sbenza.
Patch by Deanna Garcia!
Added:
clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=340411&r1=340410&r2=340411&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Wed Aug 22 06:58:25 2018
@@ -11,6 +11,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "DurationDivisionCheck.h"
+#include "FasterStrsplitDelimiterCheck.h"
#include "StringFindStartswithCheck.h"
namespace clang {
@@ -22,6 +23,8 @@ public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<DurationDivisionCheck>(
"abseil-duration-division");
+ CheckFactories.registerCheck<FasterStrsplitDelimiterCheck>(
+ "abseil-faster-strsplit-delimiter");
CheckFactories.registerCheck<StringFindStartswithCheck>(
"abseil-string-find-startswith");
}
Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=340411&r1=340410&r2=340411&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Wed Aug 22 06:58:25 2018
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
add_clang_library(clangTidyAbseilModule
AbseilTidyModule.cpp
DurationDivisionCheck.cpp
+ FasterStrsplitDelimiterCheck.cpp
StringFindStartswithCheck.cpp
LINK_LIBS
Added: clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp?rev=340411&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp Wed Aug 22 06:58:25 2018
@@ -0,0 +1,131 @@
+//===--- FasterStrsplitDelimiterCheck.cpp - clang-tidy---------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "FasterStrsplitDelimiterCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+namespace {
+
+AST_MATCHER(StringLiteral, lengthIsOne) { return Node.getLength() == 1; }
+
+::internal::Matcher<Expr>
+constructExprWithArg(llvm::StringRef ClassName,
+ const ::internal::Matcher<Expr> &Arg) {
+ auto ConstrExpr = cxxConstructExpr(hasType(recordDecl(hasName(ClassName))),
+ hasArgument(0, ignoringParenCasts(Arg)));
+
+ return anyOf(ConstrExpr, cxxBindTemporaryExpr(has(ConstrExpr)));
+}
+
+::internal::Matcher<Expr>
+copyConstructExprWithArg(llvm::StringRef ClassName,
+ const ::internal::Matcher<Expr> &Arg) {
+ return constructExprWithArg(ClassName, constructExprWithArg(ClassName, Arg));
+}
+
+llvm::Optional<std::string> makeCharacterLiteral(const StringLiteral *Literal) {
+ std::string Result;
+ {
+ llvm::raw_string_ostream Stream(Result);
+ Literal->outputString(Stream);
+ }
+
+ // Special case: If the string contains a single quote, we just need to return
+ // a character of the single quote. This is a special case because we need to
+ // escape it in the character literal.
+ if (Result == R"("'")")
+ return std::string(R"('\'')");
+
+ assert(Result.size() == 3 || (Result.size() == 4 && Result.substr(0, 2) == "\"\\"));
+
+ // Now replace the " with '.
+ auto Pos = Result.find_first_of('"');
+ if (Pos == Result.npos)
+ return llvm::None;
+ Result[Pos] = '\'';
+ Pos = Result.find_last_of('"');
+ if (Pos == Result.npos)
+ return llvm::None;
+ Result[Pos] = '\'';
+ return Result;
+}
+
+} // anonymous namespace
+
+void FasterStrsplitDelimiterCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ // Binds to one character string literals.
+ const auto SingleChar =
+ expr(ignoringParenCasts(stringLiteral(lengthIsOne()).bind("Literal")));
+
+ // Binds to a string_view (either absl or std) that was passed by value and
+ // contructed from string literal.
+ auto StringViewArg =
+ copyConstructExprWithArg("::absl::string_view", SingleChar);
+
+ auto ByAnyCharArg =
+ expr(copyConstructExprWithArg("::absl::ByAnyChar", StringViewArg))
+ .bind("ByAnyChar");
+
+ // Find uses of absl::StrSplit(..., "x") and absl::StrSplit(...,
+ // absl::ByAnyChar("x")) to transform them into absl::StrSplit(..., 'x').
+ Finder->addMatcher(callExpr(callee(functionDecl(hasName("::absl::StrSplit"))),
+ hasArgument(1, anyOf(ByAnyCharArg, SingleChar)),
+ unless(isInTemplateInstantiation()))
+ .bind("StrSplit"),
+ this);
+
+ // Find uses of absl::MaxSplits("x", N) and
+ // absl::MaxSplits(absl::ByAnyChar("x"), N) to transform them into
+ // absl::MaxSplits('x', N).
+ Finder->addMatcher(
+ callExpr(
+ callee(functionDecl(hasName("::absl::MaxSplits"))),
+ hasArgument(0, anyOf(ByAnyCharArg, ignoringParenCasts(SingleChar))),
+ unless(isInTemplateInstantiation())),
+ this);
+}
+
+void FasterStrsplitDelimiterCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Literal = Result.Nodes.getNodeAs<StringLiteral>("Literal");
+
+ if (Literal->getBeginLoc().isMacroID() || Literal->getEndLoc().isMacroID())
+ return;
+
+ llvm::Optional<std::string> Replacement = makeCharacterLiteral(Literal);
+ if (!Replacement)
+ return;
+ SourceRange Range = Literal->getSourceRange();
+
+ if (const auto *ByAnyChar = Result.Nodes.getNodeAs<Expr>("ByAnyChar"))
+ Range = ByAnyChar->getSourceRange();
+
+ diag(
+ Literal->getBeginLoc(),
+ "%select{absl::StrSplit()|absl::MaxSplits()}0 called with a string "
+ "literal "
+ "consisting of a single character; consider using the character overload")
+ << (Result.Nodes.getNodeAs<CallExpr>("StrSplit") ? 0 : 1)
+ << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(Range),
+ *Replacement);
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.h?rev=340411&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/FasterStrsplitDelimiterCheck.h Wed Aug 22 06:58:25 2018
@@ -0,0 +1,36 @@
+//===--- FasterStrsplitDelimiterCheck.h - clang-tidy-------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_FASTERSTRSPLITDELIMITERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_FASTERSTRSPLITDELIMITERCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds instances of absl::StrSplit() or absl::MaxSplits() where the delimiter
+/// is a single character string literal and replaces it with a character.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-faster-strsplit-delimiter.html
+class FasterStrsplitDelimiterCheck : public ClangTidyCheck {
+public:
+ FasterStrsplitDelimiterCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_FASTERSTRSPLITDELIMITERCHECK_H
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=340411&r1=340410&r2=340411&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Aug 22 06:58:25 2018
@@ -64,6 +64,12 @@ Improvements to clang-tidy
floating-point context, and recommends the use of a function that
returns a floating-point value.
+- New :doc:`abseil-faster-strsplit-delimiter
+ <clang-tidy/checks/abseil-faster-strsplit-delimiter>` check.
+
+ Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
+ delimiter is a single character string literal and replaces with a character.
+
- New :doc:`readability-magic-numbers
<clang-tidy/checks/readability-magic-numbers>` check.
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst?rev=340411&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst Wed Aug 22 06:58:25 2018
@@ -0,0 +1,41 @@
+.. title:: clang-tidy - abseil-faster-strsplit-delimiter
+
+abseil-faster-strsplit-delimiter
+================================
+
+Finds instances of ``absl::StrSplit()`` or ``absl::MaxSplits()`` where the
+delimiter is a single character string literal and replaces with a character.
+The check will offer a suggestion to change the string literal into a character.
+It will also catch code using ``absl::ByAnyChar()`` for just a single character
+and will transform that into a single character as well.
+
+These changes will give the same result, but using characters rather than
+single character string literals is more efficient and readable.
+
+Examples:
+
+.. code-block:: c++
+
+ // Original - the argument is a string literal.
+ for (auto piece : absl::StrSplit(str, "B")) {
+
+ // Suggested - the argument is a character, which causes the more efficient
+ // overload of absl::StrSplit() to be used.
+ for (auto piece : absl::StrSplit(str, 'B')) {
+
+
+ // Original - the argument is a string literal inside absl::ByAnyChar call.
+ for (auto piece : absl::StrSplit(str, absl::ByAnyChar("B"))) {
+
+ // Suggested - the argument is a character, which causes the more efficient
+ // overload of absl::StrSplit() to be used and we do not need absl::ByAnyChar
+ // anymore.
+ for (auto piece : absl::StrSplit(str, 'B')) {
+
+
+ // Original - the argument is a string literal inside absl::MaxSplits call.
+ for (auto piece : absl::StrSplit(str, absl::MaxSplits("B", 1))) {
+
+ // Suggested - the argument is a character, which causes the more efficient
+ // overload of absl::StrSplit() to be used.
+ for (auto piece : absl::StrSplit(str, absl::MaxSplits('B', 1))) {
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=340411&r1=340410&r2=340411&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Aug 22 06:58:25 2018
@@ -5,6 +5,7 @@ Clang-Tidy Checks
.. toctree::
abseil-duration-division
+ abseil-faster-strsplit-delimiter
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Added: clang-tools-extra/trunk/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp?rev=340411&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp Wed Aug 22 06:58:25 2018
@@ -0,0 +1,99 @@
+// RUN: %check_clang_tidy %s abseil-faster-strsplit-delimiter %t
+
+namespace absl {
+
+class string_view {
+ public:
+ string_view();
+ string_view(const char *);
+};
+
+namespace strings_internal {
+struct Splitter {};
+struct MaxSplitsImpl {
+ MaxSplitsImpl();
+ ~MaxSplitsImpl();
+};
+} //namespace strings_internal
+
+template <typename Delim>
+strings_internal::Splitter StrSplit(absl::string_view, Delim) {
+ return {};
+}
+template <typename Delim, typename Pred>
+strings_internal::Splitter StrSplit(absl::string_view, Delim, Pred) {
+ return {};
+}
+
+class ByAnyChar {
+ public:
+ explicit ByAnyChar(absl::string_view);
+ ~ByAnyChar();
+};
+
+template <typename Delim>
+strings_internal::MaxSplitsImpl MaxSplits(Delim, int) {
+ return {};
+}
+
+} //namespace absl
+
+void SplitDelimiters() {
+ absl::StrSplit("ABC", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a string literal consisting of a single character; consider using the character overload [abseil-faster-strsplit-delimiter]
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A');
+
+ absl::StrSplit("ABC", absl::ByAnyChar("\n"));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\n');
+
+ // Works with predicate
+ absl::StrSplit("ABC", "A", [](absl::string_view) { return true; });
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", 'A', [](absl::string_view) { return true; });
+
+ // Doesn't do anything with other strings lenghts.
+ absl::StrSplit("ABC", "AB");
+ absl::StrSplit("ABC", absl::ByAnyChar(""));
+ absl::StrSplit("ABC", absl::ByAnyChar(" \t"));
+
+ // Escapes a single quote in the resulting character literal.
+ absl::StrSplit("ABC", "'");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\'');
+
+ absl::StrSplit("ABC", "\"");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("ABC", '\"');
+
+ absl::StrSplit("ABC", absl::MaxSplits("\t", 1));
+ // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::MaxSplits()
+ // CHECK-FIXES: absl::StrSplit("ABC", absl::MaxSplits('\t', 1));
+
+ auto delim = absl::MaxSplits(absl::ByAnyChar(" "), 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:48: warning: absl::MaxSplits()
+ // CHECK-FIXES: auto delim = absl::MaxSplits(' ', 1);
+}
+
+#define MACRO(str) absl::StrSplit("ABC", str)
+
+void Macro() {
+ MACRO("A");
+}
+
+template <typename T>
+void FunctionTemplate() {
+ // This one should not warn because ByAnyChar is a dependent type.
+ absl::StrSplit("TTT", T("A"));
+
+ // This one will warn, but we are checking that we get a correct warning only
+ // once.
+ absl::StrSplit("TTT", "A");
+ // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit()
+ // CHECK-FIXES: absl::StrSplit("TTT", 'A');
+}
+
+void FunctionTemplateCaller() {
+ FunctionTemplate<absl::ByAnyChar>();
+ FunctionTemplate<absl::string_view>();
+}
More information about the cfe-commits
mailing list