diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp index 7de494bcedbb..48fbbfc22ce6 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp @@ -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(StringRef Name, ClangTidyContext *Context) 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(MatchFinder *Finder) { .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( + 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(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(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 *MakePairCall = Result.Nodes.getNodeAs("make_pair"); - assert((InnerCtorCall || MakePairCall) && "No push_back parameter matched"); + const auto *MakeCall = Result.Nodes.getNodeAs("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 MatchFinder::MatchResult &Result) { 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(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 72e1cfd3258c..5bd0d8e1bdbf 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h @@ -35,6 +35,8 @@ 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 44d893e82fe3..dfa1ffe72b5f 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -91,8 +91,10 @@ Improvements to clang-tidy - Improved `modernize-use-emplace `_ 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 `_ 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 53fb7a7be75b..dee79ab8e3a8 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,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> 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 b670061a16d3..c03d0241e201 100644 --- a/clang-tools-extra/test/clang-tidy/modernize-use-emplace.cpp +++ b/clang-tools-extra/test/clang-tidy/modernize-use-emplace.cpp @@ -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 @@ -46,8 +51,11 @@ public: ~deque(); }; -template -class pair { +template struct remove_reference { using type = T; }; +template struct remove_reference { using type = T; }; +template struct remove_reference { using type = T; }; + +template class pair { public: pair() = default; pair(const pair &) = default; @@ -56,17 +64,41 @@ public: pair(const T1 &, const T2 &) {} pair(T1 &&, T2 &&) {} - template - pair(const pair &p){}; - template - pair(pair &&p){}; + template pair(const pair &){}; + template pair(pair &&){}; }; template -pair make_pair(T1&&, T2&&) { +pair::type, typename remove_reference::type> +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: @@ -207,6 +239,30 @@ 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); }; @@ -328,6 +384,77 @@ 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)); @@ -447,7 +574,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 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)); }