-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[clang-tidy] Suggest std::views::reverse instead of std::ranges::reverse_view in modernize-use-ranges
#172199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…everse_view` in `modernize-use-ranges`
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Victor Chernyakin (localspook) Changes
Full diff: https://github.com/llvm/llvm-project/pull/172199.diff 6 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index 19b406f05a746..ce01a85f70fde 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -1079,7 +1079,7 @@ llvm::StringRef LoopConvertCheck::getReverseFunction() const {
if (!ReverseFunction.empty())
return ReverseFunction;
if (UseReverseRanges)
- return "std::ranges::reverse_view";
+ return "std::views::reverse";
return "";
}
diff --git a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
index 529b5a43adfc8..eb4d6e15fc722 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
@@ -197,8 +197,7 @@ std::optional<UseRangesCheck::ReverseIteratorDescriptor>
UseRangesCheck::getReverseDescriptor() const {
static const std::pair<StringRef, StringRef> Refs[] = {
{"::std::rbegin", "::std::rend"}, {"::std::crbegin", "::std::crend"}};
- return ReverseIteratorDescriptor{UseReversePipe ? "std::views::reverse"
- : "std::ranges::reverse_view",
- "<ranges>", Refs, UseReversePipe};
+ return ReverseIteratorDescriptor{"std::views::reverse", "<ranges>", Refs,
+ UseReversePipe};
}
} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d1fb1cba3e45a..3faadf21038c0 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -508,6 +508,11 @@ Changes in existing checks
on Windows when the check was enabled with a 32-bit :program:`clang-tidy`
binary.
+- Improved :doc:`modernize-use-ranges
+ <clang-tidy/checks/modernize/use-ranges>` check to suggest using
+ the more idiomatic ``std::views::reverse`` where it used to suggest
+ ``std::ranges::reverse_view``.
+
- Improved :doc:`modernize-use-scoped-lock
<clang-tidy/checks/modernize/use-scoped-lock>` check by fixing a crash
on malformed code (common when using :program:`clang-tidy` through
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-reverse.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-reverse.cpp
index 403f23c58b06f..8f35a9f514a06 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-reverse.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-reverse.cpp
@@ -95,7 +95,7 @@ void constContainer(const Reversable<int> &Numbers) {
observe(*I);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
// CHECK-FIXES-NEXT: observe(Number);
// CHECK-FIXES-NEXT: }
@@ -104,7 +104,7 @@ void constContainer(const Reversable<int> &Numbers) {
observe(*I);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
// CHECK-FIXES-NEXT: observe(Number);
// CHECK-FIXES-NEXT: }
@@ -113,7 +113,7 @@ void constContainer(const Reversable<int> &Numbers) {
observe(*I);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
// CHECK-FIXES-NEXT: observe(Number);
// CHECK-FIXES-NEXT: }
@@ -122,7 +122,7 @@ void constContainer(const Reversable<int> &Numbers) {
observe(*I);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
// CHECK-FIXES-NEXT: observe(Number);
// CHECK-FIXES-NEXT: }
@@ -141,7 +141,7 @@ void nonConstContainer(Reversable<int> &Numbers) {
mutate(*I);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES-RANGES: for (int & Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES: for (int & Number : std::views::reverse(Numbers)) {
// CHECK-FIXES-CUSTOM: for (int & Number : llvm::reverse(Numbers)) {
// CHECK-FIXES-NEXT: mutate(Number);
// CHECK-FIXES-NEXT: }
@@ -150,7 +150,7 @@ void nonConstContainer(Reversable<int> &Numbers) {
observe(*I);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES-RANGES: for (int Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES: for (int Number : std::views::reverse(Numbers)) {
// CHECK-FIXES-CUSTOM: for (int Number : llvm::reverse(Numbers)) {
// CHECK-FIXES-NEXT: observe(Number);
// CHECK-FIXES-NEXT: }
@@ -159,7 +159,7 @@ void nonConstContainer(Reversable<int> &Numbers) {
mutate(*I);
}
// CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
- // CHECK-FIXES-RANGES: for (int & Number : std::ranges::reverse_view(Numbers)) {
+ // CHECK-FIXES-RANGES: for (int & Number : std::views::reverse(Numbers)) {
// CHECK-FIXES-CUSTOM: for (int & Number : llvm::reverse(Numbers)) {
// CHECK-FIXES-NEXT: mutate(Number);
// CHECK-FIXES-NEXT: }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-pipe.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-pipe.cpp
index f53fb70427e2d..c084bcddaabb8 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-pipe.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-pipe.cpp
@@ -12,7 +12,6 @@ void stdLib() {
std::vector<int> I;
std::find(I.rbegin(), I.rend(), 0);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
- // CHECK-FIXES-NOPIPE: std::ranges::find(std::ranges::reverse_view(I), 0);
+ // CHECK-FIXES-NOPIPE: std::ranges::find(std::views::reverse(I), 0);
// CHECK-FIXES-PIPE: std::ranges::find(I | std::views::reverse, 0);
-
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp
index 5aa026038b1cd..6b196a8f42189 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp
@@ -94,19 +94,19 @@ void Reverse(){
std::find(K.rbegin(), K.rend(), std::unique_ptr<int>());
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
- // CHECK-FIXES: std::ranges::find(std::ranges::reverse_view(K), std::unique_ptr<int>());
+ // CHECK-FIXES: std::ranges::find(std::views::reverse(K), std::unique_ptr<int>());
std::find(I.rbegin(), I.rend(), 0);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
- // CHECK-FIXES: std::ranges::find(std::ranges::reverse_view(I), 0);
+ // CHECK-FIXES: std::ranges::find(std::views::reverse(I), 0);
std::equal(std::rbegin(I), std::rend(I), J.begin(), J.end());
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
- // CHECK-FIXES: std::ranges::equal(std::ranges::reverse_view(I), J);
+ // CHECK-FIXES: std::ranges::equal(std::views::reverse(I), J);
std::equal(I.begin(), I.end(), std::crbegin(J), std::crend(J));
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
- // CHECK-FIXES: std::ranges::equal(I, std::ranges::reverse_view(J));
+ // CHECK-FIXES: std::ranges::equal(I, std::views::reverse(J));
}
void Negatives() {
|
|
Please update documentation of these checks. |
…everse_view` in `modernize-use-ranges` (llvm#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).
std::views::FOOshould in almost all cases be preferred overstd::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).