Skip to content

Commit d27fae7

Browse files
[clang-tidy] Add new check 'llvm-use-ranges' (#152047)
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. --------- Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
1 parent e0f00bd commit d27fae7

File tree

8 files changed

+343
-1
lines changed

8 files changed

+343
-1
lines changed

clang-tools-extra/clang-tidy/llvm/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ add_clang_library(clangTidyLLVMModule STATIC
1212
PreferStaticOverAnonymousNamespaceCheck.cpp
1313
TwineLocalCheck.cpp
1414
UseNewMLIROpBuilderCheck.cpp
15+
UseRangesCheck.cpp
1516

1617
LINK_LIBS
1718
clangTidy

clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "PreferStaticOverAnonymousNamespaceCheck.h"
2020
#include "TwineLocalCheck.h"
2121
#include "UseNewMLIROpBuilderCheck.h"
22+
#include "UseRangesCheck.h"
2223

2324
namespace clang::tidy {
2425
namespace llvm_check {
@@ -43,6 +44,7 @@ class LLVMModule : public ClangTidyModule {
4344
CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
4445
CheckFactories.registerCheck<UseNewMlirOpBuilderCheck>(
4546
"llvm-use-new-mlir-op-builder");
47+
CheckFactories.registerCheck<UseRangesCheck>("llvm-use-ranges");
4648
}
4749

4850
ClangTidyOptions getModuleOptions() override {
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
//===--- UseRangesCheck.cpp - clang-tidy ----------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "UseRangesCheck.h"
10+
11+
namespace clang::tidy::llvm_check {
12+
13+
namespace {
14+
15+
class StdToLLVMReplacer : public utils::UseRangesCheck::Replacer {
16+
public:
17+
explicit StdToLLVMReplacer(
18+
ArrayRef<utils::UseRangesCheck::Signature> Signatures)
19+
: Signatures(Signatures) {}
20+
21+
ArrayRef<utils::UseRangesCheck::Signature>
22+
getReplacementSignatures() const override {
23+
return Signatures;
24+
}
25+
26+
std::optional<std::string>
27+
getReplaceName(const NamedDecl &OriginalName) const override {
28+
return ("llvm::" + OriginalName.getName()).str();
29+
}
30+
31+
std::optional<std::string>
32+
getHeaderInclusion(const NamedDecl &) const override {
33+
return "llvm/ADT/STLExtras.h";
34+
}
35+
36+
private:
37+
ArrayRef<utils::UseRangesCheck::Signature> Signatures;
38+
};
39+
40+
} // namespace
41+
42+
utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
43+
ReplacerMap Results;
44+
45+
static const Signature SingleSig = {{0}};
46+
static const Signature TwoSig = {{0}, {2}};
47+
48+
const auto AddStdToLLVM =
49+
[&Results](llvm::IntrusiveRefCntPtr<Replacer> Replacer,
50+
std::initializer_list<StringRef> Names) {
51+
for (const auto &Name : Names) {
52+
Results.try_emplace(("::std::" + Name).str(), Replacer);
53+
}
54+
};
55+
56+
// Single range algorithms
57+
AddStdToLLVM(llvm::makeIntrusiveRefCnt<StdToLLVMReplacer>(SingleSig),
58+
{"all_of", "any_of",
59+
"none_of", "for_each",
60+
"find", "find_if",
61+
"find_if_not", "fill",
62+
"count", "count_if",
63+
"copy", "copy_if",
64+
"transform", "replace",
65+
"remove_if", "stable_sort",
66+
"partition", "partition_point",
67+
"is_sorted", "min_element",
68+
"max_element", "binary_search",
69+
"lower_bound", "upper_bound",
70+
"unique", "uninitialized_copy"});
71+
72+
// Two range algorithms
73+
AddStdToLLVM(llvm::makeIntrusiveRefCnt<StdToLLVMReplacer>(TwoSig),
74+
{"equal", "mismatch", "includes"});
75+
76+
return Results;
77+
}
78+
79+
UseRangesCheck::UseRangesCheck(StringRef Name, ClangTidyContext *Context)
80+
: utils::UseRangesCheck(Name, Context) {}
81+
82+
DiagnosticBuilder UseRangesCheck::createDiag(const CallExpr &Call) {
83+
return diag(Call.getBeginLoc(), "use an LLVM range-based algorithm");
84+
}
85+
86+
ArrayRef<std::pair<StringRef, StringRef>>
87+
UseRangesCheck::getFreeBeginEndMethods() const {
88+
static constexpr std::pair<StringRef, StringRef> Refs[] = {
89+
{"::std::begin", "::std::end"},
90+
{"::std::cbegin", "::std::cend"},
91+
{"::std::rbegin", "::std::rend"},
92+
{"::std::crbegin", "::std::crend"},
93+
};
94+
return Refs;
95+
}
96+
97+
} // namespace clang::tidy::llvm_check
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//===--- UseRangesCheck.h - clang-tidy --------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USERANGESCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USERANGESCHECK_H
11+
12+
#include "../utils/UseRangesCheck.h"
13+
14+
namespace clang::tidy::llvm_check {
15+
16+
/// Finds calls to STL iterator algorithms that can be replaced with LLVM
17+
/// range-based algorithms from `llvm/ADT/STLExtras.h`.
18+
///
19+
/// For the user-facing documentation see:
20+
/// http://clang.llvm.org/extra/clang-tidy/checks/llvm/use-ranges.html
21+
class UseRangesCheck : public utils::UseRangesCheck {
22+
public:
23+
UseRangesCheck(StringRef Name, ClangTidyContext *Context);
24+
25+
ReplacerMap getReplacerMap() const override;
26+
DiagnosticBuilder createDiag(const CallExpr &Call) override;
27+
ArrayRef<std::pair<StringRef, StringRef>>
28+
getFreeBeginEndMethods() const override;
29+
};
30+
31+
} // namespace clang::tidy::llvm_check
32+
33+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USERANGESCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,12 @@ New checks
157157
Checks for uses of MLIR's old/to be deprecated ``OpBuilder::create<T>`` form
158158
and suggests using ``T::create`` instead.
159159

160+
- New :doc:`llvm-use-ranges
161+
<clang-tidy/checks/llvm/use-ranges>` check.
162+
163+
Finds calls to STL library iterator algorithms that could be replaced with
164+
LLVM range-based algorithms from ``llvm/ADT/STLExtras.h``.
165+
160166
- New :doc:`misc-override-with-different-visibility
161167
<clang-tidy/checks/misc/override-with-different-visibility>` check.
162168

clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,13 @@ Clang-Tidy Checks
249249
:doc:`linuxkernel-must-check-errs <linuxkernel/must-check-errs>`,
250250
:doc:`llvm-header-guard <llvm/header-guard>`,
251251
:doc:`llvm-include-order <llvm/include-order>`, "Yes"
252-
:doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
253252
:doc:`llvm-namespace-comment <llvm/namespace-comment>`,
254253
:doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm/prefer-isa-or-dyn-cast-in-conditionals>`, "Yes"
255254
:doc:`llvm-prefer-register-over-unsigned <llvm/prefer-register-over-unsigned>`, "Yes"
256255
:doc:`llvm-prefer-static-over-anonymous-namespace <llvm/prefer-static-over-anonymous-namespace>`,
257256
:doc:`llvm-twine-local <llvm/twine-local>`, "Yes"
257+
:doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
258+
:doc:`llvm-use-ranges <llvm/use-ranges>`, "Yes"
258259
:doc:`llvmlibc-callee-namespace <llvmlibc/callee-namespace>`,
259260
:doc:`llvmlibc-implementation-in-namespace <llvmlibc/implementation-in-namespace>`,
260261
:doc:`llvmlibc-inline-function-decl <llvmlibc/inline-function-decl>`, "Yes"
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
.. title:: clang-tidy - llvm-use-ranges
2+
3+
llvm-use-ranges
4+
===============
5+
6+
Finds calls to STL library iterator algorithms that could be replaced with
7+
LLVM range-based algorithms from ``llvm/ADT/STLExtras.h``.
8+
9+
Example
10+
-------
11+
12+
.. code-block:: c++
13+
14+
auto it = std::find(vec.begin(), vec.end(), value);
15+
bool all = std::all_of(vec.begin(), vec.end(),
16+
[](int x) { return x > 0; });
17+
18+
Transforms to:
19+
20+
.. code-block:: c++
21+
22+
auto it = llvm::find(vec, value);
23+
bool all = llvm::all_of(vec, [](int x) { return x > 0; });
24+
25+
Supported algorithms
26+
--------------------
27+
28+
Calls to the following STL algorithms are checked:
29+
30+
``std::all_of``,
31+
``std::any_of``,
32+
``std::binary_search``,
33+
``std::copy``,
34+
``std::copy_if``,
35+
``std::count``,
36+
``std::count_if``,
37+
``std::equal``,
38+
``std::fill``,
39+
``std::find``,
40+
``std::find_if``,
41+
``std::find_if_not``,
42+
``std::for_each``,
43+
``std::includes``,
44+
``std::is_sorted``,
45+
``std::lower_bound``,
46+
``std::max_element``,
47+
``std::min_element``,
48+
``std::mismatch``,
49+
``std::none_of``,
50+
``std::partition``,
51+
``std::partition_point``,
52+
``std::remove_if``,
53+
``std::replace``,
54+
``std::stable_sort``,
55+
``std::transform``,
56+
``std::uninitialized_copy``,
57+
``std::unique``,
58+
``std::upper_bound``.
59+
60+
The check will add the necessary ``#include "llvm/ADT/STLExtras.h"`` directive
61+
when applying fixes.
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
// RUN: %check_clang_tidy %s llvm-use-ranges %t
2+
3+
// Test that the header is included
4+
// CHECK-FIXES: #include "llvm/ADT/STLExtras.h"
5+
6+
namespace std {
7+
8+
template <typename T> class vector {
9+
public:
10+
using iterator = T *;
11+
using const_iterator = const T *;
12+
13+
iterator begin();
14+
iterator end();
15+
const_iterator begin() const;
16+
const_iterator end() const;
17+
const_iterator cbegin() const;
18+
const_iterator cend() const;
19+
};
20+
21+
template <typename T> T* begin(T (&arr)[5]);
22+
template <typename T> T* end(T (&arr)[5]);
23+
24+
template <class InputIt, class T>
25+
InputIt find(InputIt first, InputIt last, const T &value);
26+
27+
template <class RandomIt>
28+
void sort(RandomIt first, RandomIt last);
29+
30+
template <class RandomIt>
31+
void stable_sort(RandomIt first, RandomIt last);
32+
33+
template <class InputIt, class UnaryPredicate>
34+
bool all_of(InputIt first, InputIt last, UnaryPredicate p);
35+
36+
template <class InputIt, class UnaryFunction>
37+
UnaryFunction for_each(InputIt first, InputIt last, UnaryFunction f);
38+
39+
template <class ForwardIt, class T>
40+
ForwardIt remove(ForwardIt first, ForwardIt last, const T& value);
41+
42+
template <class ForwardIt>
43+
ForwardIt min_element(ForwardIt first, ForwardIt last);
44+
45+
template <class InputIt1, class InputIt2>
46+
bool equal(InputIt1 first1, InputIt1 last1, InputIt2 first2, InputIt2 last2);
47+
48+
template <class InputIt1, class InputIt2>
49+
bool equal(InputIt1 first1, InputIt1 last1, InputIt2 first2);
50+
51+
template <class InputIt, class OutputIt>
52+
OutputIt copy(InputIt first, InputIt last, OutputIt d_first);
53+
54+
template <class ForwardIt, class T>
55+
void fill(ForwardIt first, ForwardIt last, const T& value);
56+
57+
template <class BidirIt>
58+
void reverse(BidirIt first, BidirIt last);
59+
60+
template <class ForwardIt>
61+
ForwardIt unique(ForwardIt first, ForwardIt last);
62+
63+
template <class ForwardIt>
64+
bool is_sorted(ForwardIt first, ForwardIt last);
65+
66+
template <class InputIt1, class InputIt2>
67+
bool includes(InputIt1 first1, InputIt1 last1, InputIt2 first2, InputIt2 last2);
68+
69+
} // namespace std
70+
71+
bool is_even(int x);
72+
void double_ref(int& x);
73+
74+
void test_positive() {
75+
std::vector<int> vec;
76+
int arr[5] = {1, 2, 3, 4, 5};
77+
78+
auto it1 = std::find(vec.begin(), vec.end(), 3);
79+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use an LLVM range-based algorithm
80+
// CHECK-FIXES: auto it1 = llvm::find(vec, 3);
81+
82+
auto it2 = std::find(std::begin(arr), std::end(arr), 3);
83+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use an LLVM range-based algorithm
84+
// CHECK-FIXES: auto it2 = llvm::find(arr, 3);
85+
86+
std::stable_sort(vec.begin(), vec.end());
87+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use an LLVM range-based algorithm
88+
// CHECK-FIXES: llvm::stable_sort(vec);
89+
90+
bool all = std::all_of(vec.begin(), vec.end(), is_even);
91+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use an LLVM range-based algorithm
92+
// CHECK-FIXES: bool all = llvm::all_of(vec, is_even);
93+
94+
std::for_each(vec.begin(), vec.end(), double_ref);
95+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use an LLVM range-based algorithm
96+
// CHECK-FIXES: llvm::for_each(vec, double_ref);
97+
98+
auto min_it = std::min_element(vec.begin(), vec.end());
99+
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use an LLVM range-based algorithm
100+
// CHECK-FIXES: auto min_it = llvm::min_element(vec);
101+
102+
std::vector<int> vec2;
103+
bool eq = std::equal(vec.begin(), vec.end(), vec2.begin(), vec2.end());
104+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use an LLVM range-based algorithm
105+
// CHECK-FIXES: bool eq = llvm::equal(vec, vec2);
106+
107+
std::copy(vec.begin(), vec.end(), vec2.begin());
108+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use an LLVM range-based algorithm
109+
// CHECK-FIXES: llvm::copy(vec, vec2.begin());
110+
111+
std::fill(vec.begin(), vec.end(), 0);
112+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use an LLVM range-based algorithm
113+
// CHECK-FIXES: llvm::fill(vec, 0);
114+
115+
auto last = std::unique(vec.begin(), vec.end());
116+
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use an LLVM range-based algorithm
117+
// CHECK-FIXES: auto last = llvm::unique(vec);
118+
119+
bool sorted = std::is_sorted(vec.begin(), vec.end());
120+
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use an LLVM range-based algorithm
121+
// CHECK-FIXES: bool sorted = llvm::is_sorted(vec);
122+
123+
std::includes(vec.begin(), vec.end(), std::begin(arr), std::end(arr));
124+
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use an LLVM range-based algorithm
125+
// CHECK-FIXES: llvm::includes(vec, arr);
126+
}
127+
128+
void test_negative() {
129+
std::vector<int> v;
130+
131+
// can not use `llvm::sort` because of potential different ordering from `std::sort`.
132+
std::sort(v.begin(), v.end());
133+
134+
//non-begin/end iterators
135+
auto it1 = std::find(v.begin() + 1, v.end(), 2);
136+
auto it2 = std::find(v.begin(), v.end() - 1, 2);
137+
138+
// Using different containers (3-arg equal)
139+
std::vector<int> v2;
140+
bool eq = std::equal(v.begin(), v.end(), v2.begin());
141+
}

0 commit comments

Comments
 (0)