const auto DefaultContainersWithPushBack =
"::std::vector; ::std::list; ::std::deque";
+const auto DefaultContainersWithPush =
+ "::std::stack; ::std::queue; ::std::priority_queue";
+const auto DefaultContainersWithPushFront =
+ "::std::forward_list; ::std::list; ::std::deque";
const auto DefaultSmartPointers =
"::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
const auto DefaultTupleTypes = "::std::pair; ::std::tuple";
"IgnoreImplicitConstructors", false)),
ContainersWithPushBack(utils::options::parseStringList(Options.get(
"ContainersWithPushBack", DefaultContainersWithPushBack))),
+ ContainersWithPush(utils::options::parseStringList(
+ Options.get("ContainersWithPush", DefaultContainersWithPush))),
+ ContainersWithPushFront(utils::options::parseStringList(Options.get(
+ "ContainersWithPushFront", DefaultContainersWithPushFront))),
SmartPointers(utils::options::parseStringList(
Options.get("SmartPointers", DefaultSmartPointers))),
TupleTypes(utils::options::parseStringList(
void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
// FIXME: Bunch of functionality that could be easily added:
- // + add handling of `push_front` for std::forward_list, std::list
- // and std::deque.
- // + add handling of `push` for std::stack, std::queue, std::priority_queue
// + add handling of `insert` for stl associative container, but be careful
// because this requires special treatment (it could cause performance
// regression)
hasDeclaration(functionDecl(hasName("push_back"))),
on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushBack)))));
+ auto CallPush = cxxMemberCallExpr(
+ hasDeclaration(functionDecl(hasName("push"))),
+ on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPush)))));
+
+ auto CallPushFront = cxxMemberCallExpr(
+ hasDeclaration(functionDecl(hasName("push_front"))),
+ on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushFront)))));
+
auto CallEmplacy = cxxMemberCallExpr(
hasDeclaration(
functionDecl(hasAnyNameIgnoringTemplates(EmplacyFunctions))),
this);
Finder->addMatcher(
+ traverse(TK_AsIs, cxxMemberCallExpr(CallPush, has(SoughtParam),
+ unless(isInTemplateInstantiation()))
+ .bind("push_call")),
+ this);
+
+ Finder->addMatcher(
+ traverse(TK_AsIs, cxxMemberCallExpr(CallPushFront, has(SoughtParam),
+ unless(isInTemplateInstantiation()))
+ .bind("push_front_call")),
+ this);
+
+ Finder->addMatcher(
traverse(TK_AsIs,
cxxMemberCallExpr(
CallEmplacy, HasConstructExprWithValueTypeTypeAsLastArgument,
void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
const auto *PushBackCall =
Result.Nodes.getNodeAs<CXXMemberCallExpr>("push_back_call");
+ const auto *PushCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("push_call");
+ const auto *PushFrontCall =
+ Result.Nodes.getNodeAs<CXXMemberCallExpr>("push_front_call");
const auto *EmplacyCall =
Result.Nodes.getNodeAs<CXXMemberCallExpr>("emplacy_call");
const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
const auto *MakeCall = Result.Nodes.getNodeAs<CallExpr>("make");
- assert((PushBackCall || EmplacyCall) && "No call matched");
- assert((CtorCall || MakeCall) && "No push_back parameter matched");
+ const CXXMemberCallExpr *Call = [&]() {
+ if (PushBackCall) {
+ return PushBackCall;
+ }
+ if (PushCall) {
+ return PushCall;
+ }
+ if (PushFrontCall) {
+ return PushFrontCall;
+ }
+ return EmplacyCall;
+ }();
- const CXXMemberCallExpr *Call = PushBackCall ? PushBackCall : EmplacyCall;
+ assert(Call && "No call matched");
+ assert((CtorCall || MakeCall) && "No push_back parameter matched");
if (IgnoreImplicitConstructors && CtorCall && CtorCall->getNumArgs() >= 1 &&
CtorCall->getArg(0)->getSourceRange() == CtorCall->getSourceRange())
Call->getExprLoc(), Call->getArg(0)->getExprLoc());
auto Diag =
- PushBackCall
- ? diag(Call->getExprLoc(), "use emplace_back instead of push_back")
- : diag(CtorCall ? CtorCall->getBeginLoc() : MakeCall->getBeginLoc(),
- "unnecessary temporary object created while calling " +
- Call->getMethodDecl()->getName().str());
+ EmplacyCall
+ ? diag(CtorCall ? CtorCall->getBeginLoc() : MakeCall->getBeginLoc(),
+ "unnecessary temporary object created while calling %0")
+ : diag(Call->getExprLoc(), "use emplace%select{|_back|_front}0 "
+ "instead of push%select{|_back|_front}0");
+ if (EmplacyCall)
+ Diag << Call->getMethodDecl()->getName();
+ else if (PushCall)
+ Diag << 0;
+ else if (PushBackCall)
+ Diag << 1;
+ else
+ Diag << 2;
if (FunctionNameSourceRange.getBegin().isMacroID())
return;
const char *EmplacePrefix = MakeCall ? "emplace_back" : "emplace_back(";
Diag << FixItHint::CreateReplacement(FunctionNameSourceRange,
EmplacePrefix);
+ } else if (PushCall) {
+ const char *EmplacePrefix = MakeCall ? "emplace" : "emplace(";
+ Diag << FixItHint::CreateReplacement(FunctionNameSourceRange,
+ EmplacePrefix);
+ } else if (PushFrontCall) {
+ const char *EmplacePrefix = MakeCall ? "emplace_front" : "emplace_front(";
+ Diag << FixItHint::CreateReplacement(FunctionNameSourceRange,
+ EmplacePrefix);
}
const SourceRange CallParensRange =
Options.store(Opts, "IgnoreImplicitConstructors", IgnoreImplicitConstructors);
Options.store(Opts, "ContainersWithPushBack",
utils::options::serializeStringList(ContainersWithPushBack));
+ Options.store(Opts, "ContainersWithPush",
+ utils::options::serializeStringList(ContainersWithPush));
+ Options.store(Opts, "ContainersWithPushFront",
+ utils::options::serializeStringList(ContainersWithPushFront));
Options.store(Opts, "SmartPointers",
utils::options::serializeStringList(SmartPointers));
Options.store(Opts, "TupleTypes",
=====================
The check flags insertions to an STL-style container done by calling the
-``push_back`` method with an explicitly-constructed temporary of the container
-element type. In this case, the corresponding ``emplace_back`` method
-results in less verbose and potentially more efficient code.
-Right now the check doesn't support ``push_front`` and ``insert``.
-It also doesn't support ``insert`` functions for associative containers
-because replacing ``insert`` with ``emplace`` may result in
+``push_back``, ``push``, or ``push_front`` methods with an
+explicitly-constructed temporary of the container element type. In this case,
+the corresponding ``emplace`` equivalent methods result in less verbose and
+potentially more efficient code. Right now the check doesn't support
+``insert``. It also doesn't support ``insert`` functions for associative
+containers because replacing ``insert`` with ``emplace`` may result in
`speed regression <https://htmlpreview.github.io/?https://github.com/HowardHinnant/papers/blob/master/insert_vs_emplace.html>`_, but it might get support with some addition flag in the future.
-By default only ``std::vector``, ``std::deque``, ``std::list`` are considered.
-This list can be modified using the :option:`ContainersWithPushBack` option.
+The :option:`ContainersWithPushBack`, :option:`ContainersWithPush`, and
+:option:`ContainersWithPushFront` options are used to specify the container
+types that support the ``push_back``, ``push``, and ``push_front`` operations
+respectively. The default values for these options are as follows:
+
+* :option:`ContainersWithPushBack`: ``std::vector``, ``std::deque``, and ``std::list``.
+* :option:`ContainersWithPush`: ``std::stack``, ``std::queue``, and ``std::priority_queue``.
+* :option:`ContainersWithPushFront`: ``std::forward_list``, ``std::list``, and ``std::deque``.
This check also reports when an ``emplace``-like method is improperly used,
for example using ``emplace_back`` while also calling a constructor. This
Semicolon-separated list of class names of custom containers that support
``push_back``.
+.. option:: ContainersWithPush
+
+ Semicolon-separated list of class names of custom containers that support
+ ``push``.
+
+.. option:: ContainersWithPushFront
+
+ Semicolon-separated list of class names of custom containers that support
+ ``push_front``.
+
.. option:: IgnoreImplicitConstructors
When `true`, the check will ignore implicitly constructed arguments of
class const_iterator {};
const_iterator begin() { return const_iterator{}; }
+ void push_front(const T &) {}
+ void push_front(T &&) {}
+
void push_back(const T &) {}
void push_back(T &&) {}
void push_back(const T &) {}
void push_back(T &&) {}
+ void push_front(const T &) {}
+ void push_front(T &&) {}
+
template <typename... Args>
iterator emplace(const_iterator pos, Args &&...args){};
template <typename... Args>
class const_iterator {};
const_iterator begin() { return const_iterator{}; }
+ void push_front(const T &) {}
+ void push_front(T &&) {}
+
template <typename... Args>
void emplace_front(Args &&...args){};
template <typename... Args>
public:
using value_type = T;
+ void push(const T &) {}
+ void push(T &&) {}
+
template <typename... Args>
void emplace(Args &&...args){};
};
public:
using value_type = T;
+ void push(const T &) {}
+ void push(T &&) {}
+
template <typename... Args>
void emplace(Args &&...args){};
};
public:
using value_type = T;
+ void push(const T &) {}
+ void push(T &&) {}
+
template <typename... Args>
void emplace(Args &&...args){};
};
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
// CHECK-FIXES: l.emplace_back(42, 41);
+ l.push_front(Something(42, 41));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_front
+ // CHECK-FIXES: l.emplace_front(42, 41);
+
std::deque<Something> d;
d.push_back(Something(42));
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
// CHECK-FIXES: d.emplace_back(42);
+ d.push_front(Something(42));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_front
+ // CHECK-FIXES: d.emplace_front(42);
+
llvm::LikeASmallVector<Something> ls;
ls.push_back(Something(42));
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
// CHECK-FIXES: ls.emplace_back(42);
+
+ std::stack<Something> s;
+ s.push(Something(42, 41));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace
+ // CHECK-FIXES: s.emplace(42, 41);
+
+ std::queue<Something> q;
+ q.push(Something(42, 41));
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace
+ // CHECK-FIXES: q.emplace(42, 41);
+
+ std::priority_queue<Something> pq;
+ pq.push(Something(42, 41));
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace
+ // CHECK-FIXES: pq.emplace(42, 41);
+
+ std::forward_list<Something> fl;
+ fl.push_front(Something(42, 41));
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_front
+ // CHECK-FIXES: fl.emplace_front(42, 41);
}
class IntWrapper {