[clang-tools-extra] r303157 - [clang-tidy] Add "emplace_back" detection in inefficient-vector-operation.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue May 16 03:39:55 PDT 2017
Author: hokein
Date: Tue May 16 05:39:55 2017
New Revision: 303157
URL: http://llvm.org/viewvc/llvm-project?rev=303157&view=rev
Log:
[clang-tidy] Add "emplace_back" detection in inefficient-vector-operation.
Reviewers: alexfh, aaron.ballman
Reviewed By: alexfh
Subscribers: cfe-commits, Prazek, malcolm.parsons, xazax.hun
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D33209
Modified:
clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-vector-operation.cpp
Modified: clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.cpp?rev=303157&r1=303156&r2=303157&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/InefficientVectorOperationCheck.cpp Tue May 16 05:39:55 2017
@@ -39,14 +39,14 @@ namespace {
// - VectorVarDeclName: 'v' in (as VarDecl).
// - VectorVarDeclStmatName: The entire 'std::vector<T> 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 @@ void InefficientVectorOperationCheck::re
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 @@ void InefficientVectorOperationCheck::re
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 @@ void InefficientVectorOperationCheck::ch
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 *VectorAppendCall =
+ Result.Nodes.getNodeAs<CXXMemberCallExpr>(PushBackOrEmplaceBackCallName);
const auto *LoopEndExpr = Result.Nodes.getNodeAs<Expr>(LoopEndExprName);
const auto *LoopParent = Result.Nodes.getNodeAs<CompoundStmt>(LoopParentName);
@@ -173,7 +175,7 @@ void InefficientVectorOperationCheck::ch
llvm::StringRef VectorVarName = Lexer::getSourceText(
CharSourceRange::getTokenRange(
- PushBackCall->getImplicitObjectArgument()->getSourceRange()),
+ VectorAppendCall->getImplicitObjectArgument()->getSourceRange()),
SM, Context->getLangOpts());
std::string ReserveStmt;
@@ -197,9 +199,11 @@ void InefficientVectorOperationCheck::ch
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);
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst?rev=303157&r1=303156&r2=303157&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst Tue May 16 05:39:55 2017
@@ -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 @@ statement body:
* 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++
Modified: clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-vector-operation.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-vector-operation.cpp?rev=303157&r1=303156&r2=303157&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-vector-operation.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/performance-inefficient-vector-operation.cpp Tue May 16 05:39:55 2017
@@ -35,6 +35,9 @@ class vector {
explicit vector(size_type n);
void push_back(const T& val);
+
+ template <class... Args> void emplace_back(Args &&... args);
+
void reserve(size_t n);
void resize(size_t n);
@@ -61,205 +64,214 @@ int Op(int);
void f(std::vector<int>& t) {
{
- std::vector<int> v;
- // CHECK-FIXES: v.reserve(10);
+ std::vector<int> 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<int> v;
- // CHECK-FIXES: v.reserve(10);
+ std::vector<int> 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<int> v;
- // CHECK-FIXES: v.reserve(10);
+ std::vector<int> 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<int> v;
- // CHECK-FIXES: v.reserve(5);
+ std::vector<int> 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<int> v;
- std::vector<int> v2;
- v2.reserve(3);
- // CHECK-FIXES: v.reserve(10);
+ std::vector<int> v4;
+ std::vector<int> 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<int> v;
- // CHECK-FIXES: v.reserve(t.size());
+ std::vector<int> 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<int> v;
- // CHECK-FIXES: v.reserve(t.size() - 1);
+ std::vector<int> 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<int> v;
- // CHECK-FIXES: v.reserve(t.size());
+ std::vector<int> 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<int> v;
- // CHECK-FIXES: v.reserve(t.size());
+ std::vector<int> 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<Foo> v;
- // CHECK-FIXES: v.reserve(t.size());
+ std::vector<Foo> 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<Bar> v;
- // CHECK-FIXES: v.reserve(t.size());
+ std::vector<Bar> 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<Foo> 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<int> v;
- v.reserve(20);
- // CHECK-FIXES-NOT: v.reserve(10);
+ std::vector<int> 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<int> v;
- v.reserve(5);
- // CHECK-FIXES-NOT: v.reserve(10);
+ std::vector<int> 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<int> v;
- v.resize(5);
- // CHECK-FIXES-NOT: v.reserve(10);
+ std::vector<int> 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<int> v;
- v.push_back(0);
- // CHECK-FIXES-NOT: v.reserve(10);
+ std::vector<int> 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<int> v;
- f(v);
- // CHECK-FIXES-NOT: v.reserve(10);
- // There is a ref usage of v before the loop.
+ std::vector<int> 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<int> v(20);
- // CHECK-FIXES-NOT: v.reserve(10);
- // v is not constructed with default constructor.
+ std::vector<int> 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<int> v;
- // CHECK-FIXES-NOT: v.reserve(10);
+ std::vector<int> 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<int> v;
- // CHECK-FIXES-NOT: v.reserve(t.size());
- // v isn't referenced in for-loop body.
+ std::vector<int> 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<int> v;
+ std::vector<int> 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<int> v;
- // CHECK-FIXES-NOT: v.reserve(i + 1);
+ std::vector<int> 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<int> v;
+ std::vector<int> 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<int> v;
+ std::vector<int> 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<int> v;
- std::vector<int>* v2 = &t;
+ std::vector<int> z12;
+ std::vector<int>* 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);
}
}
}
More information about the cfe-commits
mailing list