Index: clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.cpp @@ -39,14 +39,14 @@ // - VectorVarDeclName: 'v' in (as VarDecl). // - VectorVarDeclStmatName: The entire 'std::vector v;' statement (as // DeclStmt). -// - PushBackCallName: 'v.push_back(i)' (as cxxMemberCallExpr). +// - PushBackOrEmplaceBackCallName: '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 PushBackOrEmplaceBackCallName[] = "append_call"; static const char LoopInitVarName[] = "loop_init_var"; static const char LoopEndExprName[] = "loop_end_expr"; @@ -81,13 +81,13 @@ const auto VectorVarDecl = varDecl(hasInitializer(VectorDefaultConstructorCall)) .bind(VectorVarDeclName); - const auto PushBackCallExpr = + const auto VectorAppendCallExpr = cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("push_back"))), on(hasType(VectorDecl)), + callee(cxxMethodDecl(hasAnyName("push_back", "emplace_back"))), + on(hasType(VectorDecl)), onImplicitObjectArgument(declRefExpr(to(VectorVarDecl)))) - .bind(PushBackCallName); - const auto PushBackCall = - expr(anyOf(PushBackCallExpr, exprWithCleanups(has(PushBackCallExpr)))); + .bind(PushBackOrEmplaceBackCallName); + const auto VectorAppendCall = expr(ignoringImplicit(VectorAppendCallExpr)); const auto VectorVarDefStmt = declStmt(hasSingleDecl(equalsBoundNode(VectorVarDeclName))) .bind(VectorVarDeclStmtName); @@ -98,9 +98,11 @@ 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)); + // Matchers for the loop whose body has only 1 push_back/emplace_back calling + // statement. + const auto HasInterestingLoopBody = + hasBody(anyOf(compoundStmt(statementCountIs(1), has(VectorAppendCall)), + VectorAppendCall)); const auto InInterestingCompoundStmt = hasParent(compoundStmt(has(VectorVarDefStmt)).bind(LoopParentName)); @@ -145,8 +147,8 @@ const auto *ForLoop = Result.Nodes.getNodeAs(LoopCounterName); const auto *RangeLoop = Result.Nodes.getNodeAs(RangeLoopName); - const auto *PushBackCall = - Result.Nodes.getNodeAs(PushBackCallName); + const auto *VectorAppendCall = + Result.Nodes.getNodeAs(PushBackOrEmplaceBackCallName); const auto *LoopEndExpr = Result.Nodes.getNodeAs(LoopEndExprName); const auto *LoopParent = Result.Nodes.getNodeAs(LoopParentName); @@ -173,7 +175,7 @@ llvm::StringRef VectorVarName = Lexer::getSourceText( CharSourceRange::getTokenRange( - PushBackCall->getImplicitObjectArgument()->getSourceRange()), + VectorAppendCall->getImplicitObjectArgument()->getSourceRange()), SM, Context->getLangOpts()); std::string ReserveStmt; @@ -197,9 +199,11 @@ 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"); + auto Diag = + diag(VectorAppendCall->getLocStart(), + "%0 is called inside a loop; " + "consider pre-allocating the vector capacity before the loop") + << VectorAppendCall->getMethodDecl()->getDeclName(); if (!ReserveStmt.empty()) Diag << FixItHint::CreateInsertion(LoopStmt->getLocStart(), ReserveStmt); Index: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst @@ -3,8 +3,8 @@ performance-inefficient-vector-operation ======================================== -Finds possible inefficient ``std::vector`` operations (e.g. ``push_back``) that -may cause unnecessary memory reallocations. +Finds possible inefficient ``std::vector`` operations (e.g. ``push_back``, +``emplace_back``) that may cause unnecessary memory reallocations. Currently, the check only detects following kinds of loops with a single statement body: @@ -24,7 +24,7 @@ * For-range loops like ``for (range-declaration : range_expression)``, the type of ``range_expression`` can be ``std::vector``, ``std::array``, - ``std::dequeue``, ``std::set``, ``std::unordered_set``, ``std::map``, + ``std::deque``, ``std::set``, ``std::unordered_set``, ``std::map``, ``std::unordered_set``: .. code-block:: c++ Index: clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-vector-operation.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-vector-operation.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-vector-operation.cpp @@ -35,6 +35,9 @@ explicit vector(size_type n); void push_back(const T& val); + + template void emplace_back(Args &&... args); + void reserve(size_t n); void resize(size_t n); @@ -61,205 +64,214 @@ void f(std::vector& t) { { - std::vector v; - // CHECK-FIXES: v.reserve(10); + std::vector v0; + // CHECK-FIXES: v0.reserve(10); for (int i = 0; i < 10; ++i) - v.push_back(i); + v0.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 v; - // CHECK-FIXES: v.reserve(10); + std::vector v1; + // CHECK-FIXES: v1.reserve(10); for (int i = 0; i < 10; i++) - v.push_back(i); + v1.push_back(i); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } { - std::vector v; - // CHECK-FIXES: v.reserve(10); + std::vector v2; + // CHECK-FIXES: v2.reserve(10); for (int i = 0; i < 10; ++i) - v.push_back(0); + v2.push_back(0); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } { - std::vector v; - // CHECK-FIXES: v.reserve(5); + std::vector v3; + // CHECK-FIXES: v3.reserve(5); for (int i = 0; i < 5; ++i) { - v.push_back(i); + v3.push_back(i); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } - // CHECK-FIXES-NOT: v.reserve(10); + // CHECK-FIXES-NOT: v3.reserve(10); for (int i = 0; i < 10; ++i) { // No fix for this loop as we encounter the prior loops. - v.push_back(i); + v3.push_back(i); } } { - std::vector v; - std::vector v2; - v2.reserve(3); - // CHECK-FIXES: v.reserve(10); + std::vector v4; + std::vector v5; + v5.reserve(3); + // CHECK-FIXES: v4.reserve(10); for (int i = 0; i < 10; ++i) - v.push_back(i); + v4.push_back(i); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } { - std::vector v; - // CHECK-FIXES: v.reserve(t.size()); + std::vector v6; + // CHECK-FIXES: v6.reserve(t.size()); for (std::size_t i = 0; i < t.size(); ++i) { - v.push_back(t[i]); + v6.push_back(t[i]); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } } { - std::vector v; - // CHECK-FIXES: v.reserve(t.size() - 1); + std::vector v7; + // CHECK-FIXES: v7.reserve(t.size() - 1); for (std::size_t i = 0; i < t.size() - 1; ++i) { - v.push_back(t[i]); + v7.push_back(t[i]); } // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } { - std::vector v; - // CHECK-FIXES: v.reserve(t.size()); + std::vector v8; + // CHECK-FIXES: v8.reserve(t.size()); for (const auto &e : t) { - v.push_back(e); + v8.push_back(e); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } } { - std::vector v; - // CHECK-FIXES: v.reserve(t.size()); + std::vector v9; + // CHECK-FIXES: v9.reserve(t.size()); for (const auto &e : t) { - v.push_back(Op(e)); + v9.push_back(Op(e)); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } } { - std::vector v; - // CHECK-FIXES: v.reserve(t.size()); + std::vector v10; + // CHECK-FIXES: v10.reserve(t.size()); for (const auto &e : t) { - v.push_back(Foo(e)); + v10.push_back(Foo(e)); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } } { - std::vector v; - // CHECK-FIXES: v.reserve(t.size()); + std::vector v11; + // CHECK-FIXES: v11.reserve(t.size()); for (const auto &e : t) { - v.push_back(e); + v11.push_back(e); // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called } } + { + std::vector v12; + // CHECK-FIXES: v12.reserve(t.size()); + for (const auto &e : t) { + v12.emplace_back(e); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'emplace_back' is called + } + } + // ---- Non-fixed Cases ---- { - std::vector v; - v.reserve(20); - // CHECK-FIXES-NOT: v.reserve(10); + std::vector z0; + z0.reserve(20); + // CHECK-FIXES-NOT: z0.reserve(10); // There is a "reserve" call already. for (int i = 0; i < 10; ++i) { - v.push_back(i); + z0.push_back(i); } } { - std::vector v; - v.reserve(5); - // CHECK-FIXES-NOT: v.reserve(10); + std::vector z1; + z1.reserve(5); + // CHECK-FIXES-NOT: z1.reserve(10); // There is a "reserve" call already. for (int i = 0; i < 10; ++i) { - v.push_back(i); + z1.push_back(i); } } { - std::vector v; - v.resize(5); - // CHECK-FIXES-NOT: v.reserve(10); + std::vector z2; + z2.resize(5); + // CHECK-FIXES-NOT: z2.reserve(10); // There is a ref usage of v before the loop. for (int i = 0; i < 10; ++i) { - v.push_back(i); + z2.push_back(i); } } { - std::vector v; - v.push_back(0); - // CHECK-FIXES-NOT: v.reserve(10); + std::vector z3; + z3.push_back(0); + // CHECK-FIXES-NOT: z3.reserve(10); // There is a ref usage of v before the loop. for (int i = 0; i < 10; ++i) { - v.push_back(i); + z3.push_back(i); } } { - std::vector v; - f(v); - // CHECK-FIXES-NOT: v.reserve(10); - // There is a ref usage of v before the loop. + std::vector z4; + f(z4); + // CHECK-FIXES-NOT: z4.reserve(10); + // There is a ref usage of z4 before the loop. for (int i = 0; i < 10; ++i) { - v.push_back(i); + z4.push_back(i); } } { - std::vector v(20); - // CHECK-FIXES-NOT: v.reserve(10); - // v is not constructed with default constructor. + std::vector z5(20); + // CHECK-FIXES-NOT: z5.reserve(10); + // z5 is not constructed with default constructor. for (int i = 0; i < 10; ++i) { - v.push_back(i); + z5.push_back(i); } } { - std::vector v; - // CHECK-FIXES-NOT: v.reserve(10); + std::vector z6; + // CHECK-FIXES-NOT: z6.reserve(10); // For-loop is not started with 0. for (int i = 1; i < 10; ++i) { - v.push_back(i); + z6.push_back(i); } } { - std::vector v; - // CHECK-FIXES-NOT: v.reserve(t.size()); - // v isn't referenced in for-loop body. + std::vector z7; + // CHECK-FIXES-NOT: z7.reserve(t.size()); + // z7 isn't referenced in for-loop body. for (std::size_t i = 0; i < t.size(); ++i) { t.push_back(i); } } { - std::vector v; + std::vector z8; int k; - // CHECK-FIXES-NOT: v.reserve(10); + // CHECK-FIXES-NOT: z8.reserve(10); // For-loop isn't a fixable loop. for (std::size_t i = 0; k < 10; ++i) { - v.push_back(t[i]); + z8.push_back(t[i]); } } { - std::vector v; - // CHECK-FIXES-NOT: v.reserve(i + 1); + std::vector z9; + // CHECK-FIXES-NOT: z9.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); + z9.push_back(i); } { - std::vector v; + std::vector z10; int k; - // CHECK-FIXES-NOT: v.reserve(10); + // CHECK-FIXES-NOT: z10.reserve(10); // For-loop isn't a fixable loop. for (std::size_t i = 0; i < 10; ++k) { - v.push_back(t[i]); + z10.push_back(t[i]); } } { - std::vector v; + std::vector z11; // initializer_list should not trigger the check. for (int e : {1, 2, 3, 4, 5}) { - v.push_back(e); + z11.push_back(e); } } { - std::vector v; - std::vector* v2 = &t; + std::vector z12; + std::vector* z13 = &t; // We only support detecting the range init expression which references // container directly. - // Complex range init expressions like `*v2` is not supported. - for (const auto &e : *v2) { - v.push_back(e); + // Complex range init expressions like `*z13` is not supported. + for (const auto &e : *z13) { + z12.push_back(e); } } }