[clang-tools-extra] r303140 - Revert "[clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls"

Jakub Kuderski via cfe-commits cfe-commits at lists.llvm.org
Mon May 15 22:07:40 PDT 2017


Author: kuhar
Date: Tue May 16 00:07:40 2017
New Revision: 303140

URL: http://llvm.org/viewvc/llvm-project?rev=303140&view=rev
Log:
Revert "[clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls"

This reverts commit r303139. The commit made docs build emit a warning.

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

Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp?rev=303140&r1=303139&r2=303140&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp Tue May 16 00:07:40 2017
@@ -24,8 +24,6 @@ const auto DefaultContainersWithPushBack
     "::std::vector; ::std::list; ::std::deque";
 const auto DefaultSmartPointers =
     "::std::shared_ptr; ::std::unique_ptr; ::std::auto_ptr; ::std::weak_ptr";
-const auto DefaultTupleTypes = "::std::pair; ::std::tuple";
-const auto DefaultTupleMakeFunctions = "::std::make_pair; ::std::make_tuple";
 } // namespace
 
 UseEmplaceCheck::UseEmplaceCheck(StringRef Name, ClangTidyContext *Context)
@@ -33,11 +31,7 @@ UseEmplaceCheck::UseEmplaceCheck(StringR
       ContainersWithPushBack(utils::options::parseStringList(Options.get(
           "ContainersWithPushBack", DefaultContainersWithPushBack))),
       SmartPointers(utils::options::parseStringList(
-          Options.get("SmartPointers", DefaultSmartPointers))),
-      TupleTypes(utils::options::parseStringList(
-          Options.get("TupleTypes", DefaultTupleTypes))),
-      TupleMakeFunctions(utils::options::parseStringList(
-          Options.get("TupleMakeFunctions", DefaultTupleMakeFunctions))) {}
+          Options.get("SmartPointers", DefaultSmartPointers))) {}
 
 void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
@@ -93,23 +87,20 @@ void UseEmplaceCheck::registerMatchers(M
           .bind("ctor");
   auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr));
 
-  auto MakeTuple = ignoringImplicit(
-      callExpr(
-          callee(expr(ignoringImplicit(declRefExpr(
-              unless(hasExplicitTemplateArgs()),
-              to(functionDecl(hasAnyName(SmallVector<StringRef, 2>(
-                  TupleMakeFunctions.begin(), TupleMakeFunctions.end())))))))))
-          .bind("make"));
+  auto MakePair = ignoringImplicit(
+      callExpr(callee(expr(ignoringImplicit(
+          declRefExpr(unless(hasExplicitTemplateArgs()),
+                      to(functionDecl(hasName("::std::make_pair"))))
+      )))).bind("make_pair"));
 
-  // make_something can return type convertible to container's element type.
+  // make_pair can return type convertible to container's element type.
   // Allow the conversion only on containers of pairs.
-  auto MakeTupleCtor = ignoringImplicit(cxxConstructExpr(
-      has(materializeTemporaryExpr(MakeTuple)),
-      hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName(
-          SmallVector<StringRef, 2>(TupleTypes.begin(), TupleTypes.end())))))));
+  auto MakePairCtor = ignoringImplicit(cxxConstructExpr(
+      has(materializeTemporaryExpr(MakePair)),
+      hasDeclaration(cxxConstructorDecl(ofClass(hasName("::std::pair"))))));
 
   auto SoughtParam = materializeTemporaryExpr(
-      anyOf(has(MakeTuple), has(MakeTupleCtor),
+      anyOf(has(MakePair), has(MakePairCtor),
             HasConstructExpr, has(cxxFunctionalCastExpr(HasConstructExpr))));
 
   Finder->addMatcher(cxxMemberCallExpr(CallPushBack, has(SoughtParam),
@@ -121,8 +112,8 @@ void UseEmplaceCheck::registerMatchers(M
 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 *MakeCall = Result.Nodes.getNodeAs<CallExpr>("make");
-  assert((InnerCtorCall || MakeCall) && "No push_back parameter matched");
+  const auto *MakePairCall = Result.Nodes.getNodeAs<CallExpr>("make_pair");
+  assert((InnerCtorCall || MakePairCall) && "No push_back parameter matched");
 
   const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
       Call->getExprLoc(), Call->getArg(0)->getExprLoc());
@@ -132,20 +123,20 @@ void UseEmplaceCheck::check(const MatchF
   if (FunctionNameSourceRange.getBegin().isMacroID())
     return;
 
-  const auto *EmplacePrefix = MakeCall ? "emplace_back" : "emplace_back(";
+  const auto *EmplacePrefix = MakePairCall ? "emplace_back" : "emplace_back(";
   Diag << FixItHint::CreateReplacement(FunctionNameSourceRange, EmplacePrefix);
 
   const SourceRange CallParensRange =
-      MakeCall ? SourceRange(MakeCall->getCallee()->getLocEnd(),
-                             MakeCall->getRParenLoc())
-               : InnerCtorCall->getParenOrBraceRange();
+      MakePairCall ? SourceRange(MakePairCall->getCallee()->getLocEnd(),
+                                 MakePairCall->getRParenLoc())
+                   : InnerCtorCall->getParenOrBraceRange();
 
   // Finish if there is no explicit constructor call.
   if (CallParensRange.getBegin().isInvalid())
     return;
 
   const SourceLocation ExprBegin =
-      MakeCall ? MakeCall->getExprLoc() : InnerCtorCall->getExprLoc();
+      MakePairCall ? MakePairCall->getExprLoc() : InnerCtorCall->getExprLoc();
 
   // Range for constructor name and opening brace.
   const auto ParamCallSourceRange =
@@ -161,10 +152,6 @@ void UseEmplaceCheck::storeOptions(Clang
                 utils::options::serializeStringList(ContainersWithPushBack));
   Options.store(Opts, "SmartPointers",
                 utils::options::serializeStringList(SmartPointers));
-  Options.store(Opts, "TupleTypes",
-                utils::options::serializeStringList(TupleTypes));
-  Options.store(Opts, "TupleMakeFunctions",
-                utils::options::serializeStringList(TupleMakeFunctions));
 }
 
 } // namespace modernize

Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.h?rev=303140&r1=303139&r2=303140&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.h (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.h Tue May 16 00:07:40 2017
@@ -35,8 +35,6 @@ public:
 private:
   std::vector<std::string> ContainersWithPushBack;
   std::vector<std::string> SmartPointers;
-  std::vector<std::string> TupleTypes;
-  std::vector<std::string> TupleMakeFunctions;
 };
 
 } // namespace modernize

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=303140&r1=303139&r2=303140&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Tue May 16 00:07:40 2017
@@ -91,10 +91,8 @@ Improvements to clang-tidy
 - Improved `modernize-use-emplace
   <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html>`_ check
 
-  Removes unnecessary ``std::make_pair`` and ``std::make_tuple`` calls in
-  push_back calls and turns them into emplace_back. The check now also is able
-  to remove user-defined make functions from ``push_back`` calls on containers
-  of custom tuple-like types by providing `TupleTypes` and `TupleMakeFunctions`.
+  Removes unnecessary std::make_pair calls in push_back(std::make_pair(a, b)) calls and turns them
+  into emplace_back(a, b).
 
 - New `performance-inefficient-vector-operation
   <http://clang.llvm.org/extra/clang-tidy/checks/performance-inefficient-vector-operation.html>`_ check

Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst?rev=303140&r1=303139&r2=303140&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst Tue May 16 00:07:40 2017
@@ -38,12 +38,6 @@ After:
     w.emplace_back(21, 37);
     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
-``std::pair``s and ``std::tuple``s. Custom tuple-like types can be modified by
-the :option:`TupleTypes` option; custom make functions can be modified by the
-:option:`TupleMakeFunctions` option.
-
 The other situation is when we pass arguments that will be converted to a type
 inside a container.
 
@@ -105,31 +99,3 @@ Options
 .. option:: SmartPointers
 
    Semicolon-separated list of class names of custom smart pointers.
-
-.. opion:: TupleTypes
-
-    Semicolon-separated list of ``std::tuple``-like class names.
-
-.. option:: TupleMakeFunctions
-
-    Semicolon-separated list of ``std::make_tuple``-like function names. Those
-    function calls will be removed from ``push_back`` calls and turned into
-    ``emplace_back``.
-
-Example
-^^^^^^^
-
-.. code-block:: c++
-
-  std::vector<MyTuple<int, bool, char>> x;
-  x.push_back(MakeMyTuple(1, false, 'x'));
-
-transforms to:
-
-.. code-block:: c++
-
-  std::vector<MyTuple<int, bool, char>> x;
-  x.emplace_back(1, false, 'x');
-
-when :option:`TupleTypes` is set to ``MyTuple`` and :option:`TupleMakeFunctions`
-is set to ``MakeMyTuple``.

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-use-emplace.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-emplace.cpp?rev=303140&r1=303139&r2=303140&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-emplace.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-emplace.cpp Tue May 16 00:07:40 2017
@@ -1,12 +1,7 @@
 // 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'}, \
-// RUN:              {key: modernize-use-emplace.TupleTypes, \
-// RUN:               value: '::std::pair; std::tuple; ::test::Single'}, \
-// RUN:              {key: modernize-use-emplace.TupleMakeFunctions, \
-// RUN:               value: '::std::make_pair; ::std::make_tuple; ::test::MakeSingle'}] \
-// RUN:             }" -- -std=c++11
+// RUN:               value: '::std::vector; ::std::list; ::std::deque; llvm::LikeASmallVector'}]}" -- -std=c++11
 
 namespace std {
 template <typename>
@@ -51,11 +46,8 @@ public:
   ~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 T1, typename T2> class pair {
+template <typename T1, typename T2>
+class pair {
 public:
   pair() = default;
   pair(const pair &) = default;
@@ -64,41 +56,17 @@ public:
   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 <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<typename remove_reference<T1>::type, typename remove_reference<T2>::type>
-make_pair(T1 &&, T2 &&) {
+pair<T1, T2> 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:
@@ -239,30 +207,6 @@ void testPair() {
   // 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);
 };
@@ -384,77 +328,6 @@ void testMakePair() {
   // 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.
-}
-
-namespace test {
-template <typename T> struct Single {
-  Single() = default;
-  Single(const Single &) = default;
-  Single(Single &&) = default;
-
-  Single(const T &) {}
-  Single(T &&) {}
-
-  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 T>
-Single<typename std::remove_reference<T>::type> MakeSingle(T &&) {
-  return {};
-}
-} // namespace test
-
-void testOtherTuples() {
-  std::vector<test::Single<int>> v;
-  v.push_back(test::Single<int>(1));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(1);
-
-  v.push_back({2});
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(2);
-
-  v.push_back(test::MakeSingle(3));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(3);
-
-  v.push_back(test::MakeSingle<long long>(4));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(test::MakeSingle<long long>(4));
-  // We don't remove make functions with explicit template parameters.
-
-  v.push_back(test::MakeSingle(5LL));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(5LL);
-
-  v.push_back(std::make_tuple(6));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(6);
-
-  v.push_back(std::make_tuple(7LL));
-  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
-  // CHECK-FIXES: v.emplace_back(7LL);
-}
-
 void testOtherContainers() {
   std::list<Something> l;
   l.push_back(Something(42, 41));
@@ -574,7 +447,7 @@ 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 declarations because pretty much
+    // Check currently doesn't support friend delcarations because pretty much
     // nobody would want to be friend with std::vector :(.
     v.push_back(PrivateCtor(42));
   }




More information about the cfe-commits mailing list