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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
