kuhar created this revision.
kuhar added a project: clang-tools-extra.

This patch makes modernize-use-emplace remove unnecessary make_tuple calls from 
push_back calls and turn them into emplace_back -- the same way make_pair calls 
are handled.

Eq.

  std::vector<std::tuple<int, char, bool>> v;
  v.push_back(std::make_tuple(1, 'A', true)); // --> v.emplace_back(1, 'A', 
true);


https://reviews.llvm.org/D32690

Files:
  clang-tidy/modernize/UseEmplaceCheck.cpp
  docs/ReleaseNotes.rst
  test/clang-tidy/modernize-use-emplace.cpp

Index: test/clang-tidy/modernize-use-emplace.cpp
===================================================================
--- test/clang-tidy/modernize-use-emplace.cpp
+++ test/clang-tidy/modernize-use-emplace.cpp
@@ -36,27 +36,54 @@
   ~deque();
 };
 
-template <typename T1, typename T2>
-class pair {
+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> 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 U1, typename U2> pair(const pair<U1, U2> &){};
+  template <typename U1, typename U2> pair(pair<U1, U2> &&){};
 };
 
 template <typename T1, typename T2>
-pair<T1, T2> make_pair(T1&&, T2&&) {
+pair<typename remove_reference<T1>::type, typename remove_reference<T2>::type>
+make_pair(T1 &&, T2 &&) {
   return {};
 };
 
+template <typename... Ts> class tuple {
+public:
+  tuple() = default;
+  tuple(const tuple &) = default;
+  tuple(tuple &&) = default;
+
+  tuple(const Ts &...) {}
+  tuple(Ts &&...) {}
+
+  template <typename... Us> tuple(const tuple<Us...> &){};
+  template <typename... Us> tuple(tuple<Us...> &&) {}
+
+  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> &&) {
+    static_assert(sizeof...(Ts) == 2, "Wrong tuple size");
+  };
+};
+
+template <typename... Ts>
+tuple<typename remove_reference<Ts>::type...> make_tuple(Ts &&...) {
+  return {};
+}
+
 template <typename T>
 class unique_ptr {
 public:
@@ -197,6 +224,30 @@
   // CHECK-FIXES: v2.emplace_back(Something(42, 42), Zoz(Something(21, 37)));
 }
 
+void testTuple() {
+  std::vector<std::tuple<bool, char, int>> v;
+  v.push_back(std::tuple<bool, char, int>(false, 'x', 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(false, 'x', 1);
+
+  v.push_back(std::tuple<bool, char, int>{false, 'y', 2});
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(false, 'y', 2);
+
+  v.push_back({true, 'z', 3});
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(true, 'z', 3);
+
+  std::vector<std::tuple<int, bool>> x;
+  x.push_back(std::make_pair(1, false));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: x.emplace_back(1, false);
+
+  x.push_back(std::make_pair(2LL, 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: x.emplace_back(2LL, 1);
+}
+
 struct Base {
   Base(int, int *, int = 42);
 };
@@ -318,6 +369,23 @@
   // make_pair cannot be removed here, as Y is not constructible with two ints.
 }
 
+void testMakeTuple() {
+  std::vector<std::tuple<int, bool, char>> v;
+  v.push_back(std::make_tuple(1, true, 'v'));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(1, true, 'v');
+
+  v.push_back(std::make_tuple(2ULL, 1, 0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(2ULL, 1, 0);
+
+  v.push_back(std::make_tuple<long long, int, int>(3LL, 1, 0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
+  // CHECK-FIXES: v.emplace_back(std::make_tuple<long long, int, int>(3LL, 1, 0));
+  // make_tuple is not removed when there are explicit template
+  // arguments provided.
+}
+
 void testOtherContainers() {
   std::list<Something> l;
   l.push_back(Something(42, 41));
@@ -437,7 +505,7 @@
   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
+    // Check currently doesn't support friend declarations because pretty much
     // nobody would want to be friend with std::vector :(.
     v.push_back(PrivateCtor(42));
   }
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -109,8 +109,8 @@
 - Improved `modernize-use-emplace
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html>`_ check
 
-  Removes unnecessary std::make_pair calls in push_back(std::make_pair(a, b)) calls and turns them
-  into emplace_back(a, b).
+  Removes unnecessary ``std::make_pair`` and ``std::make_tuple`` calls in push_back calls
+  and turns them into emplace_back.
 
 Improvements to include-fixer
 -----------------------------
Index: clang-tidy/modernize/UseEmplaceCheck.cpp
===================================================================
--- clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -85,21 +85,23 @@
           .bind("ctor");
   auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr));
 
-  auto MakePair = ignoringImplicit(
-      callExpr(callee(expr(ignoringImplicit(
-          declRefExpr(unless(hasExplicitTemplateArgs()),
-                      to(functionDecl(hasName("::std::make_pair"))))
-      )))).bind("make_pair"));
+  auto MakePairOrTuple = ignoringImplicit(
+      callExpr(callee(expr(ignoringImplicit(declRefExpr(
+          unless(hasExplicitTemplateArgs()),
+          to(functionDecl(hasAnyName("::std::make_pair",
+                                     "::std::make_tuple"))))))))
+          .bind("make"));
 
   // make_pair can return type convertible to container's element type.
   // Allow the conversion only on containers of pairs.
-  auto MakePairCtor = ignoringImplicit(cxxConstructExpr(
-      has(materializeTemporaryExpr(MakePair)),
-      hasDeclaration(cxxConstructorDecl(ofClass(hasName("::std::pair"))))));
+  auto MakePairOrTupleCtor = ignoringImplicit(
+      cxxConstructExpr(has(materializeTemporaryExpr(MakePairOrTuple)),
+                       hasDeclaration(cxxConstructorDecl(ofClass(
+                           hasAnyName("::std::pair", "::std::tuple"))))));
 
   auto SoughtParam = materializeTemporaryExpr(
-      anyOf(has(MakePair), has(MakePairCtor),
-            HasConstructExpr, has(cxxFunctionalCastExpr(HasConstructExpr))));
+      anyOf(has(MakePairOrTuple), has(MakePairOrTupleCtor), HasConstructExpr,
+            has(cxxFunctionalCastExpr(HasConstructExpr))));
 
   Finder->addMatcher(cxxMemberCallExpr(CallPushBack, has(SoughtParam),
                                        unless(isInTemplateInstantiation()))
@@ -110,8 +112,8 @@
 void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
   const auto *InnerCtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
-  const auto *MakePairCall = Result.Nodes.getNodeAs<CallExpr>("make_pair");
-  assert((InnerCtorCall || MakePairCall) && "No push_back parameter matched");
+  const auto *MakeCall = Result.Nodes.getNodeAs<CallExpr>("make");
+  assert((InnerCtorCall || MakeCall) && "No push_back parameter matched");
 
   const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
       Call->getExprLoc(), Call->getArg(0)->getExprLoc());
@@ -121,20 +123,20 @@
   if (FunctionNameSourceRange.getBegin().isMacroID())
     return;
 
-  const auto *EmplacePrefix = MakePairCall ? "emplace_back" : "emplace_back(";
+  const auto *EmplacePrefix = MakeCall ? "emplace_back" : "emplace_back(";
   Diag << FixItHint::CreateReplacement(FunctionNameSourceRange, EmplacePrefix);
 
   const SourceRange CallParensRange =
-      MakePairCall ? SourceRange(MakePairCall->getCallee()->getLocEnd(),
-                                 MakePairCall->getRParenLoc())
-                   : InnerCtorCall->getParenOrBraceRange();
+      MakeCall ? SourceRange(MakeCall->getCallee()->getLocEnd(),
+                             MakeCall->getRParenLoc())
+               : InnerCtorCall->getParenOrBraceRange();
 
   // Finish if there is no explicit constructor call.
   if (CallParensRange.getBegin().isInvalid())
     return;
 
   const SourceLocation ExprBegin =
-      MakePairCall ? MakePairCall->getExprLoc() : InnerCtorCall->getExprLoc();
+      MakeCall ? MakeCall->getExprLoc() : InnerCtorCall->getExprLoc();
 
   // Range for constructor name and opening brace.
   const auto ParamCallSourceRange =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to