[clang-tidy] Better custom class support for performance-inefficient-vector-operation

[njames93]

Summary

Adds options to the check for customising the vector like classes the check will work on and the range-for types supported. Enables you to specify classes with methods that aren't named `push_back, emplace_back, reserve and size`

[njames93]

• New line and undo clang format artefact

[njames93]

• Added check if size() method is const for range loops
• Optimised FixItHint creation

[njames93]

• Moved const size method check to matcher

[hokein]

Thanks for the patch. May I know the motivation of this patch? Personally, I'd be conservative of exposing too many options.

[njames93]

In D74689#1879065, @hokein wrote:

Thanks for the patch. May I know the motivation of this patch? Personally, I'd be conservative of exposing too many options.

I was using custom classes with a different naming convention which was not being picked up by this check. Right now this check is only good for classes that mimick the stl conventions.

[njames93]

• Added support for converting loops over c style arrays

[njames93]

• Small nit

[njames93]

• Better template support
• Removed excess code
• Refactor alot of the check

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

public:
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
 private:
   void AddMatcher(const ast_matchers::DeclarationMatcher &TargetRecordDecl,
                   StringRef VarDeclName, StringRef VarDeclStmtName,
                   const ast_matchers::DeclarationMatcher &AppendMethodDecl,
                   StringRef AppendCallName, ast_matchers::MatchFinder *Finder);
-  const std::vector<std::string> VectorLikeClasses;
+  const std::string VectorLikeClasses;
+  const std::string AppendNames;
+  const std::string ReserveNames;
+  const std::string SupportedRanges;
+  const std::string SizeNames;
 
   // If true, also check inefficient operations for proto repeated fields.
   bool EnableProto;
 };
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENT_VECTOR_OPERATION_H

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

 //===--- InefficientVectorOperationCheck.cpp - clang-tidy------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "InefficientVectorOperationCheck.h"
+
+#include "../utils/DeclRefExprUtils.h"
+#include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
-#include "../utils/DeclRefExprUtils.h"
-#include "../utils/OptionsUtils.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 namespace {

 // If EnableProto, the proto related names are bound to the following parts:
 //   - ProtoVarDeclName: 'p' (as VarDecl).
 //   - ProtoVarDeclStmtName: The entire 'SomeProto p;' statement (as DeclStmt).
 //   - ProtoAddFieldCallName: 'p.add_xxx(i)' (as cxxMemberCallExpr).
 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 VectorReserveName[] = "vector_reserve_name";
 static const char PushBackOrEmplaceBackCallName[] = "append_call";
 static const char ProtoVarDeclName[] = "proto_var_decl";
 static const char ProtoVarDeclStmtName[] = "proto_var_decl_stmt";
 static const char ProtoAddFieldCallName[] = "proto_add_field";
 static const char LoopInitVarName[] = "loop_init_var";
 static const char LoopEndExprName[] = "loop_end_expr";
 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")));
+static const char RangeLoopArrayName[] = "for_range_array_loop";
+static const char RangeLoopSizeMethod[] = "for_range_size_name";
+static const char SupportedDefaultRanges[] = "::std::vector;"
+                                             "::std::set;"
+                                             "::std::unordered_set;"
+                                             "::std::map;"
+                                             "::std::unordered_map;"
+                                             "::std::array;"
+                                             "::std::deque";
+
+static ast_matchers::internal::Matcher<NamedDecl>
+hasAnyNameStdString(std::vector<std::string> Names) {
+  return ast_matchers::internal::Matcher<NamedDecl>(
+      new ast_matchers::internal::HasNameMatcher(std::move(Names)));
+}
+
+static std::vector<std::string> parseStringListPair(StringRef LHS,
+                                                    StringRef RHS) {
+  if (LHS.empty()) {
+    if (RHS.empty())
+      return {};
+    return utils::options::parseStringList(RHS);
+  }
+  if (RHS.empty())
+    return utils::options::parseStringList(LHS);
+  llvm::SmallString<512> Buffer;
+  return utils::options::parseStringList((LHS + ";" + RHS).toStringRef(Buffer));
+}
+
+static ast_matchers::internal::Matcher<NamedDecl>
+hasAnyNameListPair(StringRef LHS, StringRef RHS) {
+  return hasAnyNameStdString(parseStringListPair(LHS, RHS));
 }
-
 } // namespace
 
 InefficientVectorOperationCheck::InefficientVectorOperationCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      VectorLikeClasses(utils::options::parseStringList(
-          Options.get("VectorLikeClasses", "::std::vector"))),
+      VectorLikeClasses(Options.get("VectorLikeClasses", "")),
+      AppendNames(Options.get("AppendNames", "")),
+      ReserveNames(Options.get("ReserveNames", "")),
+      SupportedRanges(Options.get("SupportedRanges", "")),
+      SizeNames(Options.get("SizeNames", "")),
       EnableProto(Options.getLocalOrGlobal("EnableProto", false)) {}
 
 void InefficientVectorOperationCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "VectorLikeClasses",
-                utils::options::serializeStringList(VectorLikeClasses));
+  Options.store(Opts, "VectorLikeClasses", VectorLikeClasses);
+  Options.store(Opts, "AppendNames", AppendNames);
+  Options.store(Opts, "ReserveNames", ReserveNames);
+  Options.store(Opts, "SupportedRanges", SupportedRanges);
+  Options.store(Opts, "SizeNames", SizeNames);
   Options.store(Opts, "EnableProto", EnableProto);
 }
 
 void InefficientVectorOperationCheck::AddMatcher(
     const DeclarationMatcher &TargetRecordDecl, StringRef VarDeclName,
     StringRef VarDeclStmtName, const DeclarationMatcher &AppendMethodDecl,
     StringRef AppendCallName, MatchFinder *Finder) {
   const auto DefaultConstructorCall = cxxConstructExpr(
void InefficientVectorOperationCheck::AddMatcher(
 
   // Match for-range loops:
   //   for (const auto& E : data) {
   //     v.push_back(...);
   //     // Or: proto.add_xxx(...);
   //   }
   //
   // FIXME: Support more complex range-expressions.
+
+  auto RangeMatcher = [&](bool CheckConst) {
+    return cxxForRangeStmt(
+               hasRangeInit(declRefExpr(
+                   (CheckConst ? expr(hasType(isConstQualified()))
+                               : expr(unless(hasType(isConstQualified())))),
+                   hasType(cxxRecordDecl(
+                       hasAnyNameListPair(SupportedDefaultRanges,
+                                          SupportedRanges),
+                       hasMethod(
+                           cxxMethodDecl(hasAnyNameListPair("size", SizeNames),
+                                         parameterCountIs(0), isPublic(),
+                                         (CheckConst ? isConst() : anything()),
+                                         returns(isInteger()))
+                               .bind(RangeLoopSizeMethod)))))),
+               HasInterestingLoopBody, InInterestingCompoundStmt)
+        .bind(RangeLoopName);
+  };
+  Finder->addMatcher(RangeMatcher(true), this);
+  Finder->addMatcher(RangeMatcher(false), this);
   Finder->addMatcher(
-      cxxForRangeStmt(
-          hasRangeInit(declRefExpr(supportedContainerTypesMatcher())),
+      cxxForRangeStmt(hasRangeInit(declRefExpr(hasType(arrayType()))),
+                      unless(isInTemplateInstantiation()),
                       HasInterestingLoopBody, InInterestingCompoundStmt)
-          .bind(RangeLoopName),
+          .bind(RangeLoopArrayName),
       this);
 }
 
 void InefficientVectorOperationCheck::registerMatchers(MatchFinder *Finder) {
-  const auto VectorDecl = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 5>(
-      VectorLikeClasses.begin(), VectorLikeClasses.end())));
+
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  const auto VectorDecl = cxxRecordDecl(
+      hasAnyNameListPair("::std::vector", VectorLikeClasses),
+      hasMethod(cxxMethodDecl(hasAnyNameListPair("reserve", ReserveNames),
+                              isPublic(), parameterCountIs(1),
+                              hasParameter(0, hasType(isInteger())))
+                    .bind(VectorReserveName)));
   const auto AppendMethodDecl =
-      cxxMethodDecl(hasAnyName("push_back", "emplace_back"));
+      cxxMethodDecl(hasAnyNameListPair("push_back;emplace_back", AppendNames));
   AddMatcher(VectorDecl, VectorVarDeclName, VectorVarDeclStmtName,
              AppendMethodDecl, PushBackOrEmplaceBackCallName, Finder);
 
   if (EnableProto) {
     const auto ProtoDecl =
         cxxRecordDecl(isDerivedFrom("::proto2::MessageLite"));
 
     // A method's name starts with "add_" might not mean it's an add field
if (Context->getDiagnostics().hasUncompilableErrorOccurred())
     return;
 
   const SourceManager &SM = *Result.SourceManager;
   const auto *VectorVarDecl =
       Result.Nodes.getNodeAs<VarDecl>(VectorVarDeclName);
   const auto *ForLoop = Result.Nodes.getNodeAs<ForStmt>(LoopCounterName);
   const auto *RangeLoop =
       Result.Nodes.getNodeAs<CXXForRangeStmt>(RangeLoopName);
+  const auto *RangeLoopArray =
+      Result.Nodes.getNodeAs<CXXForRangeStmt>(RangeLoopArrayName);
   const auto *VectorAppendCall =
       Result.Nodes.getNodeAs<CXXMemberCallExpr>(PushBackOrEmplaceBackCallName);
   const auto *ProtoVarDecl = Result.Nodes.getNodeAs<VarDecl>(ProtoVarDeclName);
   const auto *ProtoAddFieldCall =
       Result.Nodes.getNodeAs<CXXMemberCallExpr>(ProtoAddFieldCallName);
   const auto *LoopEndExpr = Result.Nodes.getNodeAs<Expr>(LoopEndExprName);
   const auto *LoopParent = Result.Nodes.getNodeAs<CompoundStmt>(LoopParentName);
 
   const CXXMemberCallExpr *AppendCall =
       VectorAppendCall ? VectorAppendCall : ProtoAddFieldCall;
   assert(AppendCall && "no append call expression");
 
   const Stmt *LoopStmt = ForLoop;
   if (!LoopStmt)
     LoopStmt = RangeLoop;
+  if (!LoopStmt)
+    LoopStmt = RangeLoopArray;
 
   const auto *TargetVarDecl = VectorVarDecl;
   if (!TargetVarDecl)
     TargetVarDecl = ProtoVarDecl;
 
   llvm::SmallPtrSet<const DeclRefExpr *, 16> AllVarRefs =
       utils::decl_ref_expr::allDeclRefExprs(*TargetVarDecl, *LoopParent,
                                             *Context);
   for (const auto *Ref : AllVarRefs) {
     // Skip cases where there are usages (defined as DeclRefExpr that refers
     // to "v") of vector variable / proto 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(),
                                      LoopStmt->getBeginLoc())) {
       return;
     }
   }
 
-  std::string PartialReserveStmt;
+  SmallString<64> Buffer;
+  llvm::raw_svector_ostream FixIt(Buffer);
+
+  // Add the name to the FixIt insertion
+  FixIt << Lexer::getSourceText(
+      CharSourceRange::getTokenRange(
+          AppendCall->getImplicitObjectArgument()->getSourceRange()),
+      SM, Context->getLangOpts());
+
+  // Add the start of the reserve to the FixIt insertion
+  // <Name>.reserve(
   if (VectorAppendCall != nullptr) {
-    PartialReserveStmt = ".reserve";
+    const auto *ReserveMethod =
+        Result.Nodes.getNodeAs<CXXMethodDecl>(VectorReserveName);
+    assert(ReserveMethod && "Reserve Method should not be null");
+    FixIt << '.' << ReserveMethod->getName() << '(';
   } else {
     llvm::StringRef FieldName = ProtoAddFieldCall->getMethodDecl()->getName();
+    assert(FieldName.startswith("add_") &&
+           "Method call didn't start with 'add_'");
     FieldName.consume_front("add_");
-    std::string MutableFieldName = ("mutable_" + FieldName).str();
-    PartialReserveStmt = "." + MutableFieldName +
-                         "()->Reserve"; // e.g., ".mutable_xxx()->Reserve"
+    FixIt << ".mutable_" << FieldName
+          << "()->Reserve("; // e.g., ".mutable_xxx()->Reserve("
   }
 
-  llvm::StringRef VarName = Lexer::getSourceText(
-      CharSourceRange::getTokenRange(
-          AppendCall->getImplicitObjectArgument()->getSourceRange()),
-      SM, Context->getLangOpts());
-
-  std::string ReserveSize;
+  // Add the Reserve size expression to the FixIt
+  // <Name.reserve(>AmountToReserve
   // Handle for-range loop cases.
   if (RangeLoop) {
     // Get the range-expression in a for-range statement represented as
     // `for (range-declarator: range-expression)`.
     StringRef RangeInitExpName =
         Lexer::getSourceText(CharSourceRange::getTokenRange(
                                  RangeLoop->getRangeInit()->getSourceRange()),
                              SM, Context->getLangOpts());
-    ReserveSize = (RangeInitExpName + ".size()").str();
+    const auto *SizeMethod =
+        Result.Nodes.getNodeAs<CXXMethodDecl>(RangeLoopSizeMethod);
+    assert(SizeMethod &&
+           "SizeMethod should be valid in a matched RangeForLoop");
+    FixIt << RangeInitExpName << '.' << SizeMethod->getName() << "()";
   } else if (ForLoop) {
     // Handle counter-based loop cases.
     StringRef LoopEndSource = Lexer::getSourceText(
         CharSourceRange::getTokenRange(LoopEndExpr->getSourceRange()), SM,
         Context->getLangOpts());
-    ReserveSize = std::string(LoopEndSource);
+    FixIt << LoopEndSource;
+  } else if (RangeLoopArray) {
+    const auto *Array = dyn_cast<ArrayType>(
+        RangeLoopArray->getRangeInit()->getType().getTypePtr());
+    assert(Array && "Array should not be null in a RangeLoopArray");
+    if (const auto *CArray = dyn_cast<ConstantArrayType>(Array))
+      FixIt << CArray->getSize();
+    else if (const auto *DArray = dyn_cast<DependentSizedArrayType>(Array))
+      FixIt << Lexer::getSourceText(
+          CharSourceRange::getTokenRange(
+              DArray->getSizeExpr()->getSourceRange()),
+          SM, Context->getLangOpts());
+    else if (const auto *VArray = dyn_cast<VariableArrayType>(Array)) {
+      const Expr *SizeExpr = VArray->getSizeExpr();
+      if (SizeExpr->HasSideEffects(*Context)) {
+        StringRef RangeInitExpName = Lexer::getSourceText(
+            CharSourceRange::getTokenRange(
+                RangeLoopArray->getRangeInit()->getSourceRange()),
+            SM, Context->getLangOpts());
+        FixIt << "sizeof(" << RangeInitExpName << ") / sizeof("
+              << RangeInitExpName << "[0])";
+      } else {
+        FixIt << Lexer::getSourceText(
+            CharSourceRange::getTokenRange(
+                VArray->getSizeExpr()->getSourceRange()),
+            SM, Context->getLangOpts());
       }
+    } else
+      // IncompleteArrayType can't get the size
+      return;
+  } else
+    llvm_unreachable("Unknown loop");
 
-  auto Diag = diag(AppendCall->getBeginLoc(),
-                   "%0 is called inside a loop; consider pre-allocating the "
-                   "container capacity before the loop")
-              << AppendCall->getMethodDecl()->getDeclName();
-  if (!ReserveSize.empty()) {
-    std::string ReserveStmt =
-        (VarName + PartialReserveStmt + "(" + ReserveSize + ");\n").str();
-    Diag << FixItHint::CreateInsertion(LoopStmt->getBeginLoc(), ReserveStmt);
-  }
+  FixIt << ");\n" << Lexer::getIndentationForLine(LoopStmt->getBeginLoc(), SM);
+
+  auto Diag =
+      diag(AppendCall->getBeginLoc(),
+      << AppendCall->getMethodDecl()->getDeclName()
+      << FixItHint::CreateInsertion(LoopStmt->getBeginLoc(), FixIt.str());
 }
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

 - New alias :doc:`cert-dcl51-cpp
   <clang-tidy/checks/cert-dcl51-cpp>` to
   :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone-reserved-identifier>` was added.
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Improved :doc:`performance-inefficient-vector-operation
+  <clang-tidy/checks/performance-inefficient-vector-operation>` check now has
+  extra options for custom containers.
+
 - Improved :doc:`readability-qualified-auto
   <clang-tidy/checks/readability-qualified-auto>` check now supports a 
   `AddConstToQualified` to enable adding ``const`` qualifiers to variables
   typed with ``auto *`` and ``auto &``.
 
 - Improved :doc:`readability-redundant-string-init
   <clang-tidy/checks/readability-redundant-string-init>` check now supports a
   `StringNames` option enabling its application to custom string classes. The

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

.. code-block:: c++
   for (int i = 0; i < n; ++i) {
     p.add_xxx(n);
     // This will trigger the warning since the add_xxx may cause multiple memory
     // relloacations. This can be avoid by inserting a
     // 'p.mutable_xxx().Reserve(n)' statement before the for statement.
   }
 
 * For-range loops like ``for (range-declaration : range_expression)``, the type
-  of ``range_expression`` can be ``std::vector``, ``std::array``,
-  ``std::deque``, ``std::set``, ``std::unordered_set``, ``std::map``,
-  ``std::unordered_set``:
+  of ``range_expression`` are specified in :option:`SupportedRanges`:
 
 .. code-block:: c++
 
   std::vector<int> data;
   std::vector<int> v;
 
   for (auto element : data) {
     v.push_back(element);
     // This will trigger the warning since the 'push_back' may cause multiple
     // memory reallocations in v. This can be avoid by inserting a
     // 'reserve(data.size())' statement before the for statement.
   }
 
 
 Options
 -------
 
 .. option:: VectorLikeClasses
 
    Semicolon-separated list of names of vector-like classes. By default only
    ``::std::vector`` is considered.
 
+.. option:: AppendNames
+
+   Semicolon-separated list of names of append methods in :option:`VectorLikeClasses`. 
+   By default only ``push_back`` and ``emplace_back`` are considered.
+
+.. option:: ReserveNames
+
+   Semicolon-separated list of names of reserve methods in :option:`VectorLikeClasses`. 
+   By default only ``reserve`` is considered.
+
+   Note the method must have `1` parameter that is of integral type.
+
+.. option:: SupportedRanges
+
+   Semicolon-separated list of names of Range types for for-range expressions. 
+   By default only ``std::vector``, ``std::array``, ``std::deque``, 
+   ``std::set``, ``std::unordered_set``, ``std::map`` and ``std::unordered_set``
+   are considered.
+
+.. option:: SizeNames
+
+   Semicolon-separated list of names of size methods :option:`SupportedRanges`.  
+   By default only ``size`` is considered.
+
+   Note the method must have `0` parameters and return an integral type.
+
 .. option:: EnableProto
 
    When non-zero, the check will also warn on inefficient operations for proto
    repeated fields. Otherwise, the check only warns on inefficient vector
    operations. Default is `0`.

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

 // RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- \
-// RUN: -format-style=llvm \
 // RUN: -config='{CheckOptions: \
-// RUN:  [{key: performance-inefficient-vector-operation.EnableProto, value: 1}]}'
+// RUN:  [{key: performance-inefficient-vector-operation.EnableProto, value: 1}, \
+// RUN:   {key: performance-inefficient-vector-operation.VectorLikeClasses, value : MyContainer}, \
+// RUN:   {key: performance-inefficient-vector-operation.SupportedRanges, value : MyContainer}, \
+// RUN:   {key: performance-inefficient-vector-operation.ReserveNames, value : Reserve}, \
+// RUN:   {key: performance-inefficient-vector-operation.AppendNames, value : PushBack}, \
+// RUN:   {key: performance-inefficient-vector-operation.SizeNames, value : Size}, ]}'
 
 namespace std {
 
 typedef int size_t;
 
 template<class E> class initializer_list {
 public:
   using value_type = E;
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();
     }
   }
 }
+
+namespace OptionsValidMatchDefault {
+template <typename T>
+class MyContainer {
+public:
+  unsigned size() const;
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+  MyContainer<int> CC1;
+  // CHECK-FIXES: {{^}}  CC1.reserve(C.size());
+  for (auto I : C) {
+    CC1.push_back(I);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace OptionsValidMatchDefault
+
+namespace OptionsValidMatchDifferentMethods {
+template <typename T>
+class MyContainer {
+public:
+  unsigned Size() const;
+  T *begin() const;
+  T *end() const;
+  void PushBack(const T &);
+  void Reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+  MyContainer<int> CC2;
+  // CHECK-FIXES: {{^}}  CC2.Reserve(C.Size());
+  for (auto I : C) {
+    CC2.PushBack(I);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'PushBack' is called
+  }
+}
+} // namespace OptionsValidMatchDifferentMethods
+
+namespace UnknownContainer {
+template <typename T>
+class MyUContainer {
+public:
+  unsigned size() const;
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyUContainer<int> &C) {
+  // MyUContainer isn't specified as a VectorLikeClass in the Config Options
+  MyUContainer<int> CC3;
+  // CHECK-FIXES-NOT: {{^}}  CC3.reserve(C.size());
+  for (auto I : C) {
+    CC3.push_back(I);
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace UnknownContainer
+
+namespace PrivateMethods {
+namespace Size {
+template <typename T>
+class MyContainer {
+  unsigned size() const;
+
+public:
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+  // MyContainer<int>::size is private, so calling it will be invalid
+  MyContainer<int> CC4;
+  // CHECK-FIXES-NOT: {{^}}  CC4.reserve(C.size());
+  for (auto I : C) {
+    CC4.push_back(I);
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Size
+namespace Reserve {
+template <typename T>
+class MyContainer {
+public:
+  unsigned size() const;
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+
+private:
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+  // MyContainer<int>::reserve is private, so calling it will be invalid
+  MyContainer<int> CC5;
+  // CHECK-FIXES-NOT: {{^}}  CC5.reserve(C.size());
+  for (auto I : C) {
+    CC5.push_back(I);
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Reserve
+} // namespace PrivateMethods
+
+namespace BadSignatures {
+namespace Size {
+template <typename T>
+class MyContainer {
+public:
+  char *size() const;
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+  // MyContainer<int>::size doesn't return an integral type(char *), so ignore this class
+  MyContainer<int> CC6;
+  // CHECK-FIXES-NOT: {{^}}  CC6.reserve(C.size());
+  for (auto I : C) {
+    CC6.push_back(I);
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Size
+namespace Reserve {
+template <typename T>
+class MyContainer {
+public:
+  unsigned size() const;
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve();
+};
+
+void foo(const MyContainer<int> &C) {
+  // MyContainer<int>::reserve doesn't take a single integral argument, so ignore this class
+  MyContainer<int> CC7;
+  // CHECK-FIXES-NOT: {{^}}  CC7.reserve(C.size());
+  for (auto I : C) {
+    CC7.push_back(I);
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Reserve
+} // namespace BadSignatures
+
+namespace NonConstSizeMethod {
+template <typename T>
+class MyContainer {
+public:
+  unsigned size();
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer<int> &C) {
+  // MyContainer<int>::size() isn't const, therefore we cant call C.size() as
+  // C is passed by const ref.
+  MyContainer<int> CC8;
+  // CHECK-FIXES-NOT: {{^}}  CC8.reserve(C.size());
+  for (auto I : C) {
+    CC8.push_back(I);
+    // CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+
+void bar(MyContainer<int> &C) {
+  // MyContainer<int>::size() isn't const, but C is passed by non-const ref so
+  // we can still call it.
+  MyContainer<int> CC9;
+  // CHECK-FIXES: {{^}}  CC9.reserve(C.size());
+  for (auto I : C) {
+    CC9.push_back(I);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace NonConstSizeMethod
+
+namespace ArrayForRange {
+int getSize();
+void foo() {
+  {
+    std::vector<int> A1;
+    int Arr[] = {1, 2, 3};
+    // CHECK-FIXES: {{^}}    A1.reserve(3);
+    for (auto X : Arr) {
+      A1.push_back(X);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+    }
+  }
+  {
+    std::vector<int> A2;
+    int Arr[3];
+    // CHECK-FIXES: {{^}}    A2.reserve(3);
+    for (auto X : Arr) {
+      A2.push_back(X);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+    }
+  }
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wvla-extension"
+  {
+    std::vector<int> A3;
+    int Size = getSize();
+    int Arr[Size];
+    // CHECK-FIXES: {{^}}    A3.reserve(Size);
+    for (auto X : Arr) {
+      A3.push_back(X);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+    }
+  }
+  {
+    std::vector<int> A4;
+    int Arr[getSize()];
+    // CHECK-FIXES: {{^}}    A4.reserve(sizeof(Arr) / sizeof(Arr[0]));
+    for (auto X : Arr) {
+      A4.push_back(X);
+      // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
+    }
+  }
+#pragma clang diagnostic pop
+}
+
+template <unsigned N>
+void foo() {
+  std::vector<int> A5;
+  int TArr[N];
+  // CHECK-FIXES: {{^}}  A5.reserve(N);
+  // FIXME for (auto x : TArr) doesn't get resolved as a call to push_back
+  for (int x : TArr) {
+    A5.push_back(x);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+
+void b() { foo<4>(); } // Ensure this doesn't try to insert A2.reserve(4) in template definition
+} // namespace ArrayForRange
 No newline at end of file