[clang-tidy] Fixes to modernize-use-emplace

[Prazek]

Summary

Not everything is valid (as you see in FIXME), but it should works for 99.8% cases.

I gonna fix it in next week, but I would like to land this bug fixes right now.

This is mainly fixes of the review that @sbenza gave me last time.

[Prazek]

BTW I've made clang-extra project on phabricator as you can see. Please use it, it will be much easier to filter reviews.

[aaron.ballman]

clang-tidy/modernize/UseEmplaceCheck.cpp

21

Should be Names instead.

21

Is this needed? We have the hasAnyName() matcher already.

31–33

These should start with a capital as well (coding standard information pertaining to names can be found at http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly).

32–34

These should be using ::std:: instead of std:: just in case people do awful things like embed namespace std inside of their own namespaces.

clang-tidy/modernize/UseEmplaceCheck.h

36

Should be Containers and Smart (capital letters).

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

21

The problem is that this matcher takes variadic number of strings. Here I have a vector of strings. sbenza did similar thing in performance-fast-string-find.

In the future I would like to add overload of the matcher that would take a vector of strings (it is is possible),
or at least move this function somewhere.

Another idea would be to return matcher instead of Optional<Matcher> - if list is empty then return unless(anything()) - there should be nothing() matcher :P

31–33

thanks, I always though that only local variables have to start with capital.

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

115

There is a bug here that I forgot about. Should be InnerCtorCall->getStartLoc()

[Prazek]

ping

[aaron.ballman]

clang-tidy/modernize/UseEmplaceCheck.cpp

21

Looking at VariadicFunction, it appears that it already works if you pass an ArrayRef<T> to the operator()() overload. See ASTMatchersInternal.h:81. So I still think the matcher can be used directly, just with changing the type of the object passed to the functor.

[alexfh]

clang-tidy/modernize/UseEmplaceCheck.cpp

31

A std::string is not needed here, please change to const char [].

clang-tidy/modernize/UseEmplaceCheck.h

36

Declarations with multiple variables can be misleading. Please split this into two separate declarations.

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

6–17

IMO, this sentence fails to convey the most important parts of the information. Consider changing to something like this:

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.

Maybe also explain, why the check only looks for push_back, and not insert, push_front and push.

9

s/; separated/semicolon-separated/

[Prazek]

Aaron, Alex thanks for the review. After running it on llvm I also have found the private constructor bug so I also fixed it.

clang-tidy/modernize/UseEmplaceCheck.cpp

21

That's cool! I convert the vector to SmallVector, because ArrayRef doesn't have ctor taking iterators.

I will later also change the code in FasterStringFind that also do this kind of things.

[aaron.ballman]

clang-tidy/modernize/UseEmplaceCheck.h

36–37

Why not use a SmallVector for these instead of a std::vector? Then you don't need to typecast in registerMatchers().

If it's because of parseStringList(), I think that you can work around it's tight coupling using llvm::iterator_range(parseStringList()) to perform the initialization in the ctor initializer list.

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.h

36–37

Unfortunatelly it doesn't want to work. I tried llvm::iterator_range<std::string*> and llvm::iterator_range<SmallVector<std::string, 5>::iterator> and it doesn't compile.

[alexfh]

clang-tidy/modernize/UseEmplaceCheck.cpp

45–46

What's the reason for explicitly creating a SmallVector? VariadicFunction::operator() takes an ArrayRef, which std::vector<> should be implicitly convertible to. Am I missing something?

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

12

s/chaning/changing/

12

How about the insert method of std::vector<>?

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

45–46

It is not. I guess it is because it is ArrayRef<StringRef> and I have std::vector<std::string>

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

12

insert -> emplace should be fine for the vector, but this patch is trying to mainly fix the bugs (and I want it to land in clang-3.9).

I will add `insert near the push_front`

[alexfh]

clang-tidy/modernize/UseEmplaceCheck.cpp

45–46

Fair enough.

115

There is a bug here that I forgot about. Should be InnerCtorCall->getStartLoc()

Is this still relevant?

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

12

SG

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

115

The thing is, that I have no idea. So I was expecting this to happen when I will run on llvm code base, but it seems that it works I don't know why.

So firstly, I marked wrong line, because I think the problem might be in line 94 (Call->getArg(0)->getExprLoc() should be ->getLocStart())

So what I was expecting that if The Call->getArg(0) will be a memberExpr (call to member function) then getExprLoc will point to dot instead of the beggining of argument, so
we would remove a object name like

v.push_back(obj.member());
into
v.emplace_back(member());

But I have no idea why I can't find it, and right now I don't know if it is a real bug or not, but it should be if I got a bug here
https://reviews.llvm.org/D21642

Do you have any thoughts/test case that would break it?

[Prazek]

There is one bug left:

In the ClangIncludeFixer.cpp:169 there is push back that looks like this

Symbols.push_back(find_all_symbols::SymbolInfo(

Split.first.trim(),
find_all_symbols::SymbolInfo::SymbolKind::Unknown,
CommaSplits[I].trim(), 1, {}, /*NumOccurrences=*/E - I));

There is initializer list inside that should not allow this to be matched (because I check for initializer list as argument)

Unfortunatelly clang AST doesn't mention any initializer list.

http://wklejto.pl/258691

As you can see the "{}" looks like this in AST

|-MaterializeTemporaryExpr 0x7f759d803900 <col:39, col:40> 'const std::vector<Context>':'const class std::vector<struct std::pair<enum clang::find_all_symbols::SymbolInfo::ContextType, class std::basic_string<char> >, class std::allocator<struct std::pair<enum clang::find_all_symbols::SymbolInfo::ContextType, class std::basic_string<char> > > >' lvalue
|   |   |               | `-CXXBindTemporaryExpr 0x7f759d8038e0 <col:39, col:40> 'const std::vector<Context>':'const class std::vector<struct std::pair<enum clang::find_all_symbols::SymbolInfo::ContextType, class std::basic_string<char> >, class std::allocator<struct std::pair<enum clang::find_all_symbols::SymbolInfo::ContextType, class std::basic_string<char> > > >' (CXXTemporary 0x7f759d8038d8)
|   |   |               |   `-CXXConstructExpr 0x7f759d8038a8 <col:39, col:40> 'const std::vector<Context>':'const class std::vector<struct std::pair<enum clang::find_all_symbols::SymbolInfo::ContextType, class std::basic_string<char> >, class std::allocator<struct std::pair<enum clang::find_all_symbols::SymbolInfo::ContextType, class std::basic_string<char> > > >' 'void (void)'
|   |   |

Good news are that this is the only 1 case from all push_backs that it found on LLVM+Clang+extra. This patch fixed the ExprWithCleanups by using ignoreImplicit, so I clang-tidy have many new push_backs.

[Prazek]

Can you look at my previous comment? I nedd an expert for AST.

[alexfh]

In D22208#492385, @Prazek wrote:

There is one bug left:

In the ClangIncludeFixer.cpp:169 there is push back that looks like this

Symbols.push_back(find_all_symbols::SymbolInfo(

Split.first.trim(),
find_all_symbols::SymbolInfo::SymbolKind::Unknown,
CommaSplits[I].trim(), 1, {}, /*NumOccurrences=*/E - I));

There is initializer list inside that should not allow this to be matched (because I check for initializer list as argument)

Unfortunatelly clang AST doesn't mention any initializer list.

I guess, CXXConstructExpr::isListInitialization() is what you need to check here.

[Prazek]

Ok works right now. I don't know why but I could not reproduce the error in test file, but I manged to fix it.

[alexfh]

LG

clang-tidy/modernize/UseEmplaceCheck.cpp

115

This doesn't seem to be an issue, since expression v.push_back(obj.member()) won't trigger this check: it expects that the argument of the push_back call is a cxxConstructExpr or a cxxFunctionalCastExpr.

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

115

what about the implicit conversion? What if obj.member would return object that is different from the one that v stores, but it is convertible to it?

[aaron.ballman]

clang-tidy/modernize/UseEmplaceCheck.h

36–37

What about llvm::make_range()?

[alexfh]

clang-tidy/modernize/UseEmplaceCheck.cpp

115

Sounds almost like a recipe for a test case ;) Have you tried to construct it this way?

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.h

36–37

llvm::make_range do pretty much the same thing. The patch is in upstream, so feel free to try it, but either I am doing something wrong, or just SmallVector/ArrayRef have this limitations.

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

115

Yes, I think I emulated this case in line 190. I tried to hit this problem but could not find the test case.

[alexfh]

Please add revision number (this can be automated, if include differential revision URL in your commit message as described in http://llvm.org/docs/Phabricator.html#committing-a-change).

[alexfh]

clang-tidy/modernize/UseEmplaceCheck.cpp

115

if The Call->getArg(0) will be a memberExpr (call to member function)

what about the implicit conversion?

I think, I know what happens in this case. When an implicit conversion happens in v.push_back(obj.member()), then Call->getArg(0) is an implicit CXXConstructExpr, not a MemberExpr (MemberExpr will be its indirect child). The case should be handled correctly.

[Prazek]

In D22208#503194, @alexfh wrote:

Please add revision number (this can be automated, if include differential revision URL in your commit message as described in http://llvm.org/docs/Phabricator.html#committing-a-change).

I normally use arc, but I was working in half from my laptop and in half from workstation, and I am not sure if arc would handle this.

I added the url in the patch, but I didn't know that "Differential Revision:" is required. I will keep it in mind next time.

[Prazek]

clang-tidy/modernize/UseEmplaceCheck.cpp

115

That might be it. I remember that in some early version I was biding to MemberExpr somewhere, which might cause it.

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"
+#include <string>
+#include <vector>
 
 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) {}
+  UseEmplaceCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  std::vector<std::string> ContainersWithPushBack;
+  std::vector<std::string> SmartPointers;
 };
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_EMPLACE_H

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"
-
+#include "../utils/OptionsUtils.h"
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
+static const auto DefaultContainersWithPushBack =
+    "::std::vector; ::std::list; ::std::deque";
+static const auto DefaultSmartPointers =
+    "::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
+
+UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      ContainersWithPushBack(utils::options::parseStringList(Options.get(
+          "ContainersWithPushBack", DefaultContainersWithPushBack))),
+      SmartPointers(utils::options::parseStringList(
+          Options.get("SmartPointers", DefaultSmartPointers))) {}
+
 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")))));
+      on(hasType(cxxRecordDecl(hasAnyName(SmallVector<StringRef, 5>(
+          ContainersWithPushBack.begin(), ContainersWithPushBack.end()))))));
 
   // 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"))));
+  auto isCtorOfSmartPtr = hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName(
+      SmallVector<StringRef, 5>(SmartPointers.begin(), SmartPointers.end())))));
 
   // Bitfields binds only to consts and emplace_back take it by universal ref.
-  auto bitFieldAsArgument = hasAnyArgument(ignoringParenImpCasts(
-      memberExpr(hasDeclaration(fieldDecl(matchers::isBitfield())))));
+  auto bitFieldAsArgument = hasAnyArgument(
+      ignoringImplicit(memberExpr(hasDeclaration(fieldDecl(isBitField())))));
+
+  // Initializer list can't be passed to universal reference.
+  auto initializerListAsArgument = hasAnyArgument(
+      ignoringImplicit(cxxConstructExpr(isListInitialization())));
 
   // We could have leak of resource.
-  auto newExprAsArgument = hasAnyArgument(ignoringParenImpCasts(cxxNewExpr()));
+  auto newExprAsArgument = hasAnyArgument(ignoringImplicit(cxxNewExpr()));
+  // We would call another constructor.
   auto constructingDerived =
       hasParent(implicitCastExpr(hasCastKind(CastKind::CK_DerivedToBase)));
 
-  auto hasInitList = has(ignoringParenImpCasts(initListExpr()));
+  // emplace_back can't access private constructor.
+  auto isPrivateCtor = hasDeclaration(cxxConstructorDecl(isPrivate()));
+
+  auto hasInitList = has(ignoringImplicit(initListExpr()));
+  // FIXME: Discard 0/NULL (as nullptr), static inline const data members,
+  // overloaded functions and template names.
   auto soughtConstructExpr =
       cxxConstructExpr(
           unless(anyOf(isCtorOfSmartPtr, hasInitList, bitFieldAsArgument,
-                       newExprAsArgument, constructingDerived,
-                       has(materializeTemporaryExpr(hasInitList)))))
+                       initializerListAsArgument, newExprAsArgument,
+                       constructingDerived, isPrivateCtor)))
           .bind("ctor");
-  auto hasConstructExpr = has(ignoringParenImpCasts(soughtConstructExpr));
+  auto hasConstructExpr = has(ignoringImplicit(soughtConstructExpr));
 
   auto ctorAsArgument = materializeTemporaryExpr(
       anyOf(hasConstructExpr, has(cxxFunctionalCastExpr(hasConstructExpr))));
 
-  Finder->addMatcher(
-      cxxMemberCallExpr(callPushBack, has(ctorAsArgument)).bind("call"), this);
+  Finder->addMatcher(cxxMemberCallExpr(callPushBack, has(ctorAsArgument),
+                                       unless(isInTemplateInstantiation()))
+                         .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));
+  auto CtorCallSourceRange = CharSourceRange::getTokenRange(
+      InnerCtorCall->getExprLoc(), CallParensRange.getBegin());
 
   Diag << FixItHint::CreateRemoval(CtorCallSourceRange)
-       << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
-              CallParensRange.getEnd(),
-              CallParensRange.getEnd().getLocWithOffset(1)));
+       << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
+              CallParensRange.getEnd(), CallParensRange.getEnd()));
+}
+
+void UseEmplaceCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "ContainersWithPushBack",
+                utils::options::serializeStringList(ContainersWithPushBack));
+  Options.store(Opts, "SmartPointers",
+                utils::options::serializeStringList(SmartPointers));
 }
 
 } // namespace modernize
 } // namespace tidy
 } // namespace clang

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(); }
-
 // Returns QualType matcher for references to const.
 AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, isReferenceToConst) {
   using namespace ast_matchers;
   return referenceType(pointee(qualType(isConstQualified())));
 }
 
 } // namespace matchers
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_MATCHERS_H

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.
+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
+`speed regression <http://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 by passing a semicolon-separated list of class names
+using the `ContainersWithPushBack` option.
 
 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));
+  std::vector<std::pair<int,int>> w;
+
+  w.push_back(std::pair<int,int>(21, 37));
+  w.push_back(std::make_pair(21L, 37L));
 
 After:
 
 .. code:: c++
 
   std::vector<MyClass> v;
   v.emplace_back(21, 37);
 
-  std::vector<std::pair<int,int> > w;
-  v.emplace_back(21, 37);
+  std::vector<std::pair<int,int>> w;
+  w.emplace_back(21, 37);
+  // This will be fixed to w.push_back(21, 37); in next version
+  w.emplace_back(std::make_pair(21L, 37L);
 
 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 some cases the transformation would be valid, but the code
+wouldn't be exception safe.
 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);
+    std::vector<std::unique_ptr<int>> v;
+  v.push_back(std::unique_ptr<int>(new int(0)));
+  auto *ptr = new int(1);
+  v.push_back(std::unique_ptr<int>(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++.
+of Scott Meyers Effective Modern C++.
+
+The default smart pointers that are considered are
+``std::unique_ptr``, ``std::shared_ptr``, ``std::auto_ptr``.
+To specify other smart pointers or other classes use option
+`SmartPointers` similar to `ContainersWithPushBack`.
+
 
 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.
-

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

-// RUN: %check_clang_tidy %s modernize-use-emplace %t
+// RUN: %check_clang_tidy %s modernize-use-emplace %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN:             [{key: modernize-use-emplace.ContainersWithPushBack, \
+// RUN:               value: '::std::vector; ::std::list; ::std::deque; llvm::LikeASmallVector'}]}" -- -std=c++11
 
 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){};
+  ~vector();
 };
 template <typename T>
 class list {
 public:
   void push_back(const T &) {}
   void push_back(T &&) {}
 
   template <typename... Args>
   void emplace_back(Args &&... args){};
+  ~list();
 };
 
 template <typename T>
 class deque {
 public:
   void push_back(const T &) {}
   void push_back(T &&) {}
 
   template <typename... Args>
   void emplace_back(Args &&... args){};
+  ~deque();
 };
 
 template <typename T1, typename T2>
 class pair {
 public:
   pair() = default;
   pair(const pair &) = default;
   pair(pair &&) = default;

 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 *) {}
+  explicit unique_ptr(T *) {}
+  ~unique_ptr();
 };
 } // namespace std
 
+namespace llvm {
+template <typename T>
+class LikeASmallVector {
+public:
+  void push_back(const T &) {}
+  void push_back(T &&) {}
+
+  template <typename... Args>
+  void emplace_back(Args &&... args){};
+};
+
+} // llvm
+
 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);
+  int getInt() { return 42; }
 };
 
 struct Convertable {
   operator Something() { return Something{}; }
 };
 
 struct Zoz {
   Zoz(Something, int = 42) {}
void test_Something() {
   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();
 
+  Something Different;
+  v.push_back(Something(Different.getInt(), 42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(Different.getInt(), 42);
+
+  v.push_back(Different.getInt());
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(Different.getInt());
+
   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);
 }
 
+template <typename ElemType>
+void dependOnElem() {
+  std::vector<ElemType> v;
+  v.push_back(ElemType(42));
+}
+
+template <typename ContainerType>
+void dependOnContainer() {
+  ContainerType v;
+  v.push_back(Something(42));
+}
+
+void callDependent() {
+  dependOnElem<Something>();
+  dependOnContainer<std::vector<Something>>();
+}
+
 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)));
 }
 
+struct GetPair {
+  std::pair<int, long> getPair();
+};
 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);
 
+  GetPair g;
+  v.push_back(g.getPair());
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(g.getPair());
+
   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);
void testSpaces() {
   // 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));
+  v2.push_back(std::unique_ptr<int>(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);
+  v2.push_back(std::unique_ptr<int>(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));
 
void testOtherCointainers() {
   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);
+
+  llvm::LikeASmallVector<Something> ls;
+  ls.push_back(Something(42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+  // CHECK-FIXES: ls.emplace_back(42);
 }
 
 class IntWrapper {
 public:
   IntWrapper(int x) : value(x) {}
   IntWrapper operator+(const IntWrapper other) const {
     return IntWrapper(value + other.value);
   }
void testBitfields() {
   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);
 }
+
+class PrivateCtor {
+  PrivateCtor(int z);
+
+public:
+  void doStuff() {
+    std::vector<PrivateCtor> v;
+    // This should not change it because emplace back doesn't have permission.
+    // Check currently doesn't support friend delcarations because pretty much
+    // nobody would want to be friend with std::vector :(.
+    v.push_back(PrivateCtor(42));
+  }
+};
+
+struct WithDtor {
+  WithDtor(int) {}
+  ~WithDtor();
+};
+
+void testWithDtor() {
+  std::vector<WithDtor> v;
+
+  v.push_back(WithDtor(42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(42);
+}