[PATCH] D101624: [clang-tidy] Make performance-inefficient-vector-operation work on members

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, gribozavr2.
Herald added a subscriber: xazax.hun.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fixes https://llvm.org/PR50157

Adds support for when the container being read from in a range-for is a member 
of a struct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101624

Files:
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
===================================================================
--- 
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
@@ -44,7 +44,7 @@
   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 @@
     }
   }
 }
+
+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);
+  }
+}
Index: 
clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
@@ -68,6 +68,10 @@
       "::std::unordered_map", "::std::array", "::std::deque")));
 }
 
+AST_MATCHER(Expr, hasSideEffects) {
+  return Node.HasSideEffects(Finder->getASTContext());
+}
+
 } // namespace
 
 InefficientVectorOperationCheck::InefficientVectorOperationCheck(
@@ -145,7 +149,10 @@
   // 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);



Index: clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
@@ -44,7 +44,7 @@
   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 @@
     }
   }
 }
+
+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);
+  }
+}
Index: clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
@@ -68,6 +68,10 @@
       "::std::unordered_map", "::std::array", "::std::deque")));
 }
 
+AST_MATCHER(Expr, hasSideEffects) {
+  return Node.HasSideEffects(Finder->getASTContext());
+}
+
 } // namespace
 
 InefficientVectorOperationCheck::InefficientVectorOperationCheck(
@@ -145,7 +149,10 @@
   // 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);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits