[clang-tidy] Add modernize-use-emplace

[Prazek]

Summary

This check look for cases when inserting new element into an STL
container, but the element is constructed temporarily.

It changes it to call of emplace_back

[Prazek]

• Fix

[sbarzowski]

clang-tidy/modernize/UseEmplaceCheck.cpp

57 ↗

(On Diff #59558)

capital letter?

docs/ReleaseNotes.rst

212 ↗

(On Diff #59558)

typo: change -> changed

docs/clang-tidy/checks/modernize-use-emplace.rst

6 ↗

(On Diff #59558)

Why "would"? Did you copy the description from the proposal? :-)

8 ↗

(On Diff #59558)

Same here, and it is not obvious to me what is so special about std::pair here.

[sbarzowski]

clang-tidy/modernize/UseEmplaceCheck.cpp

28 ↗

(On Diff #59558)

Shouldn't it start with an uppercase letter?

[sbarzowski]

clang-tidy/modernize/UseEmplaceCheck.cpp

37 ↗

(On Diff #59558)

Same

41 ↗

(On Diff #59558)

Same

44 ↗

(On Diff #59558)

same

[Prazek]

docs/clang-tidy/checks/modernize-use-emplace.rst

6 ↗

(On Diff #59558)

yep :D

[Prazek]

Fixed stuff

[Prazek]

Learning how to use arc

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

29 ↗

(On Diff #59562)

I don't think so. In all checks people use lowerCamelCase, because it looks like a real matchers

[dblaikie]

Could you describe in more detail (ideally in the original patch review summary/description) what this transformation does? Under what conditions does it suggest emplace, etc.

[sbarzowski]

clang-tidy/modernize/UseEmplaceCheck.cpp

30 ↗

(On Diff #59558)

Maybe it can work with some more types, with minimal changes... Like maybe std::list.

38 ↗

(On Diff #59558)

This argument really holds for _any_ class taking ownership of the created object.

Maybe it would be better to check if we pass a non-const pointer as an argument to the constructor, as a pretty good indicator of taking ownership.

Or maybe just look for new operator in the expression.

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

31 ↗

(On Diff #59562)

ok, std::list works for me. I just don't want to spend much time on it right now, and I want to be sure it works.

39 ↗

(On Diff #59562)

Look at tests - the same thing happens when

std::vector<std::unique_ptr<int>> v;

const auto *p = new int;
v.push_back(p);

Not many custom classes take pointer in constructor.
If I will look for const pointers, then I will not be able to pass "abc" into vector<string>.

So I guess this solution seems to be the best idea.

[Prazek]

• Post review fix

[sbarzowski]

clang-tidy/modernize/UseEmplaceCheck.cpp

40 ↗

(On Diff #59568)

Look at tests - the same thing happens when

Yeah. I meant looking for new in addition to blacklist.

Not many custom classes take pointer in constructor.

Obviously depends on codebase, but IMO it's quite common. However usually this classes aren't copy-constructible (or at least shouldn't be) anyway, making it impossible to use push_back, so maybe it's not such a big deal.

If I will look for const pointers, then I will not be able to pass "abc" into vector<string>.

I wrote explicitly about only non-const pointers.

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

40 ↗

(On Diff #59568)

It doesn't matter if it is const or not. Disabling any of them would disable some cases where it would be good.
I have to run it on llvm code base and see what happens

[sbarzowski]

clang-tidy/modernize/UseEmplaceCheck.cpp

40 ↗

(On Diff #59568)

Of course there would be some false negatives. It's close to impossible to know for sure if it's safe, so we need some sort of heuristic. My concern is that the current "blacklist smart pointers" solution may lead to a lot of false positives. And false positives here are quite nasty - very subtle bugs, that are unlikely to be caught by the tests or quick code review. So I'm more comfortable with false negatives.

[Eugene.Zelenko]

I think will be good idea to try this check with LLVM STL too.

docs/clang-tidy/checks/modernize-use-emplace.rst

47 ↗

(On Diff #59568)

Please highlight push_back with ``.

55 ↗

(On Diff #59568)

Please highlight emplace_back with ``. Same below.

test/clang-tidy/modernize-use-emplace.cpp

12 ↗

(On Diff #59568)

Please insert new line.

78 ↗

(On Diff #59568)

Please insert new line.

[Prazek]

• Review fixes

[Prazek]

In D20964#448455, @Eugene.Zelenko wrote:

I think will be good idea to try this check with LLVM STL too.

You mean llvm::SmallVector stuff?

[Eugene.Zelenko]

In D20964#448525, @Prazek wrote:

In D20964#448455, @Eugene.Zelenko wrote:

I think will be good idea to try this check with LLVM STL too.

You mean llvm::SmallVector stuff?

No, I meant to build example with -stdlib=libc++, -lc++, -lc++abi. Just to make sure, that hasName() is proper matcher.

[Prazek]

In D20964#448551, @Eugene.Zelenko wrote:

In D20964#448525, @Prazek wrote:

In D20964#448455, @Eugene.Zelenko wrote:

I think will be good idea to try this check with LLVM STL too.

You mean llvm::SmallVector stuff?

No, I meant to build example with -stdlib=libc++, -lc++, -lc++abi. Just to make sure, that hasName() is proper matcher.

I runned it on llvm codebase with libstdc++ and it worked perfectly. I don't think there should be any change with libc++. I will run it only on small with libc++, because it will take too much time to compile whole llvm again.

[Eugene.Zelenko]

There is alternative implementation in D21185. Will be good idea to make one which take the best from both :-)

[vsk]

@Eugene.Zelenko thanks for pointing this out, I had totally missed this patch! Once we get this reviewed I'm willing to abandon my version. Some comments inline --

clang-tidy/modernize/UseEmplaceCheck.cpp

26 ↗

(On Diff #59580)

cerfull -> careful

37 ↗

(On Diff #59580)

becase -> because

test/clang-tidy/modernize-use-emplace.cpp

147 ↗

(On Diff #59580)

I don't think this is correct. Comments immediately before/after S or immediately before/after the second right parenthesis should be preserved. C.f the implementation in D21185.

[vsk]

clang-tidy/modernize/UseEmplaceCheck.cpp

41 ↗

(On Diff #59580)

I agree that blacklisting some smart pointers is not a complete solution, and that we shouldn't introduce a check which emits false positives.

ISTM it's only safe to perform the "push(T(...)) -> emplace(...)" change if: it's safe to assume that if "emplace(...)" does not successfully call "T(...)", it's OK for the program to fail/leak/crash. Do we get to make this assumption ever? Perhaps just in no-exceptions mode?

[sbarzowski]

clang-tidy/modernize/UseEmplaceCheck.cpp

41 ↗

(On Diff #59580)

hmmm... Maybe it's not that bad.

Let's look closely at the difference in behaviour.

with push_back it works like:

1. Call the constructor
2. Allocate the memory
3. Call the copy constructor

And emplace_back is:

1. Allocate the memory
2. Call the constructor

Each of these steps in both scenarios may fail.

If we call the constructor and it fails, then it's alright. We have the same behaviour in both cases - if it can fail it should do its cleanup. (And failure of copy constructor obviously doesn't bother us).

So the scenario we have to worry about in only when memory allocation fails. Then with push_back, we'll call constructor and then destructor, while emplace_back doesn't call constructor at all.

So the real question is: "Is it safe to throw some exception and not to call the constructor at all". And I see only one real-world case it isn't - taking the ownership of the object.

[vsk]

clang-tidy/modernize/UseEmplaceCheck.cpp

41 ↗

(On Diff #59580)

@sbarzowski Right, I think we're in agreement about what the problem is. I just don't think there are heuristics or comprehensive blacklists to detect classes which have ownership semantics. E.g, consider a FileHandle class which owns a file descriptor and is responsible for closing it.

[Prazek]

Runned on LLVM and bug fixed one thing

[Prazek]

@alexfh or @hokein ping

[hokein]

A few comments.

clang-tidy/modernize/ModernizeTidyModule.cpp

59 ↗

(On Diff #60662)

Alphabetical order.

clang-tidy/modernize/UseEmplaceCheck.cpp

37 ↗

(On Diff #60662)

remove the will here? if emplacement fails, we will ...

78 ↗

(On Diff #60662)

missing . at the end of sentence.

clang-tidy/modernize/UseEmplaceCheck.h

19 ↗

(On Diff #60662)

s/look/looks

docs/clang-tidy/checks/modernize-use-emplace.rst

6 ↗

(On Diff #60662)

s/look/looks

test/clang-tidy/modernize-use-emplace.cpp

2 ↗

(On Diff #60662)

Could you also add the following case in the test?

vector<vector<int>> v1;
v1.push_back(1<<10);

Make sure the check won't change to v1.emplace_back(1<<10). Since these two statements have completely different semantic. The first one adds 1024 to v1 while the second one adds a vector with 1024 elements to v1.

94 ↗

(On Diff #60662)

I think you can remove the {{..}}.

[Prazek]

test/clang-tidy/modernize-use-emplace.cpp

2 ↗

(On Diff #60662)

I think you have made some mistake, because the code that you have showed doesn't compile.

[Prazek]

You might be interested.

[Prazek]

Changes after running og llvm code base. Adding llvm::SmallVector was a good idea, because it has fired for much more cases and I have found 3 other bugs.
Now check doesn't fire when argument of constructor is bitfield or new expression, or when constructor is converted to base class.
I will post diff of changes after running in a minute

[Prazek]

Changes after running http://reviews.llvm.org/D21507

[Prazek]

In D20964#453797, @vsk wrote:

@Eugene.Zelenko thanks for pointing this out, I had totally missed this patch! Once we get this reviewed I'm willing to abandon my version. Some comments inline --

I have tested it on llvm code base and I have found many, many corner cases. I see that your check have better fixit messages (it only prints change of push_back -> emplace_back). I guess the best idea is to push this check, and then if you would like, change it so it would also have nice fixit messages

[hokein]

Looks almost good now, a few comments. You'd better await for comments from @alexfh or @sbenza before committing.

docs/clang-tidy/checks/modernize-use-emplace.rst

62 ↗

(On Diff #61214)

s/fire/fires
s/woule/would

test/clang-tidy/modernize-use-emplace.cpp

3 ↗

(On Diff #61214)

Oops, you are right. That's another non-relevant issue (different emplace_back behaviors in vector<int>, vector<vector<int>>). I'm over-concerned here.

317 ↗

(On Diff #61214)

Looks like the test is kind of duplicated with testAggregate in L248, you can merge that one into this one.

[sbenza]

clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp

34

We should not hard code this list. Specially not non-standard types like llvm::SmallVector.
This should come from an option string.
For example, like we do in performance/FasterStringFindCheck.cpp

42

These are not the only smart pointers around.
It might be a good idea to make this list also configurable by the user.

45

There are a few more things that can break here:

• NULL can't be passed through perfect forwarding. Will be deduced as long.
• Function references/pointers can't be passed through PF if they are overloaded.
• Class scope static variables that have no definition can't be passed through PF because they will be ODR used.

92

You should avoid using offsets with locations.
For example, you are hardcoding { as one character, which might not be true in the case of digraphs.

This should be getTokenRange(InnerCtorCall->getExprLoc(), CallParensRange.getBegin())

Same thing for the other one, it should be:

CharSourceRange::getTokenRange(CallParensRange.getEnd(), CallParensRange.getEnd())

clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.h

23

runaway qoute

clang-tools-extra/trunk/clang-tidy/utils/Matchers.h

48

This matcher is generic enough that it should go into main ASTMatchers.h file.

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst

17

Don't add the space between the >>. This is not needed since we are in C++11.

52

This doesn't compile. unique_ptr is not implicitly convertible from a pointer.

53

I think you meant auto* ptr = new int(5) ?

clang-tools-extra/trunk/test/clang-tidy/modernize-use-emplace.cpp

76

You should also try with a type that has a user-defined destructor.
It changes the AST enough to make a difference in many cases.

And you should fix all the std classes above to have user-defined constructors too.

209

This test is broken. unique_ptr<T> should not have an implicit conversion from T*.

338

It is also missing tests with templates.
ie: the container being a dependent type, and the value_type being a dependent type.
We should not change the code in either of those cases.

[Prazek]

Thank you Samuel, that it is valuable review!
I wanted to add the functionality of specifing stuff by options later.

should we revert this patch? It seems that those bugs are not so critical (at least on llvm http://reviews.llvm.org/D21507).
Anyway, I will fix it in one week.

clang-tools-extra/trunk/clang-tidy/modernize/CMakeLists.txt

add_clang_library(clangTidyModernizeModule
   PassByValueCheck.cpp
   RawStringLiteralCheck.cpp
   RedundantVoidArgCheck.cpp
   ReplaceAutoPtrCheck.cpp
   ShrinkToFitCheck.cpp
   UseAutoCheck.cpp
   UseBoolLiteralsCheck.cpp
   UseDefaultCheck.cpp
+  UseEmplaceCheck.cpp
   UseNullptrCheck.cpp
   UseOverrideCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyReadabilityModule
   clangTidyUtils
   )

clang-tools-extra/trunk/clang-tidy/modernize/ModernizeTidyModule.cpp

 #include "PassByValueCheck.h"
 #include "RawStringLiteralCheck.h"
 #include "RedundantVoidArgCheck.h"
 #include "ReplaceAutoPtrCheck.h"
 #include "ShrinkToFitCheck.h"
 #include "UseAutoCheck.h"
 #include "UseBoolLiteralsCheck.h"
 #include "UseDefaultCheck.h"
+#include "UseEmplaceCheck.h"
 #include "UseNullptrCheck.h"
 #include "UseOverrideCheck.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
CheckFactories.registerCheck<RedundantVoidArgCheck>(
         "modernize-redundant-void-arg");
     CheckFactories.registerCheck<ReplaceAutoPtrCheck>(
         "modernize-replace-auto-ptr");
     CheckFactories.registerCheck<ShrinkToFitCheck>("modernize-shrink-to-fit");
     CheckFactories.registerCheck<UseAutoCheck>("modernize-use-auto");
     CheckFactories.registerCheck<UseBoolLiteralsCheck>(
         "modernize-use-bool-literals");
     CheckFactories.registerCheck<UseDefaultCheck>("modernize-use-default");
+    CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace");
     CheckFactories.registerCheck<UseNullptrCheck>("modernize-use-nullptr");
     CheckFactories.registerCheck<UseOverrideCheck>("modernize-use-override");
   }
 
   ClangTidyOptions getModuleOptions() override {
     ClangTidyOptions Options;
     auto &Opts = Options.CheckOptions;
     // For types whose size in bytes is above this threshold, we prefer taking a

clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.h

+//===--- UseEmplaceCheck.h - clang-tidy--------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_EMPLACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_EMPLACE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// This check looks for cases when inserting new element into std::vector but
+/// the element is constructed temporarily.
+/// It replaces those calls for emplace_back of arguments passed to
+/// constructor of temporary object.
+///`
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html
+class UseEmplaceCheck : public ClangTidyCheck {
+public:
+  UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_EMPLACE_H

clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp

+//===--- UseEmplaceCheck.cpp - clang-tidy----------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseEmplaceCheck.h"
+#include "../utils/Matchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+    return;
+
+  // 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)
+  // + match for emplace calls that should be replaced with insertion
+  // + match for make_pair calls.
+  auto callPushBack = cxxMemberCallExpr(
+      hasDeclaration(functionDecl(hasName("push_back"))),
+      on(hasType(cxxRecordDecl(hasAnyName("std::vector", "llvm::SmallVector",
+                                          "std::list", "std::deque")))));
+
+  // We can't replace push_backs of smart pointer because
+  // if emplacement fails (f.e. bad_alloc in vector) we will have leak of
+  // passed pointer because smart pointer won't be constructed
+  // (and destructed) as in push_back case.
+  auto isCtorOfSmartPtr = hasDeclaration(cxxConstructorDecl(
+      ofClass(hasAnyName("std::shared_ptr", "std::unique_ptr", "std::auto_ptr",
+                         "std::weak_ptr"))));
+
+  // Bitfields binds only to consts and emplace_back take it by universal ref.
+  auto bitFieldAsArgument = hasAnyArgument(ignoringParenImpCasts(
+      memberExpr(hasDeclaration(fieldDecl(matchers::isBitfield())))));
+
+  // We could have leak of resource.
+  auto newExprAsArgument = hasAnyArgument(ignoringParenImpCasts(cxxNewExpr()));
+  auto constructingDerived =
+      hasParent(implicitCastExpr(hasCastKind(CastKind::CK_DerivedToBase)));
+
+  auto hasInitList = has(ignoringParenImpCasts(initListExpr()));
+  auto soughtConstructExpr =
+      cxxConstructExpr(
+          unless(anyOf(isCtorOfSmartPtr, hasInitList, bitFieldAsArgument,
+                       newExprAsArgument, constructingDerived,
+                       has(materializeTemporaryExpr(hasInitList)))))
+          .bind("ctor");
+  auto hasConstructExpr = has(ignoringParenImpCasts(soughtConstructExpr));
+
+  auto ctorAsArgument = materializeTemporaryExpr(
+      anyOf(hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr))));
+
+  Finder->addMatcher(
+      cxxMemberCallExpr(callPushBack, has(ctorAsArgument)).bind("call"), this);
+}
+
+void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+  const auto *InnerCtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
+
+  auto FunctionNameSourceRange = CharSourceRange::getCharRange(
+      Call->getExprLoc(), Call->getArg(0)->getExprLoc());
+
+  auto Diag = diag(Call->getExprLoc(), "use emplace_back instead of push_back");
+
+  if (FunctionNameSourceRange.getBegin().isMacroID())
+    return;
+
+  Diag << FixItHint::CreateReplacement(FunctionNameSourceRange,
+                                       "emplace_back(");
+
+  auto CallParensRange = InnerCtorCall->getParenOrBraceRange();
+
+  // Finish if there is no explicit constructor call.
+  if (CallParensRange.getBegin().isInvalid())
+    return;
+
+  // Range for constructor name and opening brace.
+  auto CtorCallSourceRange = CharSourceRange::getCharRange(
+      InnerCtorCall->getExprLoc(),
+      CallParensRange.getBegin().getLocWithOffset(1));
+
+  Diag << FixItHint::CreateRemoval(CtorCallSourceRange)
+       << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+              CallParensRange.getEnd(),
+              CallParensRange.getEnd().getLocWithOffset(1)));
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang

clang-tools-extra/trunk/clang-tidy/utils/Matchers.h

AST_MATCHER(QualType, isExpensiveToCopy) {
   return IsExpensive && *IsExpensive;
 }
 
 AST_MATCHER(RecordDecl, isTriviallyDefaultConstructible) {
   return utils::type_traits::recordIsTriviallyDefaultConstructible(
       Node, Finder->getASTContext());
 }
 
+AST_MATCHER(FieldDecl, isBitfield) { return Node.isBitField(); }
+
 } // namespace matchers
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_MATCHERS_H

clang-tools-extra/trunk/docs/ReleaseNotes.rst

- New `modernize-raw-string-literal
   Selectively replaces string literals containing escaped characters with raw
   string literals.
 
 - New `modernize-use-bool-literals
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-bool-literals.html>`_ check
 
   Finds integer literals which are cast to ``bool``.
 
+- New `modernize-use-emplace
+  <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html>`_ check
+
+  Finds calls that could be changed to emplace.
+
 - New `performance-faster-string-find
   <http://clang.llvm.org/extra/clang-tidy/checks/performance-faster-string-find.html>`_ check
 
   Optimize calls to ``std::string::find()`` and friends when the needle passed
   is a single character string literal.
 
 - New `performance-implicit-cast-in-loop
   <http://clang.llvm.org/extra/clang-tidy/checks/performance-implicit-cast-in-loop.html>`_ check

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

.. toctree::
    modernize-pass-by-value
    modernize-raw-string-literal
    modernize-redundant-void-arg
    modernize-replace-auto-ptr
    modernize-shrink-to-fit
    modernize-use-auto
    modernize-use-bool-literals
    modernize-use-default
+   modernize-use-emplace
    modernize-use-nullptr
    modernize-use-override
    performance-faster-string-find
    performance-for-range-copy
    performance-implicit-cast-in-loop
    performance-unnecessary-copy-initialization
    performance-unnecessary-value-param
    readability-avoid-const-params-in-decls

clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst

+.. title:: clang-tidy - modernize-use-emplace
+
+modernize-use-emplace
+=====================
+
+This check looks for cases when inserting new element into an STL
+container (``std::vector``, ``std::deque``, ``std::list``) or ``llvm::SmallVector``
+but the element is constructed temporarily.
+
+Before:
+
+.. code:: c++
+
+  std::vector<MyClass> v;
+  v.push_back(MyClass(21, 37));
+
+  std::vector<std::pair<int,int> > w;
+  w.push_back(std::make_pair(21, 37));
+
+After:
+
+.. code:: c++
+
+  std::vector<MyClass> v;
+  v.emplace_back(21, 37);
+
+  std::vector<std::pair<int,int> > w;
+  v.emplace_back(21, 37);
+
+The other situation is when we pass arguments that will be converted to a type
+inside a container.
+
+Before:
+
+.. code:: c++
+
+  std::vector<boost::optional<std::string> > v;
+  v.push_back("abc");
+
+After:
+
+.. code:: c++
+
+  std::vector<boost::optional<std::string> > v;
+  v.emplace_back("abc");
+
+
+In this case the calls of ``push_back`` won't be replaced.
+
+.. code:: c++
+  std::vector<std::unique_ptr<int> > v;
+  v.push_back(new int(5));
+  auto *ptr = int;
+  v.push_back(ptr);
+
+This is because replacing it with ``emplace_back`` could cause a leak of this
+pointer if ``emplace_back`` would throw exception before emplacement
+(e.g. not enough memory to add new element).
+
+For more info read item 42 - "Consider emplacement instead of insertion."
+of Scott Meyers Efective Modern C++.
+
+Check also fires if any argument of constructor call would be:
+- bitfield (bitfields can't bind to rvalue/universal reference)
+- ``new`` expression (to avoid leak)
+or if the argument would be converted via derived-to-base cast.
+
+This check requires C++11 of higher to run.
+

clang-tools-extra/trunk/test/clang-tidy/modernize-use-emplace.cpp

+// RUN: %check_clang_tidy %s modernize-use-emplace %t
+
+namespace std {
+template <typename T>
+class vector {
+public:
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template <typename... Args>
+  void emplace_back(Args &&... args){};
+};
+template <typename T>
+class list {
+public:
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template <typename... Args>
+  void emplace_back(Args &&... args){};
+};
+
+template <typename T>
+class deque {
+public:
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template <typename... Args>
+  void emplace_back(Args &&... args){};
+};
+
+template <typename T1, typename T2>
+class pair {
+public:
+  pair() = default;
+  pair(const pair &) = default;
+  pair(pair &&) = default;
+
+  pair(const T1 &, const T2 &) {}
+  pair(T1 &&, T2 &&) {}
+
+  template <class U1, class U2>
+  pair(const pair<U1, U2> &p){};
+  template <class U1, class U2>
+  pair(pair<U1, U2> &&p){};
+};
+
+template <typename T1, typename T2>
+pair<T1, T2> make_pair(T1, T2) {
+  return pair<T1, T2>();
+};
+
+template <typename T>
+class unique_ptr {
+public:
+  unique_ptr(T *) {}
+};
+} // namespace std
+
+void testInts() {
+  std::vector<int> v;
+  v.push_back(42);
+  v.push_back(int(42));
+  v.push_back(int{42});
+  v.push_back(42.0);
+  int z;
+  v.push_back(z);
+}
+
+struct Something {
+  Something(int a, int b = 41) {}
+  Something() {}
+  void push_back(Something);
+};
+
+struct Convertable {
+  operator Something() { return Something{}; }
+};
+
+struct Zoz {
+  Zoz(Something, int = 42) {}
+};
+
+Zoz getZoz(Something s) { return Zoz(s); }
+
+void test_Something() {
+  std::vector<Something> v;
+
+  v.push_back(Something(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back instead of push_back [modernize-use-emplace]
+  // CHECK-FIXES: v.emplace_back(1, 2);
+
+  v.push_back(Something{1, 2});
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
+
+  v.push_back(Something());
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back();
+
+  v.push_back(Something{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back();
+
+  v.push_back(42);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(42);
+
+  Something temporary(42, 42);
+  temporary.push_back(temporary);
+  v.push_back(temporary);
+
+  v.push_back(Convertable());
+  v.push_back(Convertable{});
+  Convertable s;
+  v.push_back(s);
+}
+
+void test2() {
+  std::vector<Zoz> v;
+  v.push_back(Zoz(Something(21, 37)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(Something(21, 37));
+
+  v.push_back(Zoz(Something(21, 37), 42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(Something(21, 37), 42);
+
+  v.push_back(getZoz(Something(1, 2)));
+}
+
+void testPair() {
+  std::vector<std::pair<int, int>> v;
+  v.push_back(std::pair<int, int>(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2);
+
+  std::vector<std::pair<Something, Zoz>> v2;
+  v2.push_back(std::pair<Something, Zoz>(Something(42, 42), Zoz(Something(21, 37))));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+  // CHECK-FIXES: v2.emplace_back(Something(42, 42), Zoz(Something(21, 37)));
+}
+
+struct Base {
+  Base(int, int *, int = 42);
+};
+
+struct Derived : Base {
+  Derived(int *, Something) : Base(42, nullptr) {}
+};
+
+void testDerived() {
+  std::vector<Base> v;
+  v.push_back(Derived(nullptr, Something{}));
+}
+
+void testNewExpr() {
+  std::vector<Derived> v;
+  v.push_back(Derived(new int, Something{}));
+}
+
+void testSpaces() {
+  std::vector<Something> v;
+
+  // clang-format off
+
+  v.push_back(Something(1, //arg1
+                2 // arg2
+               ) // Something
+              );
+  // CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, //arg1
+  // CHECK-FIXES:                2 // arg2
+  // CHECK-FIXES:                  // Something
+  // CHECK-FIXES:                );
+
+  v.push_back(    Something   (1, 2)    );
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2   );
+
+  v.push_back(    Something   {1, 2}    );
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2   );
+
+  v.push_back(  Something {}    );
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(   );
+
+  v.push_back(
+             Something(1, 2)    );
+  // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, 2   );
+
+  std::vector<Base> v2;
+  v2.push_back(
+    Base(42, nullptr));
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: warning: use emplace_back
+  // CHECK-FIXES: v2.emplace_back(42, nullptr);
+
+  // clang-format on
+}
+
+void testPointers() {
+  std::vector<int *> v;
+  v.push_back(new int(5));
+
+  std::vector<std::unique_ptr<int>> v2;
+  v2.push_back(new int(42));
+  // This call can't be replaced with emplace_back.
+  // If emplacement will fail (not enough memory to add to vector)
+  // we will have leak of int because unique_ptr won't be constructed
+  // (and destructed) as in push_back case.
+
+  auto *ptr = new int;
+  v2.push_back(ptr);
+  // Same here
+}
+
+void testMakePair() {
+  std::vector<std::pair<int, int>> v;
+  // FIXME: add functionality to change calls of std::make_pair
+  v.push_back(std::make_pair(1, 2));
+
+  // FIXME: This is not a bug, but call of make_pair should be removed in the
+  // future. This one matches because the return type of make_pair is different
+  // than the pair itself.
+  v.push_back(std::make_pair(42LL, 13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_pair(42LL, 13));
+}
+
+void testOtherCointainers() {
+  std::list<Something> l;
+  l.push_back(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: l.emplace_back(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);
+}
+
+class IntWrapper {
+public:
+  IntWrapper(int x) : value(x) {}
+  IntWrapper operator+(const IntWrapper other) const {
+    return IntWrapper(value + other.value);
+  }
+
+private:
+  int value;
+};
+
+void testMultipleOpsInPushBack() {
+  std::vector<IntWrapper> v;
+  v.push_back(IntWrapper(42) + IntWrapper(27));
+}
+
+// Macro tests.
+#define PUSH_BACK_WHOLE(c, x) c.push_back(x)
+#define PUSH_BACK_NAME push_back
+#define PUSH_BACK_ARG(x) (x)
+#define SOME_OBJ Something(10)
+#define MILLION 3
+#define SOME_WEIRD_PUSH(v) v.push_back(Something(
+#define OPEN (
+#define CLOSE )
+void macroTest() {
+  std::vector<Something> v;
+  Something s;
+
+  PUSH_BACK_WHOLE(v, Something(5, 6));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use emplace_back
+
+  v.PUSH_BACK_NAME(Something(5));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+
+  v.push_back PUSH_BACK_ARG(Something(5, 6));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+
+  v.push_back(SOME_OBJ);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+
+  v.push_back(Something(MILLION));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(MILLION);
+
+  // clang-format off
+  v.push_back(  Something OPEN 3 CLOSE  );
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // clang-format on
+  PUSH_BACK_WHOLE(s, Something(1));
+}
+
+struct A {
+  int value1, value2;
+};
+
+struct B {
+  B(A) {}
+};
+
+struct C {
+  int value1, value2, value3;
+};
+
+void testAggregation() {
+  // This should not be noticed or fixed; after the correction, the code won't
+  // compile.
+
+  std::vector<A> v;
+  v.push_back(A({1, 2}));
+
+  std::vector<B> vb;
+  vb.push_back(B({10, 42}));
+}
+
+struct Bitfield {
+  unsigned bitfield : 1;
+  unsigned notBitfield;
+};
+
+void testBitfields() {
+  std::vector<Something> v;
+  Bitfield b;
+  v.push_back(Something(42, b.bitfield));
+  v.push_back(Something(b.bitfield));
+
+  v.push_back(Something(42, b.notBitfield));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(42, b.notBitfield);
+  int var;
+  v.push_back(Something(42, var));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(42, var);
+}