https://github.com/Andrewyuan34 updated https://github.com/llvm/llvm-project/pull/127377
>From fb2a4067517c1378ea6801ccd4a90207f607715f Mon Sep 17 00:00:00 2001 From: Andrewyuan34 <yi...@uvic.ca> Date: Thu, 20 Mar 2025 22:59:06 -0400 Subject: [PATCH] [clang-tidy] Fix false negative modernize-use-ranges when using getter function --- .../clang-tidy/utils/UseRangesCheck.cpp | 8 ++++- clang-tools-extra/docs/ReleaseNotes.rst | 5 +++ .../checkers/modernize/use-ranges.cpp | 32 +++++++++++++++++-- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/utils/UseRangesCheck.cpp index f7a19cd72d500..052b149eb1d47 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 72aa05eb4dcd1..69410d1f36d44 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -188,6 +188,11 @@ Changes in existing checks tolerating fix-it breaking compilation when functions is used as pointers to avoid matching usage of functions within the current compilation unit. +- 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 5aa026038b1cd..0927b617049e4 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 @@ -9,7 +9,15 @@ #include "use-ranges/fake_std.h" #include "smart-ptr/unique_ptr.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::vector<std::unique_ptr<int>> K; @@ -83,9 +91,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::vector<std::unique_ptr<int>> K; @@ -107,6 +125,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() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits