From 9ac6c0398986e011ab6a156b791356e20f790f17 Mon Sep 17 00:00:00 2001 From: Jakub Kuderski Date: Tue, 16 May 2017 05:07:40 +0000 Subject: [PATCH] Revert "[clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls" This reverts commit r303139. The commit made docs build emit a warning. llvm-svn: 303140 --- .../clang-tidy/modernize/UseEmplaceCheck.cpp | 49 +++---- .../clang-tidy/modernize/UseEmplaceCheck.h | 2 - clang-tools-extra/docs/ReleaseNotes.rst | 6 +- .../clang-tidy/checks/modernize-use-emplace.rst | 34 ----- .../test/clang-tidy/modernize-use-emplace.cpp | 145 ++------------------- 5 files changed, 29 insertions(+), 207 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp index 48fbbfc..7de494b 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp @@ -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(StringRef Name, ClangTidyContext *Context) 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(MatchFinder *Finder) { .bind("ctor"); auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr)); - auto MakeTuple = ignoringImplicit( - callExpr( - callee(expr(ignoringImplicit(declRefExpr( - unless(hasExplicitTemplateArgs()), - to(functionDecl(hasAnyName(SmallVector( - 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(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(MatchFinder *Finder) { void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) { const auto *Call = Result.Nodes.getNodeAs("call"); const auto *InnerCtorCall = Result.Nodes.getNodeAs("ctor"); - const auto *MakeCall = Result.Nodes.getNodeAs("make"); - assert((InnerCtorCall || MakeCall) && "No push_back parameter matched"); + const auto *MakePairCall = Result.Nodes.getNodeAs("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 MatchFinder::MatchResult &Result) { 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(ClangTidyOptions::OptionMap &Opts) { 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 diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h index 5bd0d8e..72e1cfd 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h @@ -35,8 +35,6 @@ public: private: std::vector ContainersWithPushBack; std::vector SmartPointers; - std::vector TupleTypes; - std::vector TupleMakeFunctions; }; } // namespace modernize diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index dfa1ffe..44d893e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -91,10 +91,8 @@ Improvements to clang-tidy - Improved `modernize-use-emplace `_ 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 `_ check 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 026c3ff..53fb7a7 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 @@ -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> x; - x.push_back(MakeMyTuple(1, false, 'x')); - -transforms to: - -.. code-block:: c++ - - std::vector> x; - x.emplace_back(1, false, 'x'); - -when :option:`TupleTypes` is set to ``MyTuple`` and :option:`TupleMakeFunctions` -is set to ``MakeMyTuple``. diff --git a/clang-tools-extra/test/clang-tidy/modernize-use-emplace.cpp b/clang-tools-extra/test/clang-tidy/modernize-use-emplace.cpp index c03d024..b670061 100644 --- a/clang-tools-extra/test/clang-tidy/modernize-use-emplace.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-use-emplace.cpp @@ -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 @@ -51,11 +46,8 @@ public: ~deque(); }; -template struct remove_reference { using type = T; }; -template struct remove_reference { using type = T; }; -template struct remove_reference { using type = T; }; - -template class pair { +template +class pair { public: pair() = default; pair(const pair &) = default; @@ -64,41 +56,17 @@ public: pair(const T1 &, const T2 &) {} pair(T1 &&, T2 &&) {} - template pair(const pair &){}; - template pair(pair &&){}; + template + pair(const pair &p){}; + template + pair(pair &&p){}; }; template -pair::type, typename remove_reference::type> -make_pair(T1 &&, T2 &&) { +pair make_pair(T1&&, T2&&) { return {}; }; -template class tuple { -public: - tuple() = default; - tuple(const tuple &) = default; - tuple(tuple &&) = default; - - tuple(const Ts &...) {} - tuple(Ts &&...) {} - - template tuple(const tuple &){}; - template tuple(tuple &&) {} - - template tuple(const pair &) { - static_assert(sizeof...(Ts) == 2, "Wrong tuple size"); - }; - template tuple(pair &&) { - static_assert(sizeof...(Ts) == 2, "Wrong tuple size"); - }; -}; - -template -tuple::type...> make_tuple(Ts &&...) { - return {}; -} - template 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> v; - v.push_back(std::tuple(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{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> 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> 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(3LL, 1, 0)); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back - // CHECK-FIXES: v.emplace_back(std::make_tuple(3LL, 1, 0)); - // make_tuple is not removed when there are explicit template - // arguments provided. -} - -namespace test { -template struct Single { - Single() = default; - Single(const Single &) = default; - Single(Single &&) = default; - - Single(const T &) {} - Single(T &&) {} - - template Single(const Single &) {} - template Single(Single &&) {} - - template Single(const std::tuple &) {} - template Single(std::tuple &&) {} -}; - -template -Single::type> MakeSingle(T &&) { - return {}; -} -} // namespace test - -void testOtherTuples() { - std::vector> v; - v.push_back(test::Single(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(4)); - // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back - // CHECK-FIXES: v.emplace_back(test::MakeSingle(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 l; l.push_back(Something(42, 41)); @@ -574,7 +447,7 @@ public: void doStuff() { std::vector 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)); } -- 2.7.4