[clang-tools-extra] 987f9cb - [clang-tidy] Add proper emplace checks to modernize-use-emplace

Ivan Murashko via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 2 16:15:21 PDT 2022


Author: Nicolas van Kempen
Date: 2022-06-03T00:14:57+01:00
New Revision: 987f9cb6b9708ff16980c4f8c3f8ba4674463fb5

URL: https://github.com/llvm/llvm-project/commit/987f9cb6b9708ff16980c4f8c3f8ba4674463fb5
DIFF: https://github.com/llvm/llvm-project/commit/987f9cb6b9708ff16980c4f8c3f8ba4674463fb5.diff

LOG: [clang-tidy] Add proper emplace checks to modernize-use-emplace

modernize-use-emplace only recommends going from a push_back to an
emplace_back, but does not provide a recommendation when emplace_back is
improperly used. This adds the functionality of warning the user when
an unecessary temporary is created while calling emplace_back or other "emplacy"
functions from the STL containers.

Reviewed By: kuhar, ivanmurashko

Differential Revision: https://reviews.llvm.org/D101471

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
    clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
    clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst
    clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index e1db4e04f53f7..e6fb4c0a87264 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -15,6 +15,69 @@ namespace tidy {
 namespace modernize {
 
 namespace {
+// Identical to hasAnyName, except it does not take template specifiers into
+// account. This is used to match the functions names as in
+// DefaultEmplacyFunctions below without caring about the template types of the
+// containers.
+AST_MATCHER_P(NamedDecl, hasAnyNameIgnoringTemplates, std::vector<StringRef>,
+              Names) {
+  const std::string FullName = "::" + Node.getQualifiedNameAsString();
+
+  // This loop removes template specifiers by only keeping characters not within
+  // template brackets. We keep a depth count to handle nested templates. For
+  // example, it'll transform a::b<c<d>>::e<f> to simply a::b::e.
+  std::string FullNameTrimmed;
+  int Depth = 0;
+  for (const auto &Character : FullName) {
+    if (Character == '<') {
+      ++Depth;
+    } else if (Character == '>') {
+      --Depth;
+    } else if (Depth == 0) {
+      FullNameTrimmed.append(1, Character);
+    }
+  }
+
+  // This loop is taken from HasNameMatcher::matchesNodeFullSlow in
+  // clang/lib/ASTMatchers/ASTMatchersInternal.cpp and checks whether
+  // FullNameTrimmed matches any of the given Names.
+  const StringRef FullNameTrimmedRef = FullNameTrimmed;
+  for (const StringRef Pattern : Names) {
+    if (Pattern.startswith("::")) {
+      if (FullNameTrimmed == Pattern)
+        return true;
+    } else if (FullNameTrimmedRef.endswith(Pattern) &&
+               FullNameTrimmedRef.drop_back(Pattern.size()).endswith("::")) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+// Checks if the given matcher is the last argument of the given CallExpr.
+AST_MATCHER_P(CallExpr, hasLastArgument,
+              clang::ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+    return false;
+
+  return InnerMatcher.matches(*Node.getArg(Node.getNumArgs() - 1), Finder,
+                              Builder);
+}
+
+// Checks if the given member call has the same number of arguments as the
+// function had parameters defined (this is useful to check if there is only one
+// variadic argument).
+AST_MATCHER(CXXMemberCallExpr, hasSameNumArgsAsDeclNumParams) {
+  if (Node.getMethodDecl()->isFunctionTemplateSpecialization())
+    return Node.getNumArgs() == Node.getMethodDecl()
+                                    ->getPrimaryTemplate()
+                                    ->getTemplatedDecl()
+                                    ->getNumParams();
+
+  return Node.getNumArgs() == Node.getMethodDecl()->getNumParams();
+}
+
 AST_MATCHER(DeclRefExpr, hasExplicitTemplateArgs) {
   return Node.hasExplicitTemplateArgs();
 }
@@ -25,6 +88,20 @@ const auto DefaultSmartPointers =
     "::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
 const auto DefaultTupleTypes = "::std::pair; ::std::tuple";
 const auto DefaultTupleMakeFunctions = "::std::make_pair; ::std::make_tuple";
+const auto DefaultEmplacyFunctions =
+    "vector::emplace_back; vector::emplace;"
+    "deque::emplace; deque::emplace_front; deque::emplace_back;"
+    "forward_list::emplace_after; forward_list::emplace_front;"
+    "list::emplace; list::emplace_back; list::emplace_front;"
+    "set::emplace; set::emplace_hint;"
+    "map::emplace; map::emplace_hint;"
+    "multiset::emplace; multiset::emplace_hint;"
+    "multimap::emplace; multimap::emplace_hint;"
+    "unordered_set::emplace; unordered_set::emplace_hint;"
+    "unordered_map::emplace; unordered_map::emplace_hint;"
+    "unordered_multiset::emplace; unordered_multiset::emplace_hint;"
+    "unordered_multimap::emplace; unordered_multimap::emplace_hint;"
+    "stack::emplace; queue::emplace; priority_queue::emplace";
 } // namespace
 
 UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
@@ -37,7 +114,9 @@ UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
       TupleTypes(utils::options::parseStringList(
           Options.get("TupleTypes", DefaultTupleTypes))),
       TupleMakeFunctions(utils::options::parseStringList(
-          Options.get("TupleMakeFunctions", DefaultTupleMakeFunctions))) {}
+          Options.get("TupleMakeFunctions", DefaultTupleMakeFunctions))),
+      EmplacyFunctions(utils::options::parseStringList(
+          Options.get("EmplacyFunctions", DefaultEmplacyFunctions))) {}
 
 void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   // FIXME: Bunch of functionality that could be easily added:
@@ -52,6 +131,13 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
       hasDeclaration(functionDecl(hasName("push_back"))),
       on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushBack)))));
 
+  auto CallEmplacy = cxxMemberCallExpr(
+      hasDeclaration(
+          functionDecl(hasAnyNameIgnoringTemplates(EmplacyFunctions))),
+      on(hasType(cxxRecordDecl(has(typedefNameDecl(
+          hasName("value_type"), hasType(type(hasUnqualifiedDesugaredType(
+                                     recordType().bind("value_type"))))))))));
+
   // 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
@@ -73,8 +159,9 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   auto ConstructingDerived =
       hasParent(implicitCastExpr(hasCastKind(CastKind::CK_DerivedToBase)));
 
-  // emplace_back can't access private constructor.
-  auto IsPrivateCtor = hasDeclaration(cxxConstructorDecl(isPrivate()));
+  // emplace_back can't access private or protected constructors.
+  auto IsPrivateOrProtectedCtor =
+      hasDeclaration(cxxConstructorDecl(anyOf(isPrivate(), isProtected())));
 
   auto HasInitList = anyOf(has(ignoringImplicit(initListExpr())),
                            has(cxxStdInitializerListExpr()));
@@ -85,7 +172,7 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
       cxxConstructExpr(
           unless(anyOf(IsCtorOfSmartPtr, HasInitList, BitFieldAsArgument,
                        InitializerListAsArgument, NewExprAsArgument,
-                       ConstructingDerived, IsPrivateCtor)))
+                       ConstructingDerived, IsPrivateOrProtectedCtor)))
           .bind("ctor");
   auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr));
 
@@ -102,22 +189,64 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
       hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName(TupleTypes))))));
 
   auto SoughtParam = materializeTemporaryExpr(
-      anyOf(has(MakeTuple), has(MakeTupleCtor),
-            HasConstructExpr, has(cxxFunctionalCastExpr(HasConstructExpr))));
+      anyOf(has(MakeTuple), has(MakeTupleCtor), HasConstructExpr,
+            has(cxxFunctionalCastExpr(HasConstructExpr))));
+
+  auto HasConstructExprWithValueTypeType =
+      has(ignoringImplicit(cxxConstructExpr(
+          SoughtConstructExpr, hasType(type(hasUnqualifiedDesugaredType(
+                                   type(equalsBoundNode("value_type"))))))));
+
+  auto HasConstructExprWithValueTypeTypeAsLastArgument =
+      hasLastArgument(materializeTemporaryExpr(anyOf(
+          HasConstructExprWithValueTypeType,
+          has(cxxFunctionalCastExpr(HasConstructExprWithValueTypeType)))));
 
   Finder->addMatcher(
       traverse(TK_AsIs, cxxMemberCallExpr(CallPushBack, has(SoughtParam),
                                           unless(isInTemplateInstantiation()))
-                            .bind("call")),
+                            .bind("push_back_call")),
+      this);
+
+  Finder->addMatcher(
+      traverse(TK_AsIs,
+               cxxMemberCallExpr(
+                   CallEmplacy, HasConstructExprWithValueTypeTypeAsLastArgument,
+                   hasSameNumArgsAsDeclNumParams(),
+                   unless(isInTemplateInstantiation()))
+                   .bind("emplacy_call")),
+      this);
+
+  Finder->addMatcher(
+      traverse(
+          TK_AsIs,
+          cxxMemberCallExpr(
+              CallEmplacy,
+              on(hasType(cxxRecordDecl(has(typedefNameDecl(
+                  hasName("value_type"),
+                  hasType(type(
+                      hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+                          cxxRecordDecl(hasAnyName(SmallVector<StringRef, 2>(
+                              TupleTypes.begin(), TupleTypes.end()))))))))))))),
+              has(MakeTuple), hasSameNumArgsAsDeclNumParams(),
+              unless(isInTemplateInstantiation()))
+              .bind("emplacy_call")),
       this);
 }
 
 void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+  const auto *PushBackCall =
+      Result.Nodes.getNodeAs<CXXMemberCallExpr>("push_back_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 = PushBackCall ? PushBackCall : EmplacyCall;
+
   if (IgnoreImplicitConstructors && CtorCall && CtorCall->getNumArgs() >= 1 &&
       CtorCall->getArg(0)->getSourceRange() == CtorCall->getSourceRange())
     return;
@@ -125,13 +254,21 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
       Call->getExprLoc(), Call->getArg(0)->getExprLoc());
 
-  auto Diag = diag(Call->getExprLoc(), "use emplace_back instead of push_back");
+  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());
 
   if (FunctionNameSourceRange.getBegin().isMacroID())
     return;
 
-  const auto *EmplacePrefix = MakeCall ? "emplace_back" : "emplace_back(";
-  Diag << FixItHint::CreateReplacement(FunctionNameSourceRange, EmplacePrefix);
+  if (PushBackCall) {
+    const char *EmplacePrefix = MakeCall ? "emplace_back" : "emplace_back(";
+    Diag << FixItHint::CreateReplacement(FunctionNameSourceRange,
+                                         EmplacePrefix);
+  }
 
   const SourceRange CallParensRange =
       MakeCall ? SourceRange(MakeCall->getCallee()->getEndLoc(),
@@ -143,7 +280,7 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
     return;
 
   const SourceLocation ExprBegin =
-      MakeCall ? MakeCall->getExprLoc() : CtorCall->getExprLoc();
+      CtorCall ? CtorCall->getExprLoc() : MakeCall->getExprLoc();
 
   // Range for constructor name and opening brace.
   const auto ParamCallSourceRange =
@@ -151,7 +288,14 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
 
   Diag << FixItHint::CreateRemoval(ParamCallSourceRange)
        << FixItHint::CreateRemoval(CharSourceRange::getTokenRange(
-           CallParensRange.getEnd(), CallParensRange.getEnd()));
+              CallParensRange.getEnd(), CallParensRange.getEnd()));
+
+  if (MakeCall && EmplacyCall) {
+    // Remove extra left parenthesis
+    Diag << FixItHint::CreateRemoval(
+        CharSourceRange::getCharRange(MakeCall->getCallee()->getEndLoc(),
+                                      MakeCall->getArg(0)->getBeginLoc()));
+  }
 }
 
 void UseEmplaceCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
@@ -164,6 +308,8 @@ void UseEmplaceCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
                 utils::options::serializeStringList(TupleTypes));
   Options.store(Opts, "TupleMakeFunctions",
                 utils::options::serializeStringList(TupleMakeFunctions));
+  Options.store(Opts, "EmplacyFunctions",
+                utils::options::serializeStringList(EmplacyFunctions));
 }
 
 } // namespace modernize

diff  --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
index 096f4e0164cc1..45a1d5dba43f5 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
@@ -40,6 +40,7 @@ class UseEmplaceCheck : public ClangTidyCheck {
   const std::vector<StringRef> SmartPointers;
   const std::vector<StringRef> TupleTypes;
   const std::vector<StringRef> TupleMakeFunctions;
+  const std::vector<StringRef> EmplacyFunctions;
 };
 
 } // namespace modernize

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst
index 8533c89589977..f08bfb4449c32 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize-use-emplace.rst
@@ -15,17 +15,29 @@ because replacing ``insert`` with ``emplace`` may result in
 By default only ``std::vector``, ``std::deque``, ``std::list`` are considered.
 This list can be modified using the :option:`ContainersWithPushBack` option.
 
+This check also reports when an ``emplace``-like method is improperly used,
+for example using ``emplace_back`` while also calling a constructor. This
+creates a temporary that requires at best a move and at worst a copy. Almost all
+``emplace``-like functions in the STL are covered by this, with ``try_emplace``
+on ``std::map`` and ``std::unordered_map`` being the exception as it behaves
+slightly 
diff erently than all the others. More containers can be added with the
+:option:`EmplacyFunctions` option, so long as the container defines a
+``value_type`` type, and the ``emplace``-like functions construct a
+``value_type`` object.
+
 Before:
 
 .. code-block:: c++
 
     std::vector<MyClass> v;
     v.push_back(MyClass(21, 37));
+    v.emplace_back(MyClass(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));
+    w.emplace_back(std::make_pair(21L, 37L));
 
 After:
 
@@ -33,10 +45,12 @@ After:
 
     std::vector<MyClass> v;
     v.emplace_back(21, 37);
+    v.emplace_back(21, 37);
 
     std::vector<std::pair<int, int>> w;
     w.emplace_back(21, 37);
     w.emplace_back(21L, 37L);
+    w.emplace_back(21L, 37L);
 
 By default, the check is able to remove unnecessary ``std::make_pair`` and
 ``std::make_tuple`` calls from ``push_back`` calls on containers of
@@ -128,6 +142,13 @@ Options
     function calls will be removed from ``push_back`` calls and turned into
     ``emplace_back``.
 
+.. option:: EmplacyFunctions
+
+    Semicolon-separated list of containers without their template parameters
+    and some ``emplace``-like method of the container. Example:
+    ``vector::emplace_back``. Those methods will be checked for improper use and
+    the check will report when a temporary is unnecessarily created.
+
 Example
 ^^^^^^^
 
@@ -135,6 +156,7 @@ Example
 
   std::vector<MyTuple<int, bool, char>> x;
   x.push_back(MakeMyTuple(1, false, 'x'));
+  x.emplace_back(MakeMyTuple(1, false, 'x'));
 
 transforms to:
 
@@ -142,6 +164,8 @@ transforms to:
 
   std::vector<MyTuple<int, bool, char>> x;
   x.emplace_back(1, false, 'x');
+  x.emplace_back(1, false, 'x');
 
-when :option:`TupleTypes` is set to ``MyTuple`` and :option:`TupleMakeFunctions`
-is set to ``MakeMyTuple``.
+when :option:`TupleTypes` is set to ``MyTuple``, :option:`TupleMakeFunctions`
+is set to ``MakeMyTuple``, and :option:`EmplacyFunctions` is set to
+``vector::emplace_back``.

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
index 4e74d49d33fb7..b869816f8d1da 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-emplace.cpp
@@ -10,15 +10,36 @@
 
 namespace std {
 template <typename>
-class initializer_list
-{
+class initializer_list {
 public:
   initializer_list() noexcept {}
 };
 
+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 <typename U1, typename U2>
+  pair(const pair<U1, U2> &){};
+  template <typename U1, typename U2>
+  pair(pair<U1, U2> &&){};
+};
+
 template <typename T>
 class vector {
 public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
   vector() = default;
   vector(initializer_list<T>) {}
 
@@ -27,54 +48,230 @@ class vector {
 
   template <typename... Args>
   void emplace_back(Args &&... args){};
+  template <typename... Args>
+  iterator emplace(const_iterator pos, Args &&...args){};
   ~vector();
 };
+
 template <typename T>
 class list {
 public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
   void push_back(const T &) {}
   void push_back(T &&) {}
 
+  template <typename... Args>
+  iterator emplace(const_iterator pos, Args &&...args){};
   template <typename... Args>
   void emplace_back(Args &&... args){};
+  template <typename... Args>
+  void emplace_front(Args &&...args){};
   ~list();
 };
 
 template <typename T>
 class deque {
 public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
   void push_back(const T &) {}
   void push_back(T &&) {}
 
+  template <typename... Args>
+  iterator emplace(const_iterator pos, Args &&...args){};
   template <typename... Args>
   void emplace_back(Args &&... args){};
+  template <typename... Args>
+  void emplace_front(Args &&...args){};
   ~deque();
 };
 
-template <typename T> struct remove_reference { using type = T; };
-template <typename T> struct remove_reference<T &> { using type = T; };
-template <typename T> struct remove_reference<T &&> { using type = T; };
+template <typename T>
+class forward_list {
+public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template <typename... Args>
+  void emplace_front(Args &&...args){};
+  template <typename... Args>
+  iterator emplace_after(const_iterator pos, Args &&...args){};
+};
 
-template <typename T1, typename T2> class pair {
+template <typename T>
+class set {
 public:
-  pair() = default;
-  pair(const pair &) = default;
-  pair(pair &&) = default;
+  using value_type = T;
 
-  pair(const T1 &, const T2 &) {}
-  pair(T1 &&, T2 &&) {}
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+  template <typename... Args>
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename Key, typename T>
+class map {
+public:
+  using value_type = std::pair<Key, T>;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+  template <typename... Args>
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename T>
+class multiset {
+public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+  template <typename... Args>
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename Key, typename T>
+class multimap {
+public:
+  using value_type = std::pair<Key, T>;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+  template <typename... Args>
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename T>
+class unordered_set {
+public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+  template <typename... Args>
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename Key, typename T>
+class unordered_map {
+public:
+  using value_type = std::pair<Key, T>;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+  template <typename... Args>
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename T>
+class unordered_multiset {
+public:
+  using value_type = T;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+  template <typename... Args>
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename Key, typename T>
+class unordered_multimap {
+public:
+  using value_type = std::pair<Key, T>;
+
+  class iterator {};
+  class const_iterator {};
+  const_iterator begin() { return const_iterator{}; }
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+  template <typename... Args>
+  iterator emplace_hint(const_iterator pos, Args &&...args){};
+};
+
+template <typename T>
+class stack {
+public:
+  using value_type = T;
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+};
 
-  template <typename U1, typename U2> pair(const pair<U1, U2> &){};
-  template <typename U1, typename U2> pair(pair<U1, U2> &&){};
+template <typename T>
+class queue {
+public:
+  using value_type = T;
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
 };
 
+template <typename T>
+class priority_queue {
+public:
+  using value_type = T;
+
+  template <typename... Args>
+  void emplace(Args &&...args){};
+};
+
+template <typename T>
+struct remove_reference { using type = T; };
+template <typename T>
+struct remove_reference<T &> { using type = T; };
+template <typename T>
+struct remove_reference<T &&> { using type = T; };
+
 template <typename T1, typename T2>
 pair<typename remove_reference<T1>::type, typename remove_reference<T2>::type>
 make_pair(T1 &&, T2 &&) {
   return {};
 };
 
-template <typename... Ts> class tuple {
+template <typename... Ts>
+class tuple {
 public:
   tuple() = default;
   tuple(const tuple &) = default;
@@ -83,13 +280,17 @@ template <typename... Ts> class tuple {
   tuple(const Ts &...) {}
   tuple(Ts &&...) {}
 
-  template <typename... Us> tuple(const tuple<Us...> &){};
-  template <typename... Us> tuple(tuple<Us...> &&) {}
+  template <typename... Us>
+  tuple(const tuple<Us...> &){};
+  template <typename... Us>
+  tuple(tuple<Us...> &&) {}
 
-  template <typename U1, typename U2> tuple(const pair<U1, U2> &) {
+  template <typename U1, typename U2>
+  tuple(const pair<U1, U2> &) {
     static_assert(sizeof...(Ts) == 2, "Wrong tuple size");
   };
-  template <typename U1, typename U2> tuple(pair<U1, U2> &&) {
+  template <typename U1, typename U2>
+  tuple(pair<U1, U2> &&) {
     static_assert(sizeof...(Ts) == 2, "Wrong tuple size");
   };
 };
@@ -118,7 +319,7 @@ class LikeASmallVector {
   void emplace_back(Args &&... args){};
 };
 
-} // llvm
+} // namespace llvm
 
 void testInts() {
   std::vector<int> v;
@@ -375,7 +576,7 @@ void testMakePair() {
   // make_pair cannot be removed here, as X is not constructible with two ints.
 
   struct Y {
-    Y(std::pair<int, int>&&) {}
+    Y(std::pair<int, int> &&) {}
   };
   std::vector<Y> y;
   y.push_back(std::make_pair(2, 3));
@@ -402,7 +603,8 @@ void testMakeTuple() {
 }
 
 namespace test {
-template <typename T> struct Single {
+template <typename T>
+struct Single {
   Single() = default;
   Single(const Single &) = default;
   Single(Single &&) = default;
@@ -410,11 +612,15 @@ template <typename T> struct Single {
   Single(const T &) {}
   Single(T &&) {}
 
-  template <typename U> Single(const Single<U> &) {}
-  template <typename U> Single(Single<U> &&) {}
+  template <typename U>
+  Single(const Single<U> &) {}
+  template <typename U>
+  Single(Single<U> &&) {}
 
-  template <typename U> Single(const std::tuple<U> &) {}
-  template <typename U> Single(std::tuple<U> &&) {}
+  template <typename U>
+  Single(const std::tuple<U> &) {}
+  template <typename U>
+  Single(std::tuple<U> &&) {}
 };
 
 template <typename T>
@@ -605,3 +811,240 @@ void testInitializerList() {
   x.push_back(PairIntVector(3, {4}));
   x.push_back({5, {6}});
 }
+
+class Foo {
+public:
+  Foo(){};
+  Foo(int){};
+  Foo(int, int){};
+  Foo(std::pair<int, int>){};
+
+protected:
+  Foo(char *) : Foo(){};
+};
+
+void testSomeEmplaceCases() {
+  std::vector<std::pair<char *, char *>> v1;
+  std::vector<Foo> v2;
+  std::unordered_map<int, char *> m1;
+
+  v1.emplace_back(std::make_pair("foo", "bar"));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: v1.emplace_back("foo", "bar");
+
+  char *foo = "bar";
+  v1.emplace_back(std::make_pair(foo, "bar"));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: v1.emplace_back(foo, "bar");
+
+  v1.emplace(v1.begin(), std::make_pair("foo", "bar"));
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: v1.emplace(v1.begin(), "foo", "bar");
+
+  v2.emplace_back(Foo());
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: v2.emplace_back();
+
+  v2.emplace_back(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: v2.emplace_back(13);
+
+  v2.emplace_back(Foo{13});
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: v2.emplace_back(13);
+
+  int a = 31;
+  v2.emplace_back(Foo(13, a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: v2.emplace_back(13, a);
+
+  v2.emplace_back(std::make_pair(3, 3));
+
+  m1.emplace(std::make_pair(13, "foo"));
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: m1.emplace(13, "foo");
+
+  std::vector<std::pair<int, int>> v3;
+  v3.emplace_back(std::pair<int, int>(13, 71));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  v3.emplace_back(std::make_pair(13, 71));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+
+  std::vector<std::tuple<int, int, int>> v4;
+  v4.emplace_back(std::tuple<int, int, int>(13, 31, 71));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  v4.emplace_back(std::make_tuple(13, 31, 71));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+
+  std::vector<test::Single<int>> v5;
+  v5.emplace_back(test::Single<int>(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+  v5.emplace_back(test::MakeSingle(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+}
+
+void testAllSTLEmplacyFunctions() {
+  std::vector<Foo> vector;
+  std::deque<Foo> deque;
+  std::forward_list<Foo> forward_list;
+  std::list<Foo> list;
+  std::set<Foo> set;
+  std::map<int, Foo> map;
+  std::multiset<Foo> multiset;
+  std::multimap<int, Foo> multimap;
+  std::unordered_set<Foo> unordered_set;
+  std::unordered_map<int, Foo> unordered_map;
+  std::unordered_multiset<Foo> unordered_multiset;
+  std::unordered_multimap<int, Foo> unordered_multimap;
+  std::stack<Foo> stack;
+  std::queue<Foo> queue;
+  std::priority_queue<Foo> priority_queue;
+
+  vector.emplace_back(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: vector.emplace_back(13);
+
+  vector.emplace(vector.begin(), Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: vector.emplace(vector.begin(), 13);
+
+  deque.emplace(deque.begin(), Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: deque.emplace(deque.begin(), 13);
+
+  deque.emplace_front(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: unnecessary temporary object created while calling emplace_front
+  // CHECK-FIXES: deque.emplace_front(13);
+
+  deque.emplace_back(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: deque.emplace_back(13);
+
+  forward_list.emplace_after(forward_list.begin(), Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:52: warning: unnecessary temporary object created while calling emplace_after
+  // CHECK-FIXES: forward_list.emplace_after(forward_list.begin(), 13);
+
+  forward_list.emplace_front(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: unnecessary temporary object created while calling emplace_front
+  // CHECK-FIXES: forward_list.emplace_front(13);
+
+  list.emplace(list.begin(), Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: list.emplace(list.begin(), 13);
+
+  list.emplace_back(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: unnecessary temporary object created while calling emplace_back
+  // CHECK-FIXES: list.emplace_back(13);
+
+  list.emplace_front(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: unnecessary temporary object created while calling emplace_front
+  // CHECK-FIXES: list.emplace_front(13);
+
+  set.emplace(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: set.emplace(13);
+
+  set.emplace_hint(set.begin(), Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: unnecessary temporary object created while calling emplace_hint
+  // CHECK-FIXES: set.emplace_hint(set.begin(), 13);
+
+  map.emplace(std::make_pair(13, Foo(13)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: map.emplace(13, Foo(13));
+
+  map.emplace_hint(map.begin(), std::make_pair(13, Foo(13)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: unnecessary temporary object created while calling emplace_hint
+  // CHECK-FIXES: map.emplace_hint(map.begin(), 13, Foo(13));
+
+  multiset.emplace(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: multiset.emplace(13);
+
+  multiset.emplace_hint(multiset.begin(), Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: unnecessary temporary object created while calling emplace_hint
+  // CHECK-FIXES: multiset.emplace_hint(multiset.begin(), 13);
+
+  multimap.emplace(std::make_pair(13, Foo(13)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: multimap.emplace(13, Foo(13));
+
+  multimap.emplace_hint(multimap.begin(), std::make_pair(13, Foo(13)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: unnecessary temporary object created while calling emplace_hint
+  // CHECK-FIXES: multimap.emplace_hint(multimap.begin(), 13, Foo(13));
+
+  unordered_set.emplace(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: unordered_set.emplace(13);
+
+  unordered_set.emplace_hint(unordered_set.begin(), Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:53: warning: unnecessary temporary object created while calling emplace_hint
+  // CHECK-FIXES: unordered_set.emplace_hint(unordered_set.begin(), 13);
+
+  unordered_map.emplace(std::make_pair(13, Foo(13)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: unordered_map.emplace(13, Foo(13));
+
+  unordered_map.emplace_hint(unordered_map.begin(), std::make_pair(13, Foo(13)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:53: warning: unnecessary temporary object created while calling emplace_hint
+  // CHECK-FIXES: unordered_map.emplace_hint(unordered_map.begin(), 13, Foo(13));
+
+  unordered_multiset.emplace(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: unordered_multiset.emplace(13);
+  unordered_multiset.emplace_hint(unordered_multiset.begin(), Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:63: warning: unnecessary temporary object created while calling emplace_hint
+  // CHECK-FIXES: unordered_multiset.emplace_hint(unordered_multiset.begin(), 13);
+
+  unordered_multimap.emplace(std::make_pair(13, Foo(13)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: unordered_multimap.emplace(13, Foo(13));
+  unordered_multimap.emplace_hint(unordered_multimap.begin(), std::make_pair(13, Foo(13)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:63: warning: unnecessary temporary object created while calling emplace_hint
+  // CHECK-FIXES: unordered_multimap.emplace_hint(unordered_multimap.begin(), 13, Foo(13));
+
+  stack.emplace(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: stack.emplace(13);
+
+  queue.emplace(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: queue.emplace(13);
+
+  priority_queue.emplace(Foo(13));
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: unnecessary temporary object created while calling emplace
+  // CHECK-FIXES: priority_queue.emplace(13);
+}
+
+struct Bar {
+public:
+  Bar(){};
+  void testWithPrivateAndProtectedCtor() {
+    std::vector<Bar> vec;
+
+    vec.emplace_back(Bar(13));
+    vec.emplace_back(Bar(13, 13));
+  }
+
+protected:
+  Bar(int){};
+
+private:
+  Bar(int, int){};
+};
+
+void testPossibleFalsePositives() {
+  struct Y {
+    Y(std::pair<int, int> &&) {}
+  };
+  std::vector<Y> y;
+  y.emplace_back(std::make_pair(2, 3));
+
+  std::vector<std::pair<int, int>> v;
+  v.emplace_back(std::make_pair<char, char>(0, 3));
+
+  struct D {
+    D(...) {}
+    operator char() const { return 0; }
+  };
+  v.emplace_back(std::make_pair<D, int>(Something(), 2));
+}


        


More information about the cfe-commits mailing list