Skip to content

Commit 21a25f4

Browse files
localspooklocalspook
authored
[clang-tidy] Suggest std::views::reverse instead of std::ranges::reverse_view in modernize-use-ranges (#172199)
`std::views::FOO` should in almost all cases be preferred over `std::ranges::FOO_view`. For a detailed explanation of why that is, see https://brevzin.github.io/c++/2023/03/14/prefer-views-meow/. The TLDR is that it's shorter to spell (which is obvious) and can in certain cases be more efficient (which is less obvious; see the article if curious).
1 parent ecfdf8c commit 21a25f4

File tree

8 files changed

+24
-21
lines changed

8 files changed

+24
-21
lines changed

‎clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1079,7 +1079,7 @@ llvm::StringRef LoopConvertCheck::getReverseFunction() const {
10791079
if (!ReverseFunction.empty())
10801080
return ReverseFunction;
10811081
if (UseReverseRanges)
1082-
return "std::ranges::reverse_view";
1082+
return "std::views::reverse";
10831083
return "";
10841084
}
10851085

‎clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,7 @@ std::optional<UseRangesCheck::ReverseIteratorDescriptor>
197197
UseRangesCheck::getReverseDescriptor() const {
198198
static const std::pair<StringRef, StringRef> Refs[] = {
199199
{"::std::rbegin", "::std::rend"}, {"::std::crbegin", "::std::crend"}};
200-
return ReverseIteratorDescriptor{UseReversePipe ? "std::views::reverse"
201-
: "std::ranges::reverse_view",
202-
"<ranges>", Refs, UseReversePipe};
200+
return ReverseIteratorDescriptor{"std::views::reverse", "<ranges>", Refs,
201+
UseReversePipe};
203202
}
204203
} // namespace clang::tidy::modernize

‎clang-tools-extra/docs/ReleaseNotes.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,11 @@ Changes in existing checks
531531
on Windows when the check was enabled with a 32-bit :program:`clang-tidy`
532532
binary.
533533

534+
- Improved :doc:`modernize-use-ranges
535+
<clang-tidy/checks/modernize/use-ranges>` check to suggest using
536+
the more idiomatic ``std::views::reverse`` where it used to suggest
537+
``std::ranges::reverse_view``.
538+
534539
- Improved :doc:`modernize-use-scoped-lock
535540
<clang-tidy/checks/modernize/use-scoped-lock>` check by fixing a crash
536541
on malformed code (common when using :program:`clang-tidy` through

‎clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,15 +147,15 @@ Options
147147
.. option:: UseCxx20ReverseRanges
148148

149149
When set to true convert loops when in C++20 or later mode using
150-
``std::ranges::reverse_view``.
150+
``std::views::reverse``.
151151
Default value is `true`.
152152

153153
.. option:: MakeReverseRangeFunction
154154

155155
Specify the function used to reverse an iterator pair, the function should
156156
accept a class with ``rbegin`` and ``rend`` methods and return a
157157
class with ``begin`` and ``end`` methods that call the ``rbegin`` and
158-
``rend`` methods respectively. Common examples are ``ranges::reverse_view``
158+
``rend`` methods respectively. Common examples are ``std::views::reverse``
159159
and ``llvm::reverse``.
160160
Default value is an empty string.
161161

‎clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ Transforms to:
122122

123123
.. code-block:: c++
124124

125-
auto AreSame = std::ranges::equal(std::ranges::reverse_view(Items1),
126-
std::ranges::reverse_view(Items2));
125+
auto AreSame = std::ranges::equal(std::views::reverse(Items1),
126+
std::views::reverse(Items2));
127127

128128
Options
129129
-------

‎clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-reverse.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ void constContainer(const Reversable<int> &Numbers) {
9595
observe(*I);
9696
}
9797
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
98-
// CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
98+
// CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
9999
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
100100
// CHECK-FIXES-NEXT: observe(Number);
101101
// CHECK-FIXES-NEXT: }
@@ -104,7 +104,7 @@ void constContainer(const Reversable<int> &Numbers) {
104104
observe(*I);
105105
}
106106
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
107-
// CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
107+
// CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
108108
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
109109
// CHECK-FIXES-NEXT: observe(Number);
110110
// CHECK-FIXES-NEXT: }
@@ -113,7 +113,7 @@ void constContainer(const Reversable<int> &Numbers) {
113113
observe(*I);
114114
}
115115
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
116-
// CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
116+
// CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
117117
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
118118
// CHECK-FIXES-NEXT: observe(Number);
119119
// CHECK-FIXES-NEXT: }
@@ -122,7 +122,7 @@ void constContainer(const Reversable<int> &Numbers) {
122122
observe(*I);
123123
}
124124
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
125-
// CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
125+
// CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
126126
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
127127
// CHECK-FIXES-NEXT: observe(Number);
128128
// CHECK-FIXES-NEXT: }
@@ -141,7 +141,7 @@ void nonConstContainer(Reversable<int> &Numbers) {
141141
mutate(*I);
142142
}
143143
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
144-
// CHECK-FIXES-RANGES: for (int & Number : std::ranges::reverse_view(Numbers)) {
144+
// CHECK-FIXES-RANGES: for (int & Number : std::views::reverse(Numbers)) {
145145
// CHECK-FIXES-CUSTOM: for (int & Number : llvm::reverse(Numbers)) {
146146
// CHECK-FIXES-NEXT: mutate(Number);
147147
// CHECK-FIXES-NEXT: }
@@ -150,7 +150,7 @@ void nonConstContainer(Reversable<int> &Numbers) {
150150
observe(*I);
151151
}
152152
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
153-
// CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
153+
// CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
154154
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
155155
// CHECK-FIXES-NEXT: observe(Number);
156156
// CHECK-FIXES-NEXT: }
@@ -159,7 +159,7 @@ void nonConstContainer(Reversable<int> &Numbers) {
159159
mutate(*I);
160160
}
161161
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
162-
// CHECK-FIXES-RANGES: for (int & Number : std::ranges::reverse_view(Numbers)) {
162+
// CHECK-FIXES-RANGES: for (int & Number : std::views::reverse(Numbers)) {
163163
// CHECK-FIXES-CUSTOM: for (int & Number : llvm::reverse(Numbers)) {
164164
// CHECK-FIXES-NEXT: mutate(Number);
165165
// CHECK-FIXES-NEXT: }

‎clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-pipe.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ void stdLib() {
1212
std::vector<int> I;
1313
std::find(I.rbegin(), I.rend(), 0);
1414
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
15-
// CHECK-FIXES-NOPIPE: std::ranges::find(std::ranges::reverse_view(I), 0);
15+
// CHECK-FIXES-NOPIPE: std::ranges::find(std::views::reverse(I), 0);
1616
// CHECK-FIXES-PIPE: std::ranges::find(I | std::views::reverse, 0);
17-
1817
}

‎clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,19 @@ void Reverse(){
9494

9595
std::find(K.rbegin(), K.rend(), std::unique_ptr<int>());
9696
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
97-
// CHECK-FIXES: std::ranges::find(std::ranges::reverse_view(K), std::unique_ptr<int>());
97+
// CHECK-FIXES: std::ranges::find(std::views::reverse(K), std::unique_ptr<int>());
9898

9999
std::find(I.rbegin(), I.rend(), 0);
100100
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
101-
// CHECK-FIXES: std::ranges::find(std::ranges::reverse_view(I), 0);
101+
// CHECK-FIXES: std::ranges::find(std::views::reverse(I), 0);
102102

103103
std::equal(std::rbegin(I), std::rend(I), J.begin(), J.end());
104104
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
105-
// CHECK-FIXES: std::ranges::equal(std::ranges::reverse_view(I), J);
105+
// CHECK-FIXES: std::ranges::equal(std::views::reverse(I), J);
106106

107107
std::equal(I.begin(), I.end(), std::crbegin(J), std::crend(J));
108108
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
109-
// CHECK-FIXES: std::ranges::equal(I, std::ranges::reverse_view(J));
109+
// CHECK-FIXES: std::ranges::equal(I, std::views::reverse(J));
110110
}
111111

112112
void Negatives() {

0 commit comments

Comments
 (0)