[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

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

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 @@ public:
         "readability-uniqueptr-delete-release");
     CheckFactories.registerCheck<UppercaseLiteralSuffixCheck>(
         "readability-uppercase-literal-suffix");
+    CheckFactories.registerCheck<UseAnyOfAllOfCheck>(
+        "readability-use-anyofallof");
   }
 };
 

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

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

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
 ^^^^^^^^^^^^^^^^^
 

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>`_,
 
 

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; });
+  }

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;
+}

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;
+}