Author: kuhar Date: Tue May 16 01:32:38 2017 New Revision: 303145 URL: http://llvm.org/viewvc/llvm-project?rev=303145&view=rev Log: [clang-tidy] modernize-use-emplace: Remove unnecessary make_tuple calls
Summary: This patch makes modernize-use-emplace remove unnecessary make_ calls from push_back calls and turn them into emplace_back -- the same way make_pair calls are handled. Custom make_ calls can be removed for custom tuple-like types -- two new options that control that are `TupleTypes` and `TupleMakeFunctions`. By default, the check removes calls to `std::make_pair` and `std::make_tuple`. Eq. ``` std::vector<std::tuple<int, char, bool>> v; v.push_back(std::make_tuple(1, 'A', true)); // --> v.emplace_back(1, 'A', true); ``` Reviewers: alexfh, aaron.ballman, Prazek, hokein Reviewed By: Prazek Subscribers: JDevlieghere, xazax.hun, JonasToth, cfe-commits Tags: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D32690 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=303145&r1=303144&r2=303145&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.cpp Tue May 16 01:32:38 2017 @@ -24,6 +24,8 @@ 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) @@ -31,7 +33,11 @@ UseEmplaceCheck::UseEmplaceCheck(StringR ContainersWithPushBack(utils::options::parseStringList(Options.get( "ContainersWithPushBack", DefaultContainersWithPushBack))), SmartPointers(utils::options::parseStringList( - Options.get("SmartPointers", DefaultSmartPointers))) {} + Options.get("SmartPointers", DefaultSmartPointers))), + TupleTypes(utils::options::parseStringList( + Options.get("TupleTypes", DefaultTupleTypes))), + TupleMakeFunctions(utils::options::parseStringList( + Options.get("TupleMakeFunctions", DefaultTupleMakeFunctions))) {} void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) { if (!getLangOpts().CPlusPlus11) @@ -87,20 +93,23 @@ void UseEmplaceCheck::registerMatchers(M .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 MakeTuple = ignoringImplicit( + callExpr( + callee(expr(ignoringImplicit(declRefExpr( + unless(hasExplicitTemplateArgs()), + to(functionDecl(hasAnyName(SmallVector<StringRef, 2>( + TupleMakeFunctions.begin(), TupleMakeFunctions.end()))))))))) + .bind("make")); - // make_pair can return type convertible to container's element type. + // make_something 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 MakeTupleCtor = ignoringImplicit(cxxConstructExpr( + has(materializeTemporaryExpr(MakeTuple)), + hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName( + SmallVector<StringRef, 2>(TupleTypes.begin(), TupleTypes.end()))))))); auto SoughtParam = materializeTemporaryExpr( - anyOf(has(MakePair), has(MakePairCtor), + anyOf(has(MakeTuple), has(MakeTupleCtor), HasConstructExpr, has(cxxFunctionalCastExpr(HasConstructExpr)))); Finder->addMatcher(cxxMemberCallExpr(CallPushBack, has(SoughtParam), @@ -112,8 +121,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 *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()); @@ -123,20 +132,20 @@ void UseEmplaceCheck::check(const MatchF 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 = @@ -152,6 +161,10 @@ 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=303145&r1=303144&r2=303145&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/modernize/UseEmplaceCheck.h Tue May 16 01:32:38 2017 @@ -35,6 +35,8 @@ 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=303145&r1=303144&r2=303145&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original) +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Tue May 16 01:32:38 2017 @@ -91,8 +91,10 @@ 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 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. 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`. - 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=303145&r1=303144&r2=303145&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 01:32:38 2017 @@ -38,6 +38,12 @@ 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`` and ``std::tuple``. 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. @@ -99,3 +105,31 @@ Options .. option:: SmartPointers Semicolon-separated list of class names of custom smart pointers. + +.. option:: 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=303145&r1=303144&r2=303145&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 01:32:38 2017 @@ -1,7 +1,12 @@ // 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 +// 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 namespace std { template <typename> @@ -46,8 +51,11 @@ public: ~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; @@ -56,17 +64,41 @@ public: 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: @@ -207,6 +239,30 @@ 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); }; @@ -328,6 +384,77 @@ 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)); @@ -447,7 +574,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 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)); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
