[clang-tidy] Add new check 'llvm-use-ranges'

[vbvictor]

First iteration of the check, mostly reused logic from #97764 without adding any LLVM-specific iterator-methods.
Successfully run on LLVM codebase with ~100 findings and a couple of odd FPs: when we have std::sort(this->begin(), this->end()) or std::sort(begin(), end()).
I didn't fix this cases since it will be a separate task for the core utils::UseRangesCheck.

Fixes #38468.

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Baranov Victor (vbvictor)

Changes

Mostly reused logic from #97764.
Successfully run on LLVM codebase with ~100 findings and a couple of odd FPs: when we have std::sort(this->begin(), this->end()) or std::sort(begin(), end()).
I didn't fix this cases since it will be a separate task for the core utils::UseRangesCheck.

Fixes #38468.

---

Full diff: https://github.com/llvm/llvm-project/pull/152047.diff

8 Files Affected:

• (modified) clang-tools-extra/clang-tidy/llvm/CMakeLists.txt (+1)
• (modified) clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp (+2)
• (added) clang-tools-extra/clang-tidy/llvm/UseRangesCheck.cpp (+90)
• (added) clang-tools-extra/clang-tidy/llvm/UseRangesCheck.h (+33)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
• (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+2-1)
• (added) clang-tools-extra/docs/clang-tidy/checks/llvm/use-ranges.rst (+58)
• (added) clang-tools-extra/test/clang-tidy/checkers/llvm/use-ranges.cpp (+128)

diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
index 41386cdb55b1f..78ef0444305ff 100644
--- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
@@ -12,6 +12,7 @@ add_clang_library(clangTidyLLVMModule STATIC
   PreferStaticOverAnonymousNamespaceCheck.cpp
   TwineLocalCheck.cpp
   UseNewMLIROpBuilderCheck.cpp
+  UseRangesCheck.cpp
 
   LINK_LIBS
   clangTidy
diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
index c7c61fd1649cc..c1f78caf44d16 100644
--- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -19,6 +19,7 @@
 #include "PreferStaticOverAnonymousNamespaceCheck.h"
 #include "TwineLocalCheck.h"
 #include "UseNewMLIROpBuilderCheck.h"
+#include "UseRangesCheck.h"
 
 namespace clang::tidy {
 namespace llvm_check {
@@ -43,6 +44,7 @@ class LLVMModule : public ClangTidyModule {
     CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
     CheckFactories.registerCheck<UseNewMlirOpBuilderCheck>(
         "llvm-use-new-mlir-op-builder");
+    CheckFactories.registerCheck<UseRangesCheck>("llvm-use-ranges");
   }
 
   ClangTidyOptions getModuleOptions() override {
diff --git a/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.cpp
new file mode 100644
index 0000000000000..0e11a017c60d5
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.cpp
@@ -0,0 +1,90 @@
+//===--- UseRangesCheck.cpp - clang-tidy ----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseRangesCheck.h"
+
+namespace clang::tidy::llvm_check {
+
+namespace {
+
+class StdToLLVMReplacer : public utils::UseRangesCheck::Replacer {
+public:
+  explicit StdToLLVMReplacer(
+      ArrayRef<utils::UseRangesCheck::Signature> Signatures)
+      : Signatures(Signatures) {}
+
+  ArrayRef<utils::UseRangesCheck::Signature>
+  getReplacementSignatures() const override {
+    return Signatures;
+  }
+
+  std::optional<std::string>
+  getReplaceName(const NamedDecl &OriginalName) const override {
+    return ("llvm::" + OriginalName.getName()).str();
+  }
+
+  std::optional<std::string>
+  getHeaderInclusion(const NamedDecl &) const override {
+    return "llvm/ADT/STLExtras.h";
+  }
+
+private:
+  SmallVector<utils::UseRangesCheck::Signature> Signatures;
+};
+
+} // namespace
+
+utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
+  ReplacerMap Results;
+
+  static const Signature SingleSig = {{0}};
+  static const Signature TwoSig = {{0}, {2}};
+
+  const auto AddStdToLLVM =
+      [&Results](llvm::IntrusiveRefCntPtr<Replacer> Replacer,
+                 std::initializer_list<StringRef> Names) {
+        for (const auto &Name : Names) {
+          Results.try_emplace(("::std::" + Name).str(), Replacer);
+        }
+      };
+
+  // Single range algorithms
+  AddStdToLLVM(llvm::makeIntrusiveRefCnt<StdToLLVMReplacer>(SingleSig),
+               {"all_of",      "any_of",        "none_of",     "for_each",
+                "find",        "find_if",       "find_if_not", "count",
+                "count_if",    "transform",     "replace",     "remove_if",
+                "sort",        "partition",     "is_sorted",   "min_element",
+                "max_element", "binary_search", "lower_bound", "upper_bound",
+                "unique",      "copy",          "copy_if",     "fill"});
+
+  // Two range algorithms
+  AddStdToLLVM(llvm::makeIntrusiveRefCnt<StdToLLVMReplacer>(TwoSig),
+               {"equal", "mismatch"});
+
+  return Results;
+}
+
+UseRangesCheck::UseRangesCheck(StringRef Name, ClangTidyContext *Context)
+    : utils::UseRangesCheck(Name, Context) {}
+
+DiagnosticBuilder UseRangesCheck::createDiag(const CallExpr &Call) {
+  return diag(Call.getBeginLoc(), "use a llvm range-based algorithm");
+}
+
+ArrayRef<std::pair<StringRef, StringRef>>
+UseRangesCheck::getFreeBeginEndMethods() const {
+  static const std::pair<StringRef, StringRef> Refs[] = {
+      {"::std::begin", "::std::end"},
+      {"::std::cbegin", "::std::cend"},
+      {"::std::rbegin", "::std::rend"},
+      {"::std::crbegin", "::std::crend"},
+  };
+  return Refs;
+}
+
+} // namespace clang::tidy::llvm_check
diff --git a/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.h b/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.h
new file mode 100644
index 0000000000000..e9904e11ced36
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/llvm/UseRangesCheck.h
@@ -0,0 +1,33 @@
+//===--- UseRangesCheck.h - clang-tidy --------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USERANGESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USERANGESCHECK_H
+
+#include "../utils/UseRangesCheck.h"
+
+namespace clang::tidy::llvm_check {
+
+/// Finds calls to STL iterator algorithms that can be replaced with LLVM
+/// range-based algorithms from `llvm/ADT/STLExtras.h`.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvm/use-ranges.html
+class UseRangesCheck : public utils::UseRangesCheck {
+public:
+  UseRangesCheck(StringRef Name, ClangTidyContext *Context);
+
+  ReplacerMap getReplacerMap() const override;
+  DiagnosticBuilder createDiag(const CallExpr &Call) override;
+  ArrayRef<std::pair<StringRef, StringRef>>
+  getFreeBeginEndMethods() const override;
+};
+
+} // namespace clang::tidy::llvm_check
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USERANGESCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 85b31bc0b42a6..823ce382c61c6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -124,6 +124,12 @@ New checks
   Checks for uses of MLIR's old/to be deprecated ``OpBuilder::create<T>`` form
   and suggests using ``T::create`` instead.
 
+- New :doc:`llvm-use-ranges
+  <clang-tidy/checks/llvm/use-ranges>` check.
+
+  Detects calls to standard library iterator algorithms that could be replaced
+  with LLVM range-based algorithms from ``llvm/ADT/STLExtras.h``.
+
 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 b6444eb3c9aec..87c24cc1dfdfe 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -248,12 +248,13 @@ Clang-Tidy Checks
    :doc:`linuxkernel-must-check-errs <linuxkernel/must-check-errs>`,
    :doc:`llvm-header-guard <llvm/header-guard>`,
    :doc:`llvm-include-order <llvm/include-order>`, "Yes"
-   :doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
    :doc:`llvm-namespace-comment <llvm/namespace-comment>`,
    :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm/prefer-isa-or-dyn-cast-in-conditionals>`, "Yes"
    :doc:`llvm-prefer-register-over-unsigned <llvm/prefer-register-over-unsigned>`, "Yes"
    :doc:`llvm-prefer-static-over-anonymous-namespace <llvm/prefer-static-over-anonymous-namespace>`,
    :doc:`llvm-twine-local <llvm/twine-local>`, "Yes"
+   :doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
+   :doc:`llvm-use-ranges <llvm/use-ranges>`, "Yes"
    :doc:`llvmlibc-callee-namespace <llvmlibc/callee-namespace>`,
    :doc:`llvmlibc-implementation-in-namespace <llvmlibc/implementation-in-namespace>`,
    :doc:`llvmlibc-inline-function-decl <llvmlibc/inline-function-decl>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/use-ranges.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/use-ranges.rst
new file mode 100644
index 0000000000000..a5fc83ad5d2c2
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/use-ranges.rst
@@ -0,0 +1,58 @@
+.. title:: clang-tidy - llvm-use-ranges
+
+llvm-use-ranges
+===============
+
+Detects calls to standard library iterator algorithms that could be replaced
+with LLVM range-based algorithms from ``llvm/ADT/STLExtras.h``.
+
+Example
+-------
+
+.. code-block:: c++
+
+  auto it = std::find(vec.begin(), vec.end(), value);
+  bool all = std::all_of(vec.begin(), vec.end(), 
+                         [](int x) { return x > 0; });
+
+Transforms to:
+
+.. code-block:: c++
+
+  auto it = llvm::find(vec, value);
+  bool all = llvm::all_of(vec, [](int x) { return x > 0; });
+
+Supported algorithms
+--------------------
+
+Calls to the following ``std`` library algorithms are checked:
+
+``std::all_of``,
+``std::any_of``,
+``std::binary_search``,
+``std::copy``,
+``std::copy_if``,
+``std::count``,
+``std::count_if``,
+``std::equal``,
+``std::fill``,
+``std::find``,
+``std::find_if``,
+``std::find_if_not``,
+``std::for_each``,
+``std::is_sorted``,
+``std::lower_bound``,
+``std::max_element``,
+``std::min_element``,
+``std::mismatch``,
+``std::none_of``,
+``std::partition``,
+``std::remove_if``,
+``std::replace``,
+``std::sort``,
+``std::transform``,
+``std::unique``,
+``std::upper_bound``.
+
+The check will add the necessary ``#include "llvm/ADT/STLExtras.h"`` directive
+when applying fixes.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-ranges.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-ranges.cpp
new file mode 100644
index 0000000000000..3c42caa8cc1d1
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-ranges.cpp
@@ -0,0 +1,128 @@
+// RUN: %check_clang_tidy %s llvm-use-ranges %t
+
+// Test that the header is included
+// CHECK-FIXES: #include "llvm/ADT/STLExtras.h"
+
+namespace std {
+
+template <typename T> class vector {
+public:
+  using iterator = T *;
+  using const_iterator = const T *;
+
+  iterator begin();
+  iterator end();
+  const_iterator begin() const;
+  const_iterator end() const;
+  const_iterator cbegin() const;
+  const_iterator cend() const;
+};
+
+template <typename T> T* begin(T (&arr)[5]);
+template <typename T> T* end(T (&arr)[5]);
+
+template <class InputIt, class T>
+InputIt find(InputIt first, InputIt last, const T &value);
+
+template <class RandomIt>
+void sort(RandomIt first, RandomIt last);
+
+template <class InputIt, class UnaryPredicate>
+bool all_of(InputIt first, InputIt last, UnaryPredicate p);
+
+template <class InputIt, class UnaryFunction>
+UnaryFunction for_each(InputIt first, InputIt last, UnaryFunction f);
+
+template <class ForwardIt, class T>
+ForwardIt remove(ForwardIt first, ForwardIt last, const T& value);
+
+template <class ForwardIt>
+ForwardIt min_element(ForwardIt first, ForwardIt last);
+
+template <class InputIt1, class InputIt2>
+bool equal(InputIt1 first1, InputIt1 last1, InputIt2 first2, InputIt2 last2);
+
+template <class InputIt1, class InputIt2>
+bool equal(InputIt1 first1, InputIt1 last1, InputIt2 first2);
+
+template <class InputIt, class OutputIt>
+OutputIt copy(InputIt first, InputIt last, OutputIt d_first);
+
+template <class ForwardIt, class T>
+void fill(ForwardIt first, ForwardIt last, const T& value);
+
+template <class BidirIt>
+void reverse(BidirIt first, BidirIt last);
+
+template <class ForwardIt>
+ForwardIt unique(ForwardIt first, ForwardIt last);
+
+template <class ForwardIt>
+bool is_sorted(ForwardIt first, ForwardIt last);
+
+} // namespace std
+
+bool is_even(int x);
+void double_ref(int& x);
+
+void test_positive() {
+  std::vector<int> vec;
+  int arr[5] = {1, 2, 3, 4, 5};
+  
+  auto it1 = std::find(vec.begin(), vec.end(), 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: auto it1 = llvm::find(vec, 3);
+
+  auto it2 = std::find(std::begin(arr), std::end(arr), 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: auto it2 = llvm::find(arr, 3);
+
+  std::sort(vec.begin(), vec.end());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: llvm::sort(vec);
+
+  bool all = std::all_of(vec.begin(), vec.end(), is_even);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: bool all = llvm::all_of(vec, is_even);
+
+  std::for_each(vec.begin(), vec.end(), double_ref);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: llvm::for_each(vec, double_ref);
+
+  auto min_it = std::min_element(vec.begin(), vec.end());
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: auto min_it = llvm::min_element(vec);
+
+  std::vector<int> vec2;
+  bool eq = std::equal(vec.begin(), vec.end(), vec2.begin(), vec2.end());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: bool eq = llvm::equal(vec, vec2);
+
+  std::copy(vec.begin(), vec.end(), vec2.begin());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: llvm::copy(vec, vec2.begin());
+
+  std::fill(vec.begin(), vec.end(), 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: llvm::fill(vec, 0);
+  
+  auto last = std::unique(vec.begin(), vec.end());
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: auto last = llvm::unique(vec);
+
+  bool sorted = std::is_sorted(vec.begin(), vec.end());
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use a llvm range-based algorithm
+  // CHECK-FIXES: bool sorted = llvm::is_sorted(vec);
+}
+
+void test_negative() {
+  std::vector<int> v;
+  
+  //non-begin/end iterators
+  auto it1 = std::find(v.begin() + 1, v.end(), 2);
+  auto it2 = std::find(v.begin(), v.end() - 1, 2);
+  
+  // Using different containers (3-arg equal)
+  std::vector<int> v2;
+  bool eq = std::equal(v.begin(), v.end(), v2.begin());
+}

[vbvictor]

CC @kazutakahirata

[EugeneZelenko]

Suggested change

	return diag(Call.getBeginLoc(), "use a llvm range-based algorithm");
	return diag(Call.getBeginLoc(), "use a LLVM range-based algorithm");

[kazutakahirata]

Thank you for coming up with this! It might be a very hacky thing to say, but could we exclude llvm::sort from the list? Changing std::sort to llvm::sort sometimes changes the underlying sort algorithm, and that breaks the order of "tie" elements. That in turn breaks tests.

By the way, you might want to add support for llvm::stable_sort, which is a lot more "peaceful" than llvm::sort. Thanks!