[clang-tidy] Add a clang-tidy check for possible inefficient vector operations

[hokein]

Summary

The "performance-inefficient-vector-operation" check finds vector oprations in
for-loop statements which may cause multiple memory reallocations.

This is the first version, only detects typical for-loop:

std::vector<int> v;
for (int i = 0; i < n; ++i) {
  v.push_back(i);
}

// or

for (int i = 0; i < v2.size(); ++i) {
  v.push_back(v2[i]);
}

We can extend it to handle more cases like for-range loop in the future.

[hokein]

Fix a small nit.

[Eugene.Zelenko]

Isn't such analysis is path-sensitive and should be implemented in Static Analyzer?

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

docs/clang-tidy/checks/performance-inefficient-vector-operation.rst

6 ↗

(On Diff #94356)

I think will be good idea to use std::vector, and enclose it and push_back in ``.

[hokein]

Mention the check in Release note.

[hokein]

In D31757#720351, @Eugene.Zelenko wrote:

Isn't such analysis is path-sensitive and should be implemented in Static Analyzer?

This check is more like a flow-sensitive analysis IMO, which can be supported by clang-tidy.

Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).

Done. I think we could add this tip in the add_new_check.py script, so that developers don't forget about it ;)

[aaron.ballman]

In D31757#721129, @hokein wrote:

In D31757#720351, @Eugene.Zelenko wrote:

Isn't such analysis is path-sensitive and should be implemented in Static Analyzer?

This check is more like a flow-sensitive analysis IMO, which can be supported by clang-tidy.

It can be supported by clang-tidy, but your patch isn't doing it. I think the check would be improved substantially by improving the analysis.

clang-tidy/performance/InefficientVectorOperationCheck.cpp

22 ↗

(On Diff #94498)

Why does this check only focus on vector? Other containers allow you to preallocate space for their elements or lend themselves to inefficient operations.

Also, this should be checking ::std::vector instead.

32 ↗

(On Diff #94498)

This isn't paying attention to what is being reserved.

46–50 ↗

(On Diff #94498)

These conditions seem rather restrictive -- why should you not get the same issue with a range-based for loop?

53–54 ↗

(On Diff #94498)

I'm really not keen on this. It will catch trivial cases, so there is some utility, but this will quickly fall apart with anything past the trivial case.

69–71 ↗

(On Diff #94498)

These asserts should come with string literal messages, though I think they can be removed entirely (I don't think there's a value add here).

74 ↗

(On Diff #94498)

Why the llvm:: qualifier?

80 ↗

(On Diff #94498)

Do not use auto, and the formatting is off.

92 ↗

(On Diff #94498)

Surround push_back with quotes.

test/clang-tidy/performance-inefficient-vector-operation.cpp

53 ↗

(On Diff #94498)

I'd like to see a test where you reserve 5 elements and then loop for 10 elements.

[hokein]

Address review comments.

[hokein]

clang-tidy/performance/InefficientVectorOperationCheck.cpp

22 ↗

(On Diff #94498)

Why does this check only focus on vector? Other containers allow you to preallocate space for their elements or lend themselves to inefficient operations.

Only a few std containers (vector, unordered_map, unorder_set) provide the preallocate ability ("reserve") to users. vector is the most widely used container with these inefficient operations in the real world.

Yes, we could extend this check to support other containers, but it will make the check more complicated (in the future, I plan to support for-range loops and for loops with iterators, and use other elegant fix-it solutions like "range insert" or "range constructor" in some specific cases rather than merely inserting reserve statements). So I think it makes more sense to make this check only focus on vector.

32 ↗

(On Diff #94498)

Good catch. Added a test for it.

46–50 ↗

(On Diff #94498)

Yeah, this is intended. We want to match typical for-loops with counters here. For-range loop will be supported in the future.

53–54 ↗

(On Diff #94498)

The motivation of this check is to find code patterns like for (int i = 0; i < n; ++i) { v.push_back(i); } and clean them in our codebase (we have lots of similar cases). These are all cases we want to support. Using hasParent is a simple and sufficient way to do it IMO.

test/clang-tidy/performance-inefficient-vector-operation.cpp

53 ↗

(On Diff #94498)

Even for this case, the check will also ignore it.

[aaron.ballman]

clang-tidy/performance/InefficientVectorOperationCheck.cpp

53–54 ↗

(On Diff #94498)

I'm not convinced of the utility without implementing this in a more sensitive way. Have you run this across any large code bases and found that it catches issues?

[hokein]

clang-tidy/performance/InefficientVectorOperationCheck.cpp

53–54 ↗

(On Diff #94498)

Yeah, the check catches ~2800 cases (regexp shows ~17,000 total usages) in our internal codebase. And all caught cases are what we are interested in. It would catch more if we support for-range loops and iterator-based for-loops.

[aaron.ballman]

clang-tidy/performance/InefficientVectorOperationCheck.cpp

53–54 ↗

(On Diff #94498)

I wasn't worried about it not triggering often enough, I was worried about it triggering too often because of the lack of sensitivity. If you randomly sample some of those 2800 cases, do they reserve the space in a way that your check isn't catching?

[hokein]

clang-tidy/performance/InefficientVectorOperationCheck.cpp

53–54 ↗

(On Diff #94498)

Ok, I see your concern now, thanks for pointing it out.

I have read through these caught cases. The results look reasonable. Most cases (> 95%) are what we expected, the code pattern is like vector<T> v; for (...) { v.push_back(...); } where the vector definition statement and for-loop statement are consecutive. Another option is to make the check more strict (only detects the consecutive code pattern).

[aaron.ballman]

clang-tidy/performance/InefficientVectorOperationCheck.cpp

53–54 ↗

(On Diff #94498)

Okay, that sounds like it has utility then. Thank you for clarifying!

[hokein]

Improve the way of detecting pre-allocation usage before the loop.

[hokein]

friendly ping @alexfh.

clang-tidy/performance/InefficientVectorOperationCheck.cpp

53–54 ↗

(On Diff #94498)

I improved the way of catching the preallocations before the loop. Could you take a look again?

[aaron.ballman]

clang-tidy/performance/InefficientVectorOperationCheck.cpp

81 ↗

(On Diff #94969)

Perhaps you could support other comparisons, but not attempt to generate a fix-it for them? It seems odd that this would trigger for < but not <=, but I can see why you'd not want to figure out the reserve call for !(foo >= 10).

83 ↗

(On Diff #94969)

Thinking out loud, but, what happens if the loop end expression has some hidden complexity to it? e.g.,

for (int i = 0; i < i + 1; ++i) { // This is a "creative" loop.
  v.push_back(i);
}

84 ↗

(On Diff #94969)

Can you add a test case for post-increment (all of your tests use pre-increment)? Also, count-down loops seem reasonable to support as well, no?

for (int i = 10; i >= 0; --i) {
  v.push_back(i);
}

86–87 ↗

(On Diff #94969)

You should update the documentation to mention that this check only worries about a for loop with a single statement in it. It will be surprising that this code does not trigger the same diagnostic:

for (...) {
  std::cout << i;
  v.push_back(i);
}

95–96 ↗

(On Diff #94969)

We don't usually add this test in to our check calls; why are you adding it here?

98 ↗

(On Diff #94969)

Might as well make this a reference rather than a pointer (simplifies code elsewhere).

100–104 ↗

(On Diff #94969)

Formatting (I would recommend running clang-format over the patch).

109–111 ↗

(On Diff #94969)

I'm not certain what types are being used here. Can you turn AllVectorVarRefs into something with an explicit type so that I can know what Ref's type is?

117 ↗

(On Diff #94969)

identified -> identify

clang-tidy/performance/InefficientVectorOperationCheck.h

20 ↗

(On Diff #94969)

Drop the hyphen in for loops.

docs/clang-tidy/checks/performance-inefficient-vector-operation.rst

7 ↗

(On Diff #94969)

The docs should talk more about the limitations of the check (like how many statements it can contain, etc).

[hokein]

Address review comments.

[hokein]

Thanks for the comments.

clang-tidy/performance/InefficientVectorOperationCheck.cpp

81 ↗

(On Diff #94969)

We could support other comparisons in the future. I think it is a good start to just support < in the first version as < usage happens more frequently than other comparisons.

Added a fixme.

83 ↗

(On Diff #94969)

Good point. It will copy the expr "i+1" to "reserve" parameter. This kind of case should not be existed, but it is safer to filter out this case.

95–96 ↗

(On Diff #94969)

It will prevent some unexpected things happened when the translation unit fails to compile.

We had a few experiences before. We encountered some misbehavior of some checks (e.g. https://reviews.llvm.org/rL294578) with a non-compilable TU, and then we added this statement to avoid the misbehavior.

[aaron.ballman]

clang-tidy/performance/InefficientVectorOperationCheck.cpp

95–96 ↗

(On Diff #94969)

I think that this should be moved up into whatever layer calls check(), because it does not seem like the checks are actually valid if the TU has had a fatal error. However, that doesn't have to be part of this patch.

109–111 ↗

(On Diff #94969)

I may not have been clear -- I don't mean that the variable name should contain type information, I mean that the type should not be automatically deduced. We only use auto when the type is spelled explicitly in the initialization or is otherwise obvious from context (like range-based for loops).

[hokein]

Explicit the type.

clang-tidy/performance/InefficientVectorOperationCheck.cpp

109–111 ↗

(On Diff #94969)

I'd prefer to use auto to initialize AllVectorVarRefExprs, as its type is SmallPtrSet<const DeclRefExpr *, 16>, which is a long and noisy name. Using auto can increases readability here.

[aaron.ballman]

clang-tidy/performance/InefficientVectorOperationCheck.cpp

116 ↗

(On Diff #95269)

Formatting (and you can use auto here once you correct the type above).

109–111 ↗

(On Diff #94969)

It doesn't increase readability when someone reviewing the code has no idea what types are involved and has to ask you for clarification. auto should only be used when the type is obvious from the context or spelling the type is *really* noisy. A single level of template goop is a very small amount of noise.

[hokein]

Address remaining comments.

[aaron.ballman]

LGTM!

[hokein]

@alexfh, I'm submitting the patch now. If you have following comments, I'm happy to address them afterwards.

[chapuni]

Does it work for targeting msvc?
See also; http://bb.pgr.jp/builders/test-clang-tools-msc-x64-on-i686-linux-RA/builds/1043

Appeased in r300545. Would it be happier if it worked with "-target x86_64-win32"?

[hokein]

@chapuni, thanks for the workaround. It makes sense to make it work for targeting msvc. Will fix it in a follow-up.

[alexfh]

Awesome, thanks! A few late comments inline.

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

56

It might make sense to make the list of types configurable to support custom vector-like types.

65

Loops with emplace_back would suffer from the same issue.

131

No need to namespace-qualify StringRef and Lexer.

133

Actual LangOptions should be used instead of a default-constructed instance. Same below.

143

Please remove the trailing period.

clang-tools-extra/trunk/clang-tidy/performance/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyPerformanceModule
   FasterStringFindCheck.cpp
   ForRangeCopyCheck.cpp
   ImplicitCastInLoopCheck.cpp
   InefficientStringConcatenationCheck.cpp
+  InefficientVectorOperationCheck.cpp
   PerformanceTidyModule.cpp
   TypePromotionInMathFnCheck.cpp
   UnnecessaryCopyInitialization.cpp
   UnnecessaryValueParamCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyUtils
   )

clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.h

+//===--- InefficientVectorOperationCheck.h - clang-tidy----------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENT_VECTOR_OPERATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENT_VECTOR_OPERATION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// Finds possible inefficient `std::vector` operations (e.g. `push_back`) in
+/// for loops that may cause unnecessary memory reallocations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-vector-operation.html
+class InefficientVectorOperationCheck : public ClangTidyCheck {
+public:
+  InefficientVectorOperationCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENT_VECTOR_OPERATION_H

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

+//===--- InefficientVectorOperationCheck.cpp - clang-tidy------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "InefficientVectorOperationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+#include "../utils/DeclRefExprUtils.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+namespace {
+
+// Matcher names. Given the code:
+//
+// \code
+// void f() {
+//   vector<T> v;
+//   for (int i = 0; i < 10 + 1; ++i) {
+//     v.push_back(i);
+//   }
+// }
+// \endcode
+//
+// The matcher names are bound to following parts of the AST:
+//   - LoopName: The entire for loop (as ForStmt).
+//   - LoopParentName: The body of function f (as CompoundStmt).
+//   - VectorVarDeclName: 'v' in  (as VarDecl).
+//   - VectorVarDeclStmatName: The entire 'std::vector<T> v;' statement (as
+//     DeclStmt).
+//   - PushBackCallName: 'v.push_back(i)' (as cxxMemberCallExpr).
+//   - LoopInitVarName: 'i' (as VarDecl).
+//   - LoopEndExpr: '10+1' (as Expr).
+static const char LoopCounterName[] = "for_loop_counter";
+static const char LoopParentName[] = "loop_parent";
+static const char VectorVarDeclName[] = "vector_var_decl";
+static const char VectorVarDeclStmtName[] = "vector_var_decl_stmt";
+static const char PushBackCallName[] = "push_back_call";
+
+static const char LoopInitVarName[] = "loop_init_var";
+static const char LoopEndExprName[] = "loop_end_expr";
+
+} // namespace
+
+void InefficientVectorOperationCheck::registerMatchers(MatchFinder *Finder) {
+  const auto VectorDecl = cxxRecordDecl(hasName("::std::vector"));
+  const auto VectorDefaultConstructorCall = cxxConstructExpr(
+      hasType(VectorDecl),
+      hasDeclaration(cxxConstructorDecl(isDefaultConstructor())));
+  const auto VectorVarDecl =
+      varDecl(hasInitializer(VectorDefaultConstructorCall))
+          .bind(VectorVarDeclName);
+  const auto PushBackCallExpr =
+      cxxMemberCallExpr(
+          callee(cxxMethodDecl(hasName("push_back"))), on(hasType(VectorDecl)),
+          onImplicitObjectArgument(declRefExpr(to(VectorVarDecl))))
+          .bind(PushBackCallName);
+  const auto PushBackCall =
+      expr(anyOf(PushBackCallExpr, exprWithCleanups(has(PushBackCallExpr))));
+  const auto VectorVarDefStmt =
+      declStmt(hasSingleDecl(equalsBoundNode(VectorVarDeclName)))
+          .bind(VectorVarDeclStmtName);
+
+  const auto LoopVarInit =
+      declStmt(hasSingleDecl(varDecl(hasInitializer(integerLiteral(equals(0))))
+                                 .bind(LoopInitVarName)));
+  const auto RefersToLoopVar = ignoringParenImpCasts(
+      declRefExpr(to(varDecl(equalsBoundNode(LoopInitVarName)))));
+
+  // Match counter-based for loops:
+  //  for (int i = 0; i < n; ++i) { v.push_back(...); }
+  //
+  // FIXME: Support more types of counter-based loops like decrement loops.
+  Finder->addMatcher(
+      forStmt(
+          hasLoopInit(LoopVarInit),
+          hasCondition(binaryOperator(
+              hasOperatorName("<"), hasLHS(RefersToLoopVar),
+              hasRHS(expr(unless(hasDescendant(expr(RefersToLoopVar))))
+                         .bind(LoopEndExprName)))),
+          hasIncrement(unaryOperator(hasOperatorName("++"),
+                                     hasUnaryOperand(RefersToLoopVar))),
+          hasBody(anyOf(compoundStmt(statementCountIs(1), has(PushBackCall)),
+                        PushBackCall)),
+          hasParent(compoundStmt(has(VectorVarDefStmt)).bind(LoopParentName)))
+          .bind(LoopCounterName),
+      this);
+}
+
+void InefficientVectorOperationCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (Result.Context->getDiagnostics().hasUncompilableErrorOccurred())
+    return;
+
+  const SourceManager &SM = *Result.SourceManager;
+  const auto *ForLoop = Result.Nodes.getNodeAs<ForStmt>(LoopCounterName);
+  const auto *PushBackCall =
+      Result.Nodes.getNodeAs<CXXMemberCallExpr>(PushBackCallName);
+  const auto *LoopEndExpr = Result.Nodes.getNodeAs<Expr>(LoopEndExprName);
+  const auto *LoopParent = Result.Nodes.getNodeAs<CompoundStmt>(LoopParentName);
+  const auto *VectorVarDecl =
+      Result.Nodes.getNodeAs<VarDecl>(VectorVarDeclName);
+
+  llvm::SmallPtrSet<const DeclRefExpr *, 16> AllVectorVarRefs =
+      utils::decl_ref_expr::allDeclRefExprs(*VectorVarDecl, *LoopParent,
+                                            *Result.Context);
+  for (const auto *Ref : AllVectorVarRefs) {
+    // Skip cases where there are usages (defined as DeclRefExpr that refers to
+    // "v") of vector variable `v` before the for loop. We consider these usages
+    // are operations causing memory preallocation (e.g. "v.resize(n)",
+    // "v.reserve(n)").
+    //
+    // FIXME: make it more intelligent to identify the pre-allocating operations
+    // before the for loop.
+    if (SM.isBeforeInTranslationUnit(Ref->getLocation(),
+                                     ForLoop->getLocStart())) {
+      return;
+    }
+  }
+
+  llvm::StringRef LoopEndSource = clang::Lexer::getSourceText(
+      CharSourceRange::getTokenRange(LoopEndExpr->getSourceRange()), SM,
+      clang::LangOptions());
+  llvm::StringRef VectorVarName = clang::Lexer::getSourceText(
+      CharSourceRange::getTokenRange(
+          PushBackCall->getImplicitObjectArgument()->getSourceRange()),
+      SM, clang::LangOptions());
+  std::string ReserveStmt =
+      (VectorVarName + ".reserve(" + LoopEndSource + ");\n").str();
+
+  diag(PushBackCall->getLocStart(),
+       "'push_back' is called inside a loop; "
+       "consider pre-allocating the vector capacity before the loop.")
+      << FixItHint::CreateInsertion(ForLoop->getLocStart(), ReserveStmt);
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang

clang-tools-extra/trunk/clang-tidy/performance/PerformanceTidyModule.cpp

 //===--- PeformanceTidyModule.cpp - clang-tidy ----------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "FasterStringFindCheck.h"
 #include "ForRangeCopyCheck.h"
 #include "ImplicitCastInLoopCheck.h"
 #include "InefficientStringConcatenationCheck.h"
+#include "InefficientVectorOperationCheck.h"
 #include "TypePromotionInMathFnCheck.h"
 #include "UnnecessaryCopyInitialization.h"
 #include "UnnecessaryValueParamCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 class PerformanceModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<FasterStringFindCheck>(
         "performance-faster-string-find");
     CheckFactories.registerCheck<ForRangeCopyCheck>(
         "performance-for-range-copy");
     CheckFactories.registerCheck<ImplicitCastInLoopCheck>(
         "performance-implicit-cast-in-loop");
     CheckFactories.registerCheck<InefficientStringConcatenationCheck>(
         "performance-inefficient-string-concatenation");
+    CheckFactories.registerCheck<InefficientVectorOperationCheck>(
+        "performance-inefficient-vector-operation");
     CheckFactories.registerCheck<TypePromotionInMathFnCheck>(
         "performance-type-promotion-in-math-fn");
     CheckFactories.registerCheck<UnnecessaryCopyInitialization>(
         "performance-unnecessary-copy-initialization");
     CheckFactories.registerCheck<UnnecessaryValueParamCheck>(
         "performance-unnecessary-value-param");
   }
 };

clang-tools-extra/trunk/docs/ReleaseNotes.rst

 Improvements to clang-tidy
 --------------------------
 
 - New `cert-dcl58-cpp
   <http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl58-cpp.html>`_ check
 
   Finds modification of the ``std`` or ``posix`` namespace.
 
-- Improved `cppcoreguidelines-no-malloc 
+- Improved `cppcoreguidelines-no-malloc
   <http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-no-malloc.html>`_ check
 
   Allow custom memory management functions to be considered as well.
 
 - New `readability-misleading-indentation
   <http://clang.llvm.org/extra/clang-tidy/checks/readability-misleading-indentation.html>`_ check
 
   Finds misleading indentation where braces should be introduced or the code should be reformatted.

 - New `misc-forwarding-reference-overload
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-forwarding-reference-overload.html>`_ check
 
   Finds perfect forwarding constructors that can unintentionally hide copy or move constructors.
 
 - Support clang-formatting of the code around applied fixes (``-format-style``
   command-line option).
 
+- New `performance-inefficient-vector-operation
+  <http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-vector-operation.html>`_ check
+
+  Finds possible inefficient vector operations in for loops that may cause
+  unnecessary memory reallocations.
+
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 
 Improvements to modularize
 --------------------------
 
 The improvements are...

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

.. toctree::
    modernize-use-transparent-functors
    modernize-use-using
    mpi-buffer-deref
    mpi-type-mismatch
    performance-faster-string-find
    performance-for-range-copy
    performance-implicit-cast-in-loop
    performance-inefficient-string-concatenation
+   performance-inefficient-vector-operation
    performance-type-promotion-in-math-fn
    performance-unnecessary-copy-initialization
    performance-unnecessary-value-param
    readability-avoid-const-params-in-decls
    readability-braces-around-statements
    readability-container-size-empty
    readability-delete-null-pointer
    readability-deleted-default

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst

+.. title:: clang-tidy - performance-inefficient-vector-operation
+
+performance-inefficient-vector-operation
+========================================
+
+Finds possible inefficient `std::vector` operations (e.g. `push_back`) that may
+cause unnecessary memory reallocations.
+
+Currently, the check only detects a typical counter-based for loop with a single
+statement in it, see below:
+
+.. code-block:: c++
+
+  std::vector<int> v;
+  for (int i = 0; i < n; ++i) {
+    v.push_back(n);
+    // This will trigger the warning since the push_back may cause multiple
+    // memory reallocations in v. This can be avoid by inserting a 'reserve(n)'
+    // statment before the for statment.
+  }

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

+// RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- -format-style=llvm --
+
+typedef int size_t;
+
+namespace std {
+template <class T>
+class vector {
+ public:
+  typedef T* iterator;
+  typedef const T* const_iterator;
+  typedef T& reference;
+  typedef const T& const_reference;
+  typedef size_t size_type;
+
+  explicit vector();
+  explicit vector(size_type n);
+
+  void push_back(const T& val);
+  void reserve(size_t n);
+  void resize(size_t n);
+
+  size_t size();
+  const_reference operator[] (size_type) const;
+  reference operator[] (size_type);
+};
+} // namespace std
+
+void f(std::vector<int>& t) {
+  {
+    std::vector<int> v;
+    // CHECK-FIXES: v.reserve(10);
+    for (int i = 0; i < 10; ++i)
+      v.push_back(i);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called inside a loop; consider pre-allocating the vector capacity before the loop
+  }
+  {
+    std::vector<int> v;
+    // CHECK-FIXES: v.reserve(10);
+    for (int i = 0; i < 10; i++)
+      v.push_back(i);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+  }
+  {
+    std::vector<int> v;
+    // CHECK-FIXES: v.reserve(10);
+    for (int i = 0; i < 10; ++i)
+      v.push_back(0);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+  }
+  {
+    std::vector<int> v;
+    // CHECK-FIXES: v.reserve(5);
+    for (int i = 0; i < 5; ++i) {
+      v.push_back(i);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+    }
+    // CHECK-FIXES-NOT: v.reserve(10);
+    for (int i = 0; i < 10; ++i) {
+      // No fix for this loop as we encounter the prior loops.
+      v.push_back(i);
+    }
+  }
+  {
+    std::vector<int> v;
+    std::vector<int> v2;
+    v2.reserve(3);
+    // CHECK-FIXES: v.reserve(10);
+    for (int i = 0; i < 10; ++i)
+      v.push_back(i);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+  }
+  {
+    std::vector<int> v;
+    // CHECK-FIXES: v.reserve(t.size());
+    for (size_t i = 0; i < t.size(); ++i) {
+      v.push_back(t[i]);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+    }
+  }
+  {
+    std::vector<int> v;
+    // CHECK-FIXES: v.reserve(t.size() - 1);
+    for (size_t i = 0; i < t.size() - 1; ++i) {
+      v.push_back(t[i]);
+    } // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+  }
+
+  // ---- Non-fixed Cases ----
+  {
+    std::vector<int> v;
+    v.reserve(20);
+    // CHECK-FIXES-NOT: v.reserve(10);
+    // There is a "reserve" call already.
+    for (int i = 0; i < 10; ++i) {
+      v.push_back(i);
+    }
+  }
+  {
+    std::vector<int> v;
+    v.reserve(5);
+    // CHECK-FIXES-NOT: v.reserve(10);
+    // There is a "reserve" call already.
+    for (int i = 0; i < 10; ++i) {
+      v.push_back(i);
+    }
+  }
+  {
+    std::vector<int> v;
+    v.resize(5);
+    // CHECK-FIXES-NOT: v.reserve(10);
+    // There is a ref usage of v before the loop.
+    for (int i = 0; i < 10; ++i) {
+      v.push_back(i);
+    }
+  }
+  {
+    std::vector<int> v;
+    v.push_back(0);
+    // CHECK-FIXES-NOT: v.reserve(10);
+    // There is a ref usage of v before the loop.
+    for (int i = 0; i < 10; ++i) {
+      v.push_back(i);
+    }
+  }
+  {
+    std::vector<int> v;
+    f(v);
+    // CHECK-FIXES-NOT: v.reserve(10);
+    // There is a ref usage of v before the loop.
+    for (int i = 0; i < 10; ++i) {
+      v.push_back(i);
+    }
+  }
+  {
+    std::vector<int> v(20);
+    // CHECK-FIXES-NOT: v.reserve(10);
+    // v is not constructed with default constructor.
+    for (int i = 0; i < 10; ++i) {
+      v.push_back(i);
+    }
+  }
+  {
+    std::vector<int> v;
+    // CHECK-FIXES-NOT: v.reserve(10);
+    // For-loop is not started with 0.
+    for (int i = 1; i < 10; ++i) {
+      v.push_back(i);
+    }
+  }
+  {
+    std::vector<int> v;
+    // CHECK-FIXES-NOT: v.reserve(t.size());
+    // v isn't referenced in for-loop body.
+    for (size_t i = 0; i < t.size(); ++i) {
+      t.push_back(i);
+    }
+  }
+  {
+    std::vector<int> v;
+    int k;
+    // CHECK-FIXES-NOT: v.reserve(10);
+    // For-loop isn't a fixable loop.
+    for (size_t i = 0; k < 10; ++i) {
+      v.push_back(t[i]);
+    }
+  }
+  {
+    std::vector<int> v;
+    // CHECK-FIXES-NOT: v.reserve(i + 1);
+    // The loop end expression refers to the loop variable i.
+    for (int i = 0; i < i + 1; i++)
+      v.push_back(i);
+  }
+  {
+    std::vector<int> v;
+    int k;
+    // CHECK-FIXES-NOT: v.reserve(10);
+    // For-loop isn't a fixable loop.
+    for (size_t i = 0; i < 10; ++k) {
+      v.push_back(t[i]);
+    }
+  }
+}