[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 6 13:37:56 PDT 2020


njames93 added a comment.

As no FixIts are made the `IncludeInserter` can be removed.

I'm struggling to see the value of this check though. If it was reworked to check for loop in the middle of a function it would have a lot more value.

  bool all_of = true;
  for (auto X : V) {
    if (!X) {
      all_of = false;
      break;
    }
  }

Being able to identify those and possibly even transform them would have much more value

  bool all_of = std::all_of(std::begin(V), std::end(V), [](const auto &X) { return static_cast<bool>(X) });

I'm guessing you'd want to check for compound statements that have a `bool` `VarDecl` with an init just before the range for loop and a condition in the loop that flips the value and then breaks after.



================
Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:21-41
+namespace ast_matchers {
+/// 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, internal::Matcher<Stmt>, InnerMatcher) {
+  const auto &Parents = Finder->getASTContext().getParents(Node);
+  if (Parents.size() != 1)
----------------
This should be in an anonymous namespace as it doesn't need external linkage. Probably shouldn't be in the `ast_matchers` namespace either


================
Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:34
+
+  const auto *I = std::find(C->body_begin(), C->body_end(), &Node);
+  assert(I != C->body_end()); // C is parent of Node.
----------------
nit: `const auto *I = llvm::find(C->body(), &Node);`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77572/new/

https://reviews.llvm.org/D77572





More information about the cfe-commits mailing list