[clang-tools-extra] add51e1 - [clang-tidy] add new check readability-use-anyofallof
Matthias Gehre via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 3 03:20:26 PDT 2020
Author: Matthias Gehre
Date: 2020-06-03T12:19:06+02:00
New Revision: add51e152aa6dc3aa7a51901a099b2ebe8cfe377
URL: https://github.com/llvm/llvm-project/commit/add51e152aa6dc3aa7a51901a099b2ebe8cfe377
DIFF: https://github.com/llvm/llvm-project/commit/add51e152aa6dc3aa7a51901a099b2ebe8cfe377.diff
LOG: [clang-tidy] add new check readability-use-anyofallof
Summary:
Finds range-based for loops that can be replaced by a call to ``std::any_of`` or
``std::all_of``. In C++ 20 mode, suggests ``std::ranges::any_of`` or
``std::ranges::all_of``.
For now, no fixits are produced.
Reviewers: aaron.ballman, alexfh, hokein
Subscribers: mgorny, xazax.hun, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77572
Added:
clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability-use-anyofallof.rst
clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
Modified:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index 5f674beaa8be..02003a7537f0 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -42,8 +42,10 @@ add_clang_library(clangTidyReadabilityModule
StringCompareCheck.cpp
UniqueptrDeleteReleaseCheck.cpp
UppercaseLiteralSuffixCheck.cpp
+ UseAnyOfAllOfCheck.cpp
LINK_LIBS
+ clangAnalysis
clangAST
clangASTMatchers
clangBasic
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 5ece15ed2912..5ff5e2022839 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -45,6 +45,7 @@
#include "StringCompareCheck.h"
#include "UniqueptrDeleteReleaseCheck.h"
#include "UppercaseLiteralSuffixCheck.h"
+#include "UseAnyOfAllOfCheck.h"
namespace clang {
namespace tidy {
@@ -125,6 +126,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-uniqueptr-delete-release");
CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>(
"readability-uppercase-literal-suffix");
+ CheckFactories.registerCheck<UseAnyOfAllOfCheck>(
+ "readability-use-anyofallof");
}
};
diff --git a/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp b/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
new file mode 100644
index 000000000000..d3c002f5ad1d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
@@ -0,0 +1,109 @@
+//===--- UseAnyOfAllOfCheck.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 "UseAnyOfAllOfCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace {
+/// Matches a Stmt whose parent is a CompoundStmt, and which is directly
+/// followed by a Stmt matching the inner matcher.
+AST_MATCHER_P(Stmt, nextStmt, ast_matchers::internal::Matcher<Stmt>,
+ InnerMatcher) {
+ DynTypedNodeList Parents = Finder->getASTContext().getParents(Node);
+ if (Parents.size() != 1)
+ return false;
+
+ auto *C = Parents[0].get<CompoundStmt>();
+ if (!C)
+ return false;
+
+ const auto *I = llvm::find(C->body(), &Node);
+ assert(I != C->body_end() && "C is parent of Node");
+ if (++I == C->body_end())
+ return false; // Node is last statement.
+
+ return InnerMatcher.matches(**I, Finder, Builder);
+}
+} // namespace
+
+namespace tidy {
+namespace readability {
+
+void UseAnyOfAllOfCheck::registerMatchers(MatchFinder *Finder) {
+ auto returns = [](bool V) {
+ return returnStmt(hasReturnValue(cxxBoolLiteral(equals(V))));
+ };
+
+ auto returnsButNotTrue =
+ returnStmt(hasReturnValue(unless(cxxBoolLiteral(equals(true)))));
+ auto returnsButNotFalse =
+ returnStmt(hasReturnValue(unless(cxxBoolLiteral(equals(false)))));
+
+ Finder->addMatcher(
+ cxxForRangeStmt(
+ nextStmt(returns(false).bind("final_return")),
+ hasBody(allOf(hasDescendant(returns(true)),
+ unless(anyOf(hasDescendant(breakStmt()),
+ hasDescendant(gotoStmt()),
+ hasDescendant(returnsButNotTrue))))))
+ .bind("any_of_loop"),
+ this);
+
+ Finder->addMatcher(
+ cxxForRangeStmt(
+ nextStmt(returns(true).bind("final_return")),
+ hasBody(allOf(hasDescendant(returns(false)),
+ unless(anyOf(hasDescendant(breakStmt()),
+ hasDescendant(gotoStmt()),
+ hasDescendant(returnsButNotFalse))))))
+ .bind("all_of_loop"),
+ this);
+}
+
+static bool isViableLoop(const CXXForRangeStmt &S, ASTContext &Context) {
+
+ ExprMutationAnalyzer Mutations(*S.getBody(), Context);
+ if (Mutations.isMutated(S.getLoopVariable()))
+ return false;
+ const auto Matches =
+ match(findAll(declRefExpr().bind("decl_ref")), *S.getBody(), Context);
+
+ return llvm::none_of(Matches, [&Mutations](auto &DeclRef) {
+ // TODO: allow modifications of loop-local variables
+ return Mutations.isMutated(
+ DeclRef.template getNodeAs<DeclRefExpr>("decl_ref")->getDecl());
+ });
+}
+
+void UseAnyOfAllOfCheck::check(const MatchFinder::MatchResult &Result) {
+ StringRef Ranges = getLangOpts().CPlusPlus2a ? "::ranges" : "";
+
+ if (const auto *S = Result.Nodes.getNodeAs<CXXForRangeStmt>("any_of_loop")) {
+ if (!isViableLoop(*S, *Result.Context))
+ return;
+
+ diag(S->getForLoc(), "replace loop by 'std%0::any_of()'") << Ranges;
+ } else if (const auto *S =
+ Result.Nodes.getNodeAs<CXXForRangeStmt>("all_of_loop")) {
+ if (!isViableLoop(*S, *Result.Context))
+ return;
+
+ diag(S->getForLoc(), "replace loop by 'std%0::all_of()'") << Ranges;
+ }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h b/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
new file mode 100644
index 000000000000..c44ef2e70ae9
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
@@ -0,0 +1,41 @@
+//===--- UseAnyOfAllOfCheck.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_READABILITY_USEALGORITHMCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEALGORITHMCHECK_H
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds ranged-based for loops that can be replaced by a call to std::any_of
+/// or std::all_of.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-use-anyofallof.html
+class UseAnyOfAllOfCheck : public ClangTidyCheck {
+public:
+ using ClangTidyCheck::ClangTidyCheck;
+
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus11;
+ }
+
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_USEALGORITHMCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e6583c17978b..8da24a93d7f4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -152,6 +152,12 @@ New checks
Finds calls to ``NSInvocation`` methods under ARC that don't have proper
argument object lifetimes.
+- New :doc:`readability-use-anyofallof
+ <clang-tidy/checks/readability-use-anyofallof>` check.
+
+ Finds range-based for loops that can be replaced by a call to ``std::any_of``
+ or ``std::all_of``.
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 6d5f8fcbb05a..3794aa5bc3d8 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -299,6 +299,7 @@ Clang-Tidy Checks
`readability-string-compare <readability-string-compare.html>`_, "Yes"
`readability-uniqueptr-delete-release <readability-uniqueptr-delete-release.html>`_, "Yes"
`readability-uppercase-literal-suffix <readability-uppercase-literal-suffix.html>`_, "Yes"
+ `readability-use-anyofallof`_, "No"
`zircon-temporary-objects <zircon-temporary-objects.html>`_,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-use-anyofallof.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-use-anyofallof.rst
new file mode 100644
index 000000000000..f7bd9ff89345
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability-use-anyofallof.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - readability-use-anyofallof
+
+readability-use-anyofallof
+==========================
+
+Finds range-based for loops that can be replaced by a call to ``std::any_of`` or
+``std::all_of``. In C++ 20 mode, suggests ``std::ranges::any_of`` or
+``std::ranges::all_of``.
+
+Example:
+
+.. code-block:: c++
+
+ bool all_even(std::vector<int> V) {
+ for (int I : V) {
+ if (I % 2)
+ return false;
+ }
+ return true;
+ // Replace loop by
+ // return std::ranges::all_of(V, [](int I) { return I % 2 == 0; });
+ }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
new file mode 100644
index 000000000000..fdf98fddfb78
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy -std=c++2a-or-later %s readability-use-anyofallof %t
+
+bool good_any_of() {
+ int v[] = {1, 2, 3};
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::ranges::any_of()'
+ for (int i : v)
+ if (i)
+ return true;
+ return false;
+}
+
+bool good_all_of() {
+ int v[] = {1, 2, 3};
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::ranges::all_of()'
+ for (int i : v)
+ if (i)
+ return false;
+ return true;
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
new file mode 100644
index 000000000000..5b19dd0b8db6
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
@@ -0,0 +1,183 @@
+// RUN: %check_clang_tidy -std=c++14,c++17 %s readability-use-anyofallof %t
+
+bool good_any_of() {
+ int v[] = {1, 2, 3};
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::any_of()' [readability-use-anyofallof]
+ for (int i : v)
+ if (i)
+ return true;
+ return false;
+}
+
+bool cond(int i);
+
+bool good_any_of2() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: replace loop by 'std::any_of()'
+ int k = i / 2;
+ if (cond(k))
+ return true;
+ }
+ return false;
+}
+
+bool good_any_of3() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: replace loop by 'std::any_of()'
+ if (i == 3)
+ continue;
+ if (i)
+ return true;
+ }
+
+ return false;
+}
+
+bool good_any_of_use_external(int comp) {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: replace loop by 'std::any_of()'
+ if (i == comp)
+ return true;
+ }
+
+ return false;
+}
+
+bool good_any_of_no_cond() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: replace loop by 'std::any_of()'
+ return true; // Not a real loop, but technically can become any_of.
+ }
+
+ return false;
+}
+
+bool good_any_of_local_modification() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ int j = i;
+ j++; // FIXME: Any non-const use disables check.
+ if (j > 3)
+ return true;
+ }
+
+ return false;
+}
+
+bool good_any_of_throw() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: replace loop by 'std::any_of()'
+ if (i > 3)
+ return true;
+ if (i == 42)
+ throw 0;
+ }
+
+ return false;
+}
+
+bool bad_any_of1() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ if (i)
+ return false; // bad constant
+ }
+ return false;
+}
+
+bool bad_any_of2() {
+ int v[] = {1, 2, 3};
+ for (int i : v)
+ if (i)
+ return true;
+
+ return true; // bad return
+}
+
+bool bad_any_of3() {
+ int v[] = {1, 2, 3};
+ for (int i : v)
+ if (i)
+ return true;
+ else
+ return i / 2; // bad return
+
+ return false;
+}
+
+bool bad_any_of_control_flow1() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ break; // bad control flow
+ if (i)
+ return true;
+ }
+
+ return false;
+}
+
+bool bad_any_of_control_flow2() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ goto end; // bad control flow
+ if (i)
+ return true;
+ }
+
+ end:
+ return false;
+}
+
+bool bad_any_of4() {
+ return false; // wrong order
+
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ if (i)
+ return true;
+ }
+}
+
+bool bad_any_of5() {
+ int v[] = {1, 2, 3};
+ int j = 0;
+ for (int i : v) {
+ j++; // modifications
+ if (i)
+ return true;
+ }
+ return false;
+}
+
+bool bad_any_of6() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ if (i)
+ return true;
+ }
+ int j = 0; // Statements between loop and return
+ j++;
+ return false;
+}
+
+bool bad_any_of7() {
+ int v[] = {1, 2, 3};
+ for (int i : v) {
+ i; // No 'return true' in body.
+ }
+ return false;
+}
+
+bool good_all_of() {
+ int v[] = {1, 2, 3};
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: replace loop by 'std::all_of()' [readability-use-anyofallof]
+ for (int i : v)
+ if (i)
+ return false;
+ return true;
+}
More information about the cfe-commits
mailing list