[clang-tools-extra] [clang-tidy] Fix false negative `modernize-use-ranges` when using getter function (PR #127377)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy

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

Author: None (Andrewyuan34)

<details>
<summary>Changes</summary>


This PR fixes issue #<!-- -->124906, where `modernize-use-ranges` failed to detect cases where a `const` member function returning an lvalue was incorrectly assumed to have side effects, preventing the transformation.


### Changes:
1. **Modified `clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp`**:
   - Updated `hasSideEffects` AST matcher to correctly treat `const` member functions returning lvalues as side-effect-free.
   
2. **Added unit tests**:
  - `const` getters returning lvalues (should trigger transformation).
  - Non-const getters returning lvalues (should not trigger transformation).
  - Getters returning rvalues (should not trigger transformation).
  - Corresponding reverse iterators usage.

### Linked Issue:
Fixes #<!-- -->124906 .

### Testing:
- All existing unit tests pass.
- New unit tests have been added to verify the fix.

Note:
If there are any issues or areas for improvement, please let me know—I’m happy to make adjustments and learn from your feedback! Thank you for your patience and guidance.

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


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp (+7-1) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-ranges.cpp (+30-2) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
index aba4d17ccd035..f5fd0a47fb136 100644
--- a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp
@@ -51,7 +51,13 @@ static std::string getFullPrefix(ArrayRef<UseRangesCheck::Indexes> Signature) {
 namespace {
 
 AST_MATCHER(Expr, hasSideEffects) {
-  return Node.HasSideEffects(Finder->getASTContext());
+  bool CheckArg = true;
+  if (const CXXMemberCallExpr *Call = dyn_cast<CXXMemberCallExpr>(&Node)) {
+    if (Call->isLValue() && Call->getMethodDecl()->isConst()) {
+      CheckArg = false;
+    }
+  }
+  return Node.HasSideEffects(Finder->getASTContext(), CheckArg);
 }
 } // namespace
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6b8fe22242417..f97c46d3b5dff 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -110,6 +110,11 @@ Changes in existing checks
   <clang-tidy/checks/misc/redundant-expression>` check by providing additional
   examples and fixing some macro related false positives.
 
+- Improved :doc:`modernize-use-ranges
+  <clang-tidy/checks/modernize/use-ranges>` check by correctly recognizes 
+  const member functions returning lvalues as side-effect-free, preventing 
+  missed transformations.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
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 b022efebfdf4d..63a6946728f6f 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
@@ -7,7 +7,15 @@
 
 #include "fake_std.h"
 
-void Positives() {
+struct S {
+  std::vector<int> v;
+
+  const std::vector<int>& get() const { return v; }
+  std::vector<int>& getMutable() { return v; } 
+  std::vector<int> getVal() const { return v; } 
+};
+
+void Positives(S& s) {
   std::vector<int> I, J;
   std::find(I.begin(), I.end(), 0);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
@@ -72,9 +80,19 @@ void Positives() {
   my_std::find(I.begin(), I.end(), 6);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
   // CHECK-FIXES: std::ranges::find(I, 6);
+
+  std::find(s.get().begin(), s.get().end(), 0); 
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+  // CHECK-FIXES: std::ranges::find(s.get(), 0);
+
+  // Return non-const function, should not generate message
+  std::find(s.getMutable().begin(), s.getMutable().end(), 0);
+
+  // Return value, should not generate message
+  std::find(s.getVal().begin(), s.getVal().end(), 0); 
 }
 
-void Reverse(){
+void Reverse(S& s){
   std::vector<int> I, J;
   std::find(I.rbegin(), I.rend(), 0);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
@@ -87,6 +105,16 @@ void Reverse(){
   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));
+
+  std::find(s.get().rbegin(), s.get().rend(), 0); 
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use a ranges version of this algorithm
+  // CHECK-FIXES: std::ranges::find(std::ranges::reverse_view(s.get()), 0);
+
+  // Return non-const function, should not generate message
+  std::find(s.getMutable().rbegin(), s.getMutable().rend(), 0);
+
+  // Return value, should not generate message
+  std::find(s.getVal().rbegin(), s.getVal().rend(), 0); 
 }
 
 void Negatives() {

``````````

</details>


https://github.com/llvm/llvm-project/pull/127377