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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Sorry for breaking the build and thanks for the fixes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D77572#2071045 , @RKSimon wrote:

> @mgehre Please can you take at the remaining buildbot failures here : 
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/68662


Pushed a fix for that too. 
https://github.com/llvm/llvm-project/commit/6780be4c63e582466a35d7644c35e09ba85d4f67


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

@mgehre Please can you take at the remaining buildbot failures here : 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/68662


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

This change broke the build: 
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/29716. The 
build is already fixed by 
https://github.com/llvm/llvm-project/commit/fd2740143e626ca32432aac0b51b2880a3b1e0bc.

Please always run `ninja check-all` before pushing commits. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matthias Gehre via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGadd51e152aa6: [clang-tidy] add new check 
readability-use-anyofallof (authored by mgehre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  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

Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
===
--- /dev/null
+++ 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;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
===
--- /dev/null
+++ 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};

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 268120.
mgehre added a comment.

Implemented njames93's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  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

Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
===
--- /dev/null
+++ 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;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
===
--- /dev/null
+++ 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 

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

[Nathan James via Phabricator via cfe-commits]

njames93 accepted this revision.
njames93 added a comment.

LGMT, just a few minor nits though.




Comment at: 
clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:15-16
+#include "clang/Frontend/CompilerInstance.h"
+#include 
+#include 
+

Fairly certain these headers aren't used



Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h:30
+  bool isLanguageVersionSupported(const LangOptions ) const override {
+return LangOpts.CPlusPlus;
+  }

`std::all_of`, `std::any_of` and `std::none_of` were only introduced in c++11, 
maybe `CPlusPlus11` should be the minimum requirement.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:202
 ---
-

This removed line in unrelated and should be added back


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, though please wait a bit for @njames93 to speak up if they still have 
concerns.




Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:61
+  hasBody(allOf(hasDescendant(returns(true)),
+unless(anyOf(hasDescendant(breakStmt()),
+ hasDescendant(returnsButNotTrue))

mgehre wrote:
> aaron.ballman wrote:
> > Should we reject other ways to break out of the loop, like `goto` or 
> > `throw`?
> I think throw statements still can be transformed. We cannot transform 
> `break` because the loop is gone and we cannot transform `goto` because we 
> cannot jump from the lambda into its caller.
> But we can keep `throw` statements because exceptions can propagate from the 
> lambda through the algorithm back into the original caller. If we could not 
> allow `throw` statements, we would also have to disallow any other kind of 
> call statements.
Ah, good point on `throw`, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 266083.
mgehre marked an inline comment as done.
mgehre added a comment.

Fix comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  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

Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
===
--- /dev/null
+++ 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;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
===
--- /dev/null
+++ 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 

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre marked 5 inline comments as done.
mgehre added a comment.

Aaron, thank you very much for taking the time to review this PR.




Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:61
+  hasBody(allOf(hasDescendant(returns(true)),
+unless(anyOf(hasDescendant(breakStmt()),
+ hasDescendant(returnsButNotTrue))

aaron.ballman wrote:
> Should we reject other ways to break out of the loop, like `goto` or `throw`?
I think throw statements still can be transformed. We cannot transform `break` 
because the loop is gone and we cannot transform `goto` because we cannot jump 
from the lambda into its caller.
But we can keep `throw` statements because exceptions can propagate from the 
lambda through the algorithm back into the original caller. If we could not 
allow `throw` statements, we would also have to disallow any other kind of call 
statements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D77572#1981871 , @mgehre wrote:

> Thanks for the comments so far.
>  I'm a bit lost now. Which changes that cannot wait for the next PR do you 
> see necessary to get this check merged?


I'd be curious to know what @njames93 thinks -- I spot-checked the diagnostics 
you attached earlier (thank you for those!) and all of them seemed reasonable 
to me, which suggests there's not an extraordinary amount of false positives. 
The functionality seems useful in its current state, though as you point out, 
there are improvements you want to make in follow-up patches. That seems 
reasonable to me.




Comment at: 
clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:22-24
+/// Matches a Stmt whose parent is a CompoundStmt,
+/// and which is directly followed by
+/// a Stmt matching the inner matcher.

It looks like these comments got formatted a bit strangely -- probably should 
re-flow them.



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

I think the comment should turn into a string literal that's part of the 
assertion.



Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:61
+  hasBody(allOf(hasDescendant(returns(true)),
+unless(anyOf(hasDescendant(breakStmt()),
+ hasDescendant(returnsButNotTrue))

Should we reject other ways to break out of the loop, like `goto` or `throw`?



Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:98
+
+diag(S->getForLoc(), "Replace loop by std%0::any_of() from ")
+<< Ranges;

clang-tidy diagnostics don't start with a capital letter, and I'd probably drop 
the `from ` part under the assumption the user can figure out the 
header pretty easily from the diagnostic wording. Also, you should put single 
quotes around the `std%0::any_of()`. Similar below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Ping :-)
I'm looking for directions on what the next steps are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Thanks for the comments so far.
I'm a bit lost now. Which changes that cannot wait for the next PR do you see 
necessary to get this check merged?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Right now it would be a great candidate for this - 
http://lists.llvm.org/pipermail/cfe-dev/2020-March/064980.html, however in its 
current state I wouldn't say its ready to get the green light right now.
No point worrying about the fix-its yet, but when it is time, could add options 
for `RangeAllOfName`, `RangeAnyOfName`, `RangeNoneOfName`, `AlgorithmHeader`.
Gonna throw it out there that generating that templated lambda automatically... 
just don't :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Thanks @njames93, this is a good examples which I did not consider yet. I agree 
that only transforming
the second loop would make the code worse.

I would argue that a human seeing a warning on the second loop might also 
realize
that the first loop can be transformed in a similar way, and possibly combine 
them into

  return llvm::all_of(getConds(), [](const Init *Cond) { return Val->Cond(); }) 
&& llvm::all_of(getVals(), [](const Init *Val) { return Val->isConcrete(); });

(I was wondering whether the check should have an option to suggest 
`llvm::all_of` (or `boost::algorithm::all_of`, ...) instead of `std::all_of`, 
but
I thought that this could go into another PR.)

I have the feeling that extracting code into algorithms is generally a hard 
topic,
and automatic fixits would possible give a false sense of automatism (at least 
at the current point).
Your example is a good reminder of that.

Personally, clang-tidy has been a good source of learning C++ best practices. 
And I hope that this clang-tidy check would help
me and my coworkers to remember using those algorithms.

Are you saying that this check should not land unless its scope is extended? 
What would be the minimal scope making this check
worth-while to land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D77572#1967956 , @mgehre wrote:

> In D77572#1965143 , @njames93 wrote:
>
> > 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.
>
>
> This is (just) a first step. The next step is to detect the local variable 
> case as you also described it. Note that this also catches functions
>  that do some preprocessing and end with a any_of-style loop.
>  I also have a local branch that generates fixits, but they add quite some 
> code, so I wanted to put them in a separate PR.
>
> In LLVM & clang, the check in this PR already emits 370 unique warnings.


Take this example from TableGen/Record.cpp:

  bool CondOpInit::isConcrete() const {
for (const Init *Case : getConds())
  if (!Case->isConcrete())
return false;
  
for (const Init *Val : getVals())
  if (!Val->isConcrete())
return false;
  
return true;
  }

Firstly, the warning is only emitted on the second loop.
Secondly how does your fix it code handle temporaries?
Would it transform to

  bool CondOpInit::isConcrete() const {
for (const Init *Case : getConds())
  if (!Case->isConcrete())
return false;
  
return std::all_of(getVals().begin(), getVals().end(),
   [](const Init *Val) { return Val->isConcrete(); });
  }

I'd argue that it actually makes the code less readable as there are 2 
different constructs for the same thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 255814.
mgehre added a comment.

Review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  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

Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
@@ -0,0 +1,158 @@
+// 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() from  [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 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_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() from  [readability-use-anyofallof]
+  for (int i : v)
+if (i)
+  return false;
+  return true;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
===
--- /dev/null
+++ 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() from 
+  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() from 
+  for (int i : v)
+if (i)
+  return false;
+  return true;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-use-anyofallof.rst
===
--- /dev/null

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 255812.
mgehre added a comment.

Review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572

Files:
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -132,8 +132,8 @@
 - New :doc:`readability-use-anyofallof
   ` check.
 
-  Finds range-based for loops that can be replaced by a call to std::any_of or
-  std::all_of.
+  Finds range-based for loops that can be replaced by a call to ``std::any_of``
+  or ``std::all_of``.
 
 New check aliases
 ^
Index: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
===
--- clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
+++ clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
@@ -24,19 +24,14 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/readability-use-anyofallof.html
 class UseAnyOfAllOfCheck : public ClangTidyCheck {
 public:
-  UseAnyOfAllOfCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context),
-IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
-Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {}
+  using ClangTidyCheck::ClangTidyCheck;
+
+  bool isLanguageVersionSupported(const LangOptions ) const override {
+return LangOpts.CPlusPlus;
+  }
 
-  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
-   Preprocessor *ModuleExpanderPP) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
-
-private:
-  std::unique_ptr IncludeInserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
@@ -18,12 +18,13 @@
 using namespace clang::ast_matchers;
 
 namespace clang {
-namespace ast_matchers {
+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, internal::Matcher, InnerMatcher) {
-  const auto  = Finder->getASTContext().getParents(Node);
+AST_MATCHER_P(Stmt, nextStmt, ast_matchers::internal::Matcher,
+  InnerMatcher) {
+  DynTypedNodeList Parents = Finder->getASTContext().getParents(Node);
   if (Parents.size() != 1)
 return false;
 
@@ -31,30 +32,19 @@
   if (!C)
 return false;
 
-  const auto *I = std::find(C->body_begin(), C->body_end(), );
+  const auto *I = llvm::find(C->body(), );
   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 ast_matchers
+} // namespace
 
 namespace tidy {
 namespace readability {
 
-void UseAnyOfAllOfCheck::registerPPCallbacks(const SourceManager ,
- Preprocessor *PP,
- Preprocessor *ModuleExpanderPP) {
-  IncludeInserter =
-  std::make_unique(SM, getLangOpts(), IncludeStyle);
-  PP->addPPCallbacks(IncludeInserter->CreatePPCallbacks());
-}
-
 void UseAnyOfAllOfCheck::registerMatchers(MatchFinder *Finder) {
-  if (!getLangOpts().CPlusPlus)
-return;
-
   auto returns = [](bool V) {
 return returnStmt(hasReturnValue(cxxBoolLiteral(equals(V;
   };
@@ -107,7 +97,6 @@
 
 diag(S->getForLoc(), "Replace loop by std%0::any_of() from ")
 << Ranges;
-
   } else if (const auto *S =
  Result.Nodes.getNodeAs("all_of_loop")) {
 if (!isViableLoop(*S, *Result.Context))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre marked 6 inline comments as done.
mgehre added a comment.

In D77572#1965143 , @njames93 wrote:

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


This is (just) a first step. The next step is to detect the local variable case 
as you also described it. Note that this also catches functions
that do some preprocessing and end with a any_of-style loop.
I also have a local branch that generates fixits, but they add quite some code, 
so I wanted to put them in a separate PR.

In LLVM & clang, the check in this PR already emits 370 unique warnings. 
F11685854: readability-use-anyofallof.log 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

See also suggestion  for more generic 
loop-to-algorithm transformations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nathan James via Phabricator via cfe-commits]

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 ) { 
return static_cast(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, InnerMatcher) {
+  const auto  = 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(), );
+  assert(I != C->body_end()); // C is parent of Node.

nit: `const auto *I = llvm::find(C->body(), );`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:26
+AST_MATCHER_P(Stmt, nextStmt, internal::Matcher, InnerMatcher) {
+  const auto  = Finder->getASTContext().getParents(Node);
+  if (Parents.size() != 1)

Please don't use auto unless type is spelled in same statement or iterator.



Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:55
+void UseAnyOfAllOfCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;

Must be in isLanguageVersionSupported() method.



Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp:110
+<< Ranges;
+
+  } else if (const auto *S =

Unnecessary empty line.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:135
+
+  Finds range-based for loops that can be replaced by a call to std::any_of or
+  std::all_of.

Please enclose std::any_of and std::all_of in double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77572



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
mgehre added reviewers: aaron.ballman, alexfh, hokein.
Herald added subscribers: xazax.hun, mgorny.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77572

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  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

Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof.cpp
@@ -0,0 +1,158 @@
+// 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() from  [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 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_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() from  [readability-use-anyofallof]
+  for (int i : v)
+if (i)
+  return false;
+  return true;
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability-use-anyofallof-cpp20.cpp
===
--- /dev/null
+++ 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() from 
+  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() from 
+  for (int i