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

[njames93]

Summary

Fixes https://llvm.org/PR50157

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

[njames93]

Ping?

[aaron.ballman]

Sorry, this fell off my radar! LGTM, but please add a release note for the fix when landing.

clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp

 static const char RangeLoopName[] = "for_range_loop";
 
 ast_matchers::internal::Matcher<Expr> supportedContainerTypesMatcher() {
   return hasType(cxxRecordDecl(hasAnyName(
       "::std::vector", "::std::set", "::std::unordered_set", "::std::map",
       "::std::unordered_map", "::std::array", "::std::deque")));
 }
 
+AST_MATCHER(Expr, hasSideEffects) {
+  return Node.HasSideEffects(Finder->getASTContext());
+}
+
 } // namespace
 
 InefficientVectorOperationCheck::InefficientVectorOperationCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       VectorLikeClasses(utils::options::parseStringList(
           Options.get("VectorLikeClasses", "::std::vector"))),
       EnableProto(Options.getLocalOrGlobal("EnableProto", false)) {}
void InefficientVectorOperationCheck::addMatcher(
   //   for (const auto& E : data) {
   //     v.push_back(...);
   //     // Or: proto.add_xxx(...);
   //   }
   //
   // 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);
 }
 
 void InefficientVectorOperationCheck::registerMatchers(MatchFinder *Finder) {
   const auto VectorDecl = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 5>(
       VectorLikeClasses.begin(), VectorLikeClasses.end())));

clang-tools-extra/docs/ReleaseNotes.rst

 
 - New alias :doc:`cppcoreguidelines-macro-to-enum
   <clang-tidy/checks/cppcoreguidelines-macro-to-enum>` to :doc:`modernize-macro-to-enum
   <clang-tidy/checks/modernize-macro-to-enum>` was added.
 
 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
 
 - Fixed a crash in :doc:`readability-const-return-type
   <clang-tidy/checks/readability-const-return-type>` when a pure virtual function
   overrided has a const return type. Removed the fix for a virtual function.
 
 - Fixed a false positive in :doc:`misc-redundant-expression <clang-tidy/checks/misc-redundant-expression>`

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

public:
 
   void push_back(const T& val);
 
   template <class... Args> void emplace_back(Args &&... args);
 
   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);
 
   const_iterator begin() const;
   const_iterator end() const;
 };
 } // namespace std
 
void f(std::vector<int>& t) {
   {
     FooProto foo;
     // CHECK-FIXES-NOT: foo.mutable_z()->Reserve(5);
     for (int i = 0; i < 5; i++) {
       foo.add_z();
     }
   }
 }
+
+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);
+  }
+}