[clang-tidy] Support detecting for-range loop in inefficient-vector-operation check.

Summary:
Also add an option "VectorLikeClasses" allowing user specify customized
vectors.

Reviewers: alexfh, aaron.ballman

Reviewed By: alexfh

Subscribers: Eugene.Zelenko, cfe-commits

Differential Revision: https://reviews.llvm.org/D32436

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@301440 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 82 insertions, 19 deletions

diff --git a/clang-tidy/performance/InefficientVectorOperationCheck.cpp b/clang-tidy/performance/InefficientVectorOperationCheck.cpp
index a2207d39..be0437a6 100644
--- a/clang-tidy/performance/InefficientVectorOperationCheck.cpp
+++ b/clang-tidy/performance/InefficientVectorOperationCheck.cpp
@@ -12,6 +12,7 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
 #include "../utils/DeclRefExprUtils.h"
+#include "../utils/OptionsUtils.h"
 
 using namespace clang::ast_matchers;
 
@@ -33,7 +34,7 @@ namespace {
 // \endcode
 //
 // The matcher names are bound to following parts of the AST:
-//   - LoopName: The entire for loop (as ForStmt).
+//   - LoopCounterName: 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
@@ -46,14 +47,34 @@ 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";
 
+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::dequeue")));
+}
+
 } // namespace
 
+InefficientVectorOperationCheck::InefficientVectorOperationCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      VectorLikeClasses(utils::options::parseStringList(
+          Options.get("VectorLikeClasses", "::std::vector"))) {}
+
+void InefficientVectorOperationCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "VectorLikeClasses",
+                utils::options::serializeStringList(VectorLikeClasses));
+}
+
 void InefficientVectorOperationCheck::registerMatchers(MatchFinder *Finder) {
-  const auto VectorDecl = cxxRecordDecl(hasName("::std::vector"));
+  const auto VectorDecl = cxxRecordDecl(hasAnyName(SmallVector<StringRef, 5>(
+      VectorLikeClasses.begin(), VectorLikeClasses.end())));
   const auto VectorDefaultConstructorCall = cxxConstructExpr(
       hasType(VectorDecl),
       hasDeclaration(cxxConstructorDecl(isDefaultConstructor())));
@@ -77,6 +98,12 @@ void InefficientVectorOperationCheck::registerMatchers(MatchFinder *Finder) {
   const auto RefersToLoopVar = ignoringParenImpCasts(
       declRefExpr(to(varDecl(equalsBoundNode(LoopInitVarName)))));
 
+  // Matchers for the loop whose body has only 1 push_back calling statement.
+  const auto HasInterestingLoopBody = hasBody(anyOf(
+      compoundStmt(statementCountIs(1), has(PushBackCall)), PushBackCall));
+  const auto InInterestingCompoundStmt =
+      hasParent(compoundStmt(has(VectorVarDefStmt)).bind(LoopParentName));
+
   // Match counter-based for loops:
   //  for (int i = 0; i < n; ++i) { v.push_back(...); }
   //
@@ -90,11 +117,20 @@ void InefficientVectorOperationCheck::registerMatchers(MatchFinder *Finder) {
                          .bind(LoopEndExprName)))),
           hasIncrement(unaryOperator(hasOperatorName("++"),
                                      hasUnaryOperand(RefersToLoopVar))),
-          hasBody(anyOf(compoundStmt(statementCountIs(1), has(PushBackCall)),
-                        PushBackCall)),
-          hasParent(compoundStmt(has(VectorVarDefStmt)).bind(LoopParentName)))
+          HasInterestingLoopBody, InInterestingCompoundStmt)
           .bind(LoopCounterName),
       this);
+
+  // Match for-range loops:
+  //   for (const auto& E : data) { v.push_back(...); }
+  //
+  // FIXME: Support more complex range-expressions.
+  Finder->addMatcher(
+      cxxForRangeStmt(
+          hasRangeInit(declRefExpr(supportedContainerTypesMatcher())),
+          HasInterestingLoopBody, InInterestingCompoundStmt)
+          .bind(RangeLoopName),
+      this);
 }
 
 void InefficientVectorOperationCheck::check(
@@ -104,13 +140,19 @@ void InefficientVectorOperationCheck::check(
     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 *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);
+
+  const Stmt *LoopStmt = ForLoop;
+  if (!LoopStmt)
+    LoopStmt = RangeLoop;
 
   llvm::SmallPtrSet<const DeclRefExpr *, 16> AllVectorVarRefs =
       utils::decl_ref_expr::allDeclRefExprs(*VectorVarDecl, *LoopParent,
@@ -124,25 +166,43 @@ void InefficientVectorOperationCheck::check(
     // FIXME: make it more intelligent to identify the pre-allocating operations
     // before the for loop.
     if (SM.isBeforeInTranslationUnit(Ref->getLocation(),
-                                     ForLoop->getLocStart())) {
+                                     LoopStmt->getLocStart())) {
       return;
     }
   }
 
-  llvm::StringRef LoopEndSource = Lexer::getSourceText(
-      CharSourceRange::getTokenRange(LoopEndExpr->getSourceRange()), SM,
-      Context->getLangOpts());
   llvm::StringRef VectorVarName = Lexer::getSourceText(
       CharSourceRange::getTokenRange(
           PushBackCall->getImplicitObjectArgument()->getSourceRange()),
       SM, Context->getLangOpts());
-  std::string ReserveStmt =
-      (VectorVarName + ".reserve(" + LoopEndSource + ");\n").str();
 
-  diag(PushBackCall->getLocStart(),
+  std::string ReserveStmt;
+  // 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());
+
+    ReserveStmt =
+        (VectorVarName + ".reserve(" + RangeInitExpName + ".size()" + ");\n")
+            .str();
+  } else if (ForLoop) {
+    // Handle counter-based loop cases.
+    StringRef LoopEndSource = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(LoopEndExpr->getSourceRange()), SM,
+        Context->getLangOpts());
+    ReserveStmt = (VectorVarName + ".reserve(" + LoopEndSource + ");\n").str();
+  }
+
+  auto Diag = diag(PushBackCall->getLocStart(),
        "'push_back' is called inside a loop; "
-       "consider pre-allocating the vector capacity before the loop")
-      << FixItHint::CreateInsertion(ForLoop->getLocStart(), ReserveStmt);
+       "consider pre-allocating the vector capacity before the loop");
+
+  if (!ReserveStmt.empty())
+    Diag << FixItHint::CreateInsertion(LoopStmt->getLocStart(), ReserveStmt);
 }
 
 } // namespace performance
diff --git a/clang-tidy/performance/InefficientVectorOperationCheck.h b/clang-tidy/performance/InefficientVectorOperationCheck.h
index 246f8c0b..1427ff13 100644
--- a/clang-tidy/performance/InefficientVectorOperationCheck.h
+++ b/clang-tidy/performance/InefficientVectorOperationCheck.h
@@ -23,10 +23,13 @@ namespace performance {
 /// 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) {}
+  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:
+  const std::vector<std::string> VectorLikeClasses;
 };
 
 } // namespace performance