hokein updated this revision to Diff 99119.
hokein marked an inline comment as done.
hokein added a comment.
Adress review comments.
https://reviews.llvm.org/D33209
Files:
clang-tidy/performance/InefficientVectorOperationCheck.cpp
docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
test/clang-tidy/performance-inefficient-vector-operation.cpp
Index: test/clang-tidy/performance-inefficient-vector-operation.cpp
===================================================================
--- test/clang-tidy/performance-inefficient-vector-operation.cpp
+++ test/clang-tidy/performance-inefficient-vector-operation.cpp
@@ -35,6 +35,9 @@
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);
@@ -150,6 +153,14 @@
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'push_back' is called
}
}
+ {
+ std::vector<Foo> v;
+ // CHECK-FIXES: v.reserve(t.size());
+ for (const auto &e : t) {
+ v.emplace_back(e);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'emplace_back' is called
+ }
+ }
// ---- Non-fixed Cases ----
{
std::vector<int> v;
Index: docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
===================================================================
--- docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
+++ 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-tidy/performance/InefficientVectorOperationCheck.cpp
===================================================================
--- clang-tidy/performance/InefficientVectorOperationCheck.cpp
+++ clang-tidy/performance/InefficientVectorOperationCheck.cpp
@@ -39,14 +39,15 @@
// - 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[] =
+ "push_back_or_emplace_back_call";
static const char LoopInitVarName[] = "loop_init_var";
static const char LoopEndExprName[] = "loop_end_expr";
@@ -81,13 +82,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 +99,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 +148,8 @@
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 +176,7 @@
llvm::StringRef VectorVarName = Lexer::getSourceText(
CharSourceRange::getTokenRange(
- PushBackCall->getImplicitObjectArgument()->getSourceRange()),
+ VectorAppendCall->getImplicitObjectArgument()->getSourceRange()),
SM, Context->getLangOpts());
std::string ReserveStmt;
@@ -197,9 +200,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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
