[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

 class InefficientVectorOperationCheck : public ClangTidyCheck {
 public:
   InefficientVectorOperationCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
 private:
-  void AddMatcher(const ast_matchers::DeclarationMatcher &TargetRecordDecl,
+  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 {

 //   SomeProto p;
 //   for (int i = 0; i < 10 + 1; ++i) {
 //     p.add_xxx(i);
 //   }
 // }
 // \endcode
 //
 // The matcher names are bound to following parts of the AST:
-//   - LoopCounterName: The entire for loop (as ForStmt).
 //   - LoopParentName: The body of function f (as CompoundStmt).
-//   - VectorVarDeclName: 'v' (as VarDecl).
-//   - VectorVarDeclStmatName: The entire 'std::vector<T> v;' statement (as
-//     DeclStmt).
-//   - PushBackOrEmplaceBackCallName: 'v.push_back(i)' (as cxxMemberCallExpr).
+//   - VarDeclName: 'v' (as VarDecl).
+//   - VarDeclStmtName: The entire 'std::vector<T> v;' or
+//     'SomeProto p;' statement (as DeclStmt).
+//   - AppendMethodName: 'v.push_back(i), p.add_xxx(i)' (as CallExpr).
+//   - VectorReserveName: The name of the reserve method in the VectorClass
+//     e.g `reserve(size_type amountToReserve);`. If this isn't matched then we
+//     are dealing with a Proto.
+// If A ForLoop is matched, the related names are bound to the following parts:
+//   - LoopCounterName: The entire for loop (as ForStmt).
 //   - LoopInitVarName: 'i' (as VarDecl).
-//   - LoopEndExpr: '10+1' (as Expr).
-// 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";
+//   - LoopEndExprName: '10+1' (as Expr).
+// If A RangeFor is matched, the related named are bound to the following parts:
+//   - RangeLoopName: The entire for range loop (as CXXForRangeStmt).
+//   - RangeLoopSizeMethodName: The name of the size method in the VectorClass
+//     e.g. `size_type size() const;`. If this isn't matched then we are dealing
+//     with a range over a c style array.
 static const char LoopParentName[] = "loop_parent";
-static const char VectorVarDeclName[] = "vector_var_decl";
-static const char VectorVarDeclStmtName[] = "vector_var_decl_stmt";
-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 VarDeclName[] = "var_decl";
+static const char VarDeclStmtName[] = "var_decl_stmt";
+static const char AppendMethodName[] = "append_call";
+static const char VectorReserveName[] = "vector_reserve_name";
+static const char LoopCounterName[] = "for_loop_counter";
 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 RangeLoopSizeMethodName[] = "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)));
+}
+
+AST_MATCHER_P(FunctionTemplateDecl, hasTemplatedDecl,
+              ast_matchers::internal::Matcher<FunctionDecl>, InnerMatcher) {
+  FunctionDecl *Decl = Node.getTemplatedDecl();
+  return Decl && InnerMatcher.matches(*Decl, Finder, Builder);
+}
+
+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));
+}
+struct MemberCallData {
+  const Expr *Base;
+  DeclarationName Name;
+  SourceLocation Location;
+};
+
+static llvm::Optional<MemberCallData> getMemberCallData(const CallExpr *Call) {
+  assert(Call && "no append call expression");
+  if (const auto *Cxx = dyn_cast<CXXMemberCallExpr>(Call))
+    return MemberCallData{Cxx->getImplicitObjectArgument(),
+                          Cxx->getMethodDecl()->getDeclName(),
+                          Call->getBeginLoc()};
+  const auto *Callee = Call->getCallee();
+  if (const auto *Mem = dyn_cast<MemberExpr>(Callee))
+    return MemberCallData{Mem->getBase(), Mem->getMemberDecl()->getDeclName(),
+                          Call->getBeginLoc()};
+  if (const auto *Unres = dyn_cast<UnresolvedMemberExpr>(Callee))
+    return MemberCallData{Unres->getBase(), Unres->getName(),
+                          Call->getBeginLoc()};
+  return None;
 }
-
 } // 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(
+void InefficientVectorOperationCheck::addMatcher(
     const DeclarationMatcher &TargetRecordDecl, StringRef VarDeclName,
     StringRef VarDeclStmtName, const DeclarationMatcher &AppendMethodDecl,
     StringRef AppendCallName, MatchFinder *Finder) {
   const auto DefaultConstructorCall = cxxConstructExpr(
       hasType(TargetRecordDecl),
       hasDeclaration(cxxConstructorDecl(isDefaultConstructor())));
   const auto TargetVarDecl =
       varDecl(hasInitializer(DefaultConstructorCall)).bind(VarDeclName);
   const auto TargetVarDefStmt =
       declStmt(hasSingleDecl(equalsBoundNode(std::string(VarDeclName))))
           .bind(VarDeclStmtName);
 
+  // this handles unresolved functions that are templates(e.g. emplace_back)
+  const auto TemplateResolved =
+      decl(anyOf(AppendMethodDecl,
+                 functionTemplateDecl(hasTemplatedDecl(AppendMethodDecl))));
+
   const auto AppendCallExpr =
       cxxMemberCallExpr(
-          callee(AppendMethodDecl), on(hasType(TargetRecordDecl)),
+          callee(TemplateResolved), on(hasType(TargetRecordDecl)),
           onImplicitObjectArgument(declRefExpr(to(TargetVarDecl))))
           .bind(AppendCallName);
-  const auto AppendCall = expr(ignoringImplicit(AppendCallExpr));
+
+  // In templates sometimes the call expr can't be resolved to a
+  // CXXMemberCallExpr so handle these edge cases
+  const auto MemberCallExpr =
+      callExpr(callee(memberExpr(
+                   hasDeclaration(TemplateResolved),
+                   hasObjectExpression(hasType(TargetRecordDecl)),
+                   hasObjectExpression(declRefExpr(to(TargetVarDecl))))))
+          .bind(AppendCallName);
+  const auto UnresolvedCallExpr =
+      callExpr(callee(unresolvedMemberExpr(
+                   hasAnyDeclaration(TemplateResolved),
+                   hasObjectExpression(hasType(TargetRecordDecl)),
+                   hasObjectExpression(declRefExpr(to(TargetVarDecl))))))
+          .bind(AppendCallName);
+  const auto AppendCall = expr(ignoringImplicit(
+      anyOf(AppendCallExpr, MemberCallExpr, UnresolvedCallExpr)));
   const auto LoopVarInit =
       declStmt(hasSingleDecl(varDecl(hasInitializer(integerLiteral(equals(0))))
                                  .bind(LoopInitVarName)));
   const auto RefersToLoopVar = ignoringParenImpCasts(
       declRefExpr(to(varDecl(equalsBoundNode(LoopInitVarName)))));
 
   // Matchers for the loop whose body has only 1 push_back/emplace_back calling
   // statement.
Finder->addMatcher(
               hasRHS(expr(unless(hasDescendant(expr(RefersToLoopVar))))
                          .bind(LoopEndExprName)))),
           hasIncrement(unaryOperator(hasOperatorName("++"),
                                      hasUnaryOperand(RefersToLoopVar))),
           HasInterestingLoopBody, InInterestingCompoundStmt)
           .bind(LoopCounterName),
       this);
 
+  const auto SupportedRangeName =
+      hasAnyNameListPair(SupportedDefaultRanges, SupportedRanges);
+  const auto SupportedSizeName = hasAnyNameListPair("size", SizeNames);
+
   // 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(
+                       SupportedRangeName,
+                       hasMethod(
+                           cxxMethodDecl(SupportedSizeName, parameterCountIs(0),
+                                         isPublic(),
+                                         (CheckConst ? isConst() : anything()),
+                                         returns(isInteger()))
+                               .bind(RangeLoopSizeMethodName)))))),
+               HasInterestingLoopBody, InInterestingCompoundStmt)
+        .bind(RangeLoopName);
+  };
+  Finder->addMatcher(RangeMatcher(true), this);
+  Finder->addMatcher(RangeMatcher(false), this);
   Finder->addMatcher(
       cxxForRangeStmt(
-          hasRangeInit(declRefExpr(supportedContainerTypesMatcher())),
-          HasInterestingLoopBody, InInterestingCompoundStmt)
+          hasRangeInit(declRefExpr(hasType(ignoringParens(arrayType())))),
+          unless(isInTemplateInstantiation()), HasInterestingLoopBody,
+          InInterestingCompoundStmt)
           .bind(RangeLoopName),
       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"));
-  AddMatcher(VectorDecl, VectorVarDeclName, VectorVarDeclStmtName,
-             AppendMethodDecl, PushBackOrEmplaceBackCallName, Finder);
+      cxxMethodDecl(hasAnyNameListPair("push_back;emplace_back", AppendNames));
+  addMatcher(VectorDecl, VarDeclName, VarDeclStmtName, AppendMethodDecl,
+             AppendMethodName, 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
     // call; it could be the getter for a proto field of which the name starts
     // with "add_". So we exclude const methods.
     const auto AddFieldMethodDecl =
         cxxMethodDecl(matchesName("::add_"), unless(isConst()));
-    AddMatcher(ProtoDecl, ProtoVarDeclName, ProtoVarDeclStmtName,
-               AddFieldMethodDecl, ProtoAddFieldCallName, Finder);
+    addMatcher(ProtoDecl, VarDeclName, VarDeclStmtName, AddFieldMethodDecl,
+               AppendMethodName, Finder);
   }
 }
 
 void InefficientVectorOperationCheck::check(
     const MatchFinder::MatchResult &Result) {
   auto* Context = Result.Context;
   if (Context->getDiagnostics().hasUncompilableErrorOccurred())
     return;
 
   const SourceManager &SM = *Result.SourceManager;
-  const auto *VectorVarDecl =
-      Result.Nodes.getNodeAs<VarDecl>(VectorVarDeclName);
+  auto GetSourceText = [&](const Stmt *Statement) {
+    return Lexer::getSourceText(
+        CharSourceRange::getTokenRange(Statement->getSourceRange()), SM,
+        Context->getLangOpts());
+  };
+  const auto *VDecl = Result.Nodes.getNodeAs<VarDecl>(VarDeclName);
   const auto *ForLoop = Result.Nodes.getNodeAs<ForStmt>(LoopCounterName);
   const auto *RangeLoop =
       Result.Nodes.getNodeAs<CXXForRangeStmt>(RangeLoopName);
-  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;
-
-  const auto *TargetVarDecl = VectorVarDecl;
-  if (!TargetVarDecl)
-    TargetVarDecl = ProtoVarDecl;
+  auto AppendCallInfo =
+      getMemberCallData(Result.Nodes.getNodeAs<CallExpr>(AppendMethodName));
+
+  if (!AppendCallInfo)
+    return;
+
+  const Stmt *LoopStmt = ForLoop ? cast<Stmt>(ForLoop) : RangeLoop;
+  assert(LoopStmt && "LoopStmt shouldn't be null on a match");
 
   llvm::SmallPtrSet<const DeclRefExpr *, 16> AllVarRefs =
-      utils::decl_ref_expr::allDeclRefExprs(*TargetVarDecl, *LoopParent,
-                                            *Context);
+      utils::decl_ref_expr::allDeclRefExprs(*VDecl, *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;
-  if (VectorAppendCall != nullptr) {
-    PartialReserveStmt = ".reserve";
+  SmallString<64> Buffer;
+  llvm::raw_svector_ostream FixIt(Buffer);
+
+  // Add the name to the FixIt insertion
+  FixIt << GetSourceText(AppendCallInfo->Base);
+
+  // Add the start of the reserve to the FixIt insertion
+  // <Name>.reserve(
+  if (const auto *ReserveMethod =
+          Result.Nodes.getNodeAs<CXXMethodDecl>(VectorReserveName)) {
+    FixIt << '.' << ReserveMethod->getName() << '(';
   } else {
-    llvm::StringRef FieldName = ProtoAddFieldCall->getMethodDecl()->getName();
+    // No found reserve method, must be a Proto method
+    llvm::StringRef FieldName =
+        AppendCallInfo->Name.getAsIdentifierInfo()->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;
-  // Handle for-range loop cases.
+  // Add the Reserve size expression to the FixIt
+  // <Name.reserve(>AmountToReserve
   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();
+    // Handle for-range loop cases.
+    const Expr *RangeInit = RangeLoop->getRangeInit();
+    if (const auto *SizeMethod =
+            Result.Nodes.getNodeAs<CXXMethodDecl>(RangeLoopSizeMethodName)) {
+      FixIt << GetSourceText(RangeInit) << '.' << SizeMethod->getName() << "()";
+    } else if (const auto *Array = dyn_cast<ArrayType>(
+                   RangeInit->getType().IgnoreParens().getTypePtr())) {
+      if (const auto *CArray = dyn_cast<ConstantArrayType>(Array))
+        FixIt << CArray->getSize();
+      else if (const auto *DArray = dyn_cast<DependentSizedArrayType>(Array))
+        FixIt << GetSourceText(DArray->getSizeExpr());
+      else if (const auto *VArray = dyn_cast<VariableArrayType>(Array)) {
+        const Expr *SizeExpr = VArray->getSizeExpr();
+        if (SizeExpr->HasSideEffects(*Context)) {
+          StringRef RangeInitExpName = GetSourceText(RangeInit);
+          FixIt << "sizeof(" << RangeInitExpName << ") / sizeof("
+                << RangeInitExpName << "[0])";
+        } else {
+          FixIt << GetSourceText(VArray->getSizeExpr());
+        }
+      } else
+        // IncompleteArrayType can't get the size
+        return;
+    } else
+      llvm_unreachable("Unknown RangeFor Expression");
   } else if (ForLoop) {
     // Handle counter-based loop cases.
-    StringRef LoopEndSource = Lexer::getSourceText(
-        CharSourceRange::getTokenRange(LoopEndExpr->getSourceRange()), SM,
-        Context->getLangOpts());
-    ReserveSize = std::string(LoopEndSource);
-  }
+    const auto *LoopEndExpr = Result.Nodes.getNodeAs<Expr>(LoopEndExprName);
+    assert(LoopEndExpr && "LoopEndExpr null in matched for loop");
+    FixIt << GetSourceText(LoopEndExpr);
+  } 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(AppendCallInfo->Location,
+      << AppendCallInfo->Name
+      << 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>
+constexpr unsigned Times2 = 2 * N;
+
+template <unsigned N>
+void foo(const int (&Param)[N]) {
+  std::vector<int> A5;
+  int Arr[N];
+  // CHECK-FIXES: {{^}}  A5.reserve(N);
+  for (auto X : Arr) {
+    A5.push_back(X);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+  std::vector<int> A6;
+  int CompArr[Times2<N>];
+  // CHECK-FIXES: {{^}}  A6.reserve(Times2<N>);
+  for (auto X : CompArr) {
+    A6.emplace_back(X);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'emplace_back' is called
+  }
+  std::vector<int> A7;
+  // CHECK-FIXES: {{^}}  A7.reserve(N);
+  for (auto X : Param) {
+    A7.push_back(X);
+    // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+
+void b() {
+  int Arr[4];
+  foo(Arr); // Ensure this doesn't try to insert reserve statements based on
+            // the instantiation value of 4
+}
+} // namespace ArrayForRange
 No newline at end of file