From 0e0b0feff194358e5e68bf36f5a563d269fa8e88 Mon Sep 17 00:00:00 2001 From: Nathan James <n.james93@hotmail.co.uk> Date: Fri, 8 Apr 2022 14:17:37 +0100 Subject: [PATCH] [clang-tidy] Make performance-inefficient-vector-operation work on members Fixes https://llvm.org/PR50157 Adds support for when the container being read from in a range-for is a member of a struct. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D101624 --- .../InefficientVectorOperationCheck.cpp | 9 +++++- clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ ...rformance-inefficient-vector-operation.cpp | 30 ++++++++++++++++++- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp index a12d403d6ba0..e8c895e683c4 100644 --- a/clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp +++ b/clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp @@ -68,6 +68,10 @@ ast_matchers::internal::Matcher<Expr> supportedContainerTypesMatcher() { "::std::unordered_map", "::std::array", "::std::deque"))); } +AST_MATCHER(Expr, hasSideEffects) { + return Node.HasSideEffects(Finder->getASTContext()); +} + } // namespace InefficientVectorOperationCheck::InefficientVectorOperationCheck( @@ -145,7 +149,10 @@ void InefficientVectorOperationCheck::addMatcher( // FIXME: Support more complex range-expressions. Finder->addMatcher( cxxForRangeStmt( - hasRangeInit(declRefExpr(supportedContainerTypesMatcher())), + hasRangeInit( + anyOf(declRefExpr(supportedContainerTypesMatcher()), + memberExpr(hasObjectExpression(unless(hasSideEffects())), + supportedContainerTypesMatcher()))), HasInterestingLoopBody, InInterestingCompoundStmt) .bind(RangeLoopName), this); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 58ddb116f984..11ed0e3ed2b6 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -127,6 +127,10 @@ New check aliases Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ +- Improved :doc:`performance-inefficient-vector-operation + <clang-tidy/checks/performance-inefficient-vector-operation>` to work when + the vector is a member of a structure. + - Fixed a false positive in :doc:`readability-non-const-parameter <clang-tidy/checks/readability-non-const-parameter>` when the parameter is referenced by an lvalue diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp index c603cf83df23..105d27656217 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp @@ -44,7 +44,7 @@ class vector { void reserve(size_t n); void resize(size_t n); - size_t size(); + size_t size() const; const_reference operator[] (size_type) const; reference operator[] (size_type); @@ -359,3 +359,31 @@ void f(std::vector<int>& t) { } } } + +struct StructWithFieldContainer { + std::vector<int> Numbers; + std::vector<int> getNumbers() const { + std::vector<int> Result; + // CHECK-FIXES: Result.reserve(Numbers.size()); + for (auto Number : Numbers) { + Result.push_back(Number); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called + } + return Result; + } +}; + +StructWithFieldContainer getStructWithField(); + +void foo(const StructWithFieldContainer &Src) { + std::vector<int> A; + // CHECK-FIXES: A.reserve(Src.Numbers.size()); + for (auto Number : Src.Numbers) { + A.push_back(Number); + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'push_back' is called + } + std::vector<int> B; + for (auto Number : getStructWithField().Numbers) { + B.push_back(Number); + } +} -- GitLab