llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Victor Chernyakin (localspook) <details> <summary>Changes</summary> `std::views::FOO` should in almost all cases be preferred over `std::ranges::FOO_view`. For a very 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 potentially more efficient (which is less obvious; see the article if curious). --- Full diff: https://github.com/llvm/llvm-project/pull/172199.diff 6 Files Affected: - (modified) clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp (+1-1) - (modified) clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp (+2-3) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-reverse.cpp (+7-7) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges-pipe.cpp (+1-2) - (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp (+4-4) ``````````diff 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() { `````````` </details> https://github.com/llvm/llvm-project/pull/172199 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
