Add support for std::rotate(_copy) and inplace_merge to modernize-use-ranges

[njames93]

These algorithms take 3 iterators for the range and we are only interested in the first and last iterator argument. The ranges versions of these take a range and an iterator(defined to be inside the range) so the transformation is pretty similar algo(I.begin, other, I.end,...) -> ranges::algo(I, other,...)

[llvmbot]

@llvm/pr-subscribers-clang-tidy

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

Author: Nathan James (njames93)

Changes

These algorithms take 3 iterators for the range and we are only interested in the first and last iterator argument. The ranges versions of these take a range and an iterator(defined to be inside the range) so the transformation is pretty similar algo(I.begin, other, I.end,...) -> ranges::algo(I, other,...)

---

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

4 Files Affected:

• (modified) clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp (+11-1)
• (modified) clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst (+3)
• (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h (+3)
• (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp (+11)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
index b0a31ad53be3f..057ec9327d700 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseRangesCheck.cpp
@@ -96,6 +96,10 @@ static constexpr const char *TwoRangeNames[] = {
     "is_permutation",
 };
 
+static constexpr const char *SinglePivotRangeNames[] = {
+  "rotate","rotate_copy","inplace_merge"
+};
+
 namespace {
 class StdReplacer : public utils::UseRangesCheck::Replacer {
 public:
@@ -141,13 +145,19 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
   // Func(Iter1 first1, Iter1 last1, Iter2 first2, Iter2 last2,...).
   static const Signature TwoRangeArgs = {{0}, {2}};
 
+  // template<typename Iter> Func(Iter first, Iter pivot, Iter last,...).
+  static const Signature SinglePivotRange = {{0, 2}};
+
   static const Signature SingleRangeFunc[] = {SingleRangeArgs};
 
   static const Signature TwoRangeFunc[] = {TwoRangeArgs};
 
+  static const Signature SinglePivotFunc[] = {SinglePivotRange};
+
   static const std::pair<ArrayRef<Signature>, ArrayRef<const char *>>
       AlgorithmNames[] = {{SingleRangeFunc, SingleRangeNames},
-                          {TwoRangeFunc, TwoRangeNames}};
+                          {TwoRangeFunc, TwoRangeNames},
+                          {SinglePivotFunc, SinglePivotRangeNames}};
   SmallString<64> Buff;
   for (const auto &[Signatures, Values] : AlgorithmNames) {
     auto Replacer = llvm::makeIntrusiveRefCnt<StdAlgorithmReplacer>(
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst
index 5c0b8058e4535..1ce866ca1f66a 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-ranges.rst
@@ -46,6 +46,7 @@ Calls to the following std library algorithms are checked:
 ``std::for_each``,
 ``std::generate``,
 ``std::includes``,
+``std::inplace_merge``,
 ``std::iota``,
 ``std::is_heap_until``,
 ``std::is_heap``,
@@ -79,6 +80,8 @@ Calls to the following std library algorithms are checked:
 ``std::replace``,
 ``std::reverse_copy``,
 ``std::reverse``,
+``std::rotate``,
+``std::rotate_copy``,
 ``std::sample``,
 ``std::search``,
 ``std::set_difference``,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h
index 987ee4e35b3bc..6596511c7a38b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/use-ranges/fake_std.h
@@ -106,6 +106,9 @@ bool equal(InputIt1 first1, InputIt1 last1,
 template <class ForwardIt, class T>
 void iota(ForwardIt first, ForwardIt last, T value);
 
+template <class ForwardIt>
+ForwardIt rotate(ForwardIt first, ForwardIt middle, ForwardIt last);
+
 } // namespace std
 
 #endif // USE_RANGES_FAKE_STD_H
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 e937e1e4e7d3b..b022efebfdf4d 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
@@ -57,6 +57,10 @@ void Positives() {
   // CHECK-MESSAGES-CPP23: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
   // CHECK-FIXES-CPP23: std::ranges::iota(I, 0);
 
+  std::rotate(I.begin(), I.begin() + 2, I.end());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+  // CHECK-FIXES: std::ranges::rotate(I, I.begin() + 2);
+
   using std::find;
   namespace my_std = std;
 
@@ -100,4 +104,11 @@ void Negatives() {
   std::equal(I.begin(), I.end(), J.end(), J.end());
   std::equal(std::rbegin(I), std::rend(I), std::rend(J), std::rbegin(J));
   std::equal(I.begin(), J.end(), I.begin(), I.end());
+
+  // std::rotate expects the full range in the 1st and 3rd argument.
+  // Anyone writing this code has probably written a bug, but this isn't the
+  // purpose of this check.
+  std::rotate(I.begin(), I.end(), I.begin() + 2);
+  // Pathological, but probably shouldn't diagnose this
+  std::rotate(I.begin(), I.end(), I.end() + 0);
 }

[PiotrZSL]

LGTM, some tests for rotate_copy and implace_merge would be welcome but it isn't must as it work similar to rotate.

[njames93]

LGTM, some tests for rotate_copy and implace_merge would be welcome but it isn't must as it work similar to rotate.

Yeah, like with the other supported algorithms, I didn't see the need to add a test for every single function, as long as there's a test for each kind of function it should be ok