diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp --- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp @@ -167,7 +167,8 @@ // Initializer list can't be passed to universal reference. auto InitializerListAsArgument = hasAnyArgument( - ignoringImplicit(cxxConstructExpr(isListInitialization()))); + ignoringImplicit(allOf(cxxConstructExpr(isListInitialization()), + unless(cxxTemporaryObjectExpr())))); // We could have leak of resource. auto NewExprAsArgument = hasAnyArgument(ignoringImplicit(cxxNewExpr())); @@ -192,6 +193,16 @@ .bind("ctor"); auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr)); + // allow for T{} to be replaced, even if no CTOR is declared + auto HasConstructInitListExpr = has(initListExpr(anyOf( + allOf(has(SoughtConstructExpr), + has(cxxConstructExpr(argumentCountIs(0)))), + has(cxxBindTemporaryExpr(has(SoughtConstructExpr), + has(cxxConstructExpr(argumentCountIs(0)))))))); + auto HasBracedInitListExpr = + anyOf(has(cxxBindTemporaryExpr(HasConstructInitListExpr)), + HasConstructInitListExpr); + auto MakeTuple = ignoringImplicit( callExpr(callee(expr(ignoringImplicit(declRefExpr( unless(hasExplicitTemplateArgs()), @@ -204,19 +215,35 @@ has(materializeTemporaryExpr(MakeTuple)), hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName(TupleTypes)))))); - auto SoughtParam = materializeTemporaryExpr( - anyOf(has(MakeTuple), has(MakeTupleCtor), HasConstructExpr, - has(cxxFunctionalCastExpr(HasConstructExpr)))); + auto SoughtParam = + materializeTemporaryExpr( + anyOf(has(MakeTuple), has(MakeTupleCtor), HasConstructExpr, + HasBracedInitListExpr, + has(cxxFunctionalCastExpr(HasConstructExpr)), + has(cxxFunctionalCastExpr(HasBracedInitListExpr)))) + .bind("temporary_expr"); auto HasConstructExprWithValueTypeType = has(ignoringImplicit(cxxConstructExpr( SoughtConstructExpr, hasType(type(hasUnqualifiedDesugaredType( type(equalsBoundNode("value_type")))))))); - auto HasConstructExprWithValueTypeTypeAsLastArgument = - hasLastArgument(materializeTemporaryExpr(anyOf( - HasConstructExprWithValueTypeType, - has(cxxFunctionalCastExpr(HasConstructExprWithValueTypeType))))); + auto HasBracedInitListWithValueTypeType = + anyOf(allOf(HasConstructInitListExpr, + has(initListExpr(hasType(type(hasUnqualifiedDesugaredType( + type(equalsBoundNode("value_type")))))))), + has(cxxBindTemporaryExpr( + HasConstructInitListExpr, + has(initListExpr(hasType(type(hasUnqualifiedDesugaredType( + type(equalsBoundNode("value_type")))))))))); + + auto HasConstructExprWithValueTypeTypeAsLastArgument = hasLastArgument( + materializeTemporaryExpr( + anyOf(HasConstructExprWithValueTypeType, + HasBracedInitListWithValueTypeType, + has(cxxFunctionalCastExpr(HasConstructExprWithValueTypeType)), + has(cxxFunctionalCastExpr(HasBracedInitListWithValueTypeType)))) + .bind("temporary_expr")); Finder->addMatcher( traverse(TK_AsIs, cxxMemberCallExpr(CallPushBack, has(SoughtParam), @@ -272,6 +299,8 @@ Result.Nodes.getNodeAs("emplacy_call"); const auto *CtorCall = Result.Nodes.getNodeAs("ctor"); const auto *MakeCall = Result.Nodes.getNodeAs("make"); + const auto *TemporaryExpr = + Result.Nodes.getNodeAs("temporary_expr"); const CXXMemberCallExpr *Call = [&]() { if (PushBackCall) { @@ -298,7 +327,9 @@ auto Diag = EmplacyCall - ? diag(CtorCall ? CtorCall->getBeginLoc() : MakeCall->getBeginLoc(), + ? diag(TemporaryExpr ? TemporaryExpr->getBeginLoc() + : CtorCall ? CtorCall->getBeginLoc() + : MakeCall->getBeginLoc(), "unnecessary temporary object created while calling %0") : diag(Call->getExprLoc(), "use emplace%select{|_back|_front}0 " "instead of push%select{|_back|_front}0"); @@ -337,16 +368,23 @@ if (CallParensRange.getBegin().isInvalid()) return; + // FIXME: Will there ever be a CtorCall, if there is no TemporaryExpr? const SourceLocation ExprBegin = - CtorCall ? CtorCall->getExprLoc() : MakeCall->getExprLoc(); + TemporaryExpr + ? TemporaryExpr->getExprLoc() + : CtorCall ? CtorCall->getExprLoc() : MakeCall->getExprLoc(); // Range for constructor name and opening brace. const auto ParamCallSourceRange = CharSourceRange::getTokenRange(ExprBegin, CallParensRange.getBegin()); + // Range for constructor closing brace and end of temporary expr. + const auto EndCallSourceRange = CharSourceRange::getTokenRange( + CallParensRange.getEnd(), + TemporaryExpr ? TemporaryExpr->getEndLoc() : CallParensRange.getEnd()); + Diag << FixItHint::CreateRemoval(ParamCallSourceRange) - << FixItHint::CreateRemoval(CharSourceRange::getTokenRange( - CallParensRange.getEnd(), CallParensRange.getEnd())); + << FixItHint::CreateRemoval(EndCallSourceRange); if (MakeCall && EmplacyCall) { // Remove extra left parenthesis diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp @@ -1170,3 +1170,167 @@ }; v.emplace_back(std::make_pair(Something(), 2)); } + +struct InnerType { + InnerType(); + InnerType(char const*); +}; + +struct NonTrivialNoCtor { + InnerType it; +}; + +struct NonTrivialWithVector { + std::vector it; +}; + +struct NonTrivialWithCtor { + NonTrivialWithCtor(); + NonTrivialWithCtor(std::vector const&); +}; + +void testBracedInitTemporaries() { + std::vector v1; + + v1.push_back(NonTrivialNoCtor()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.push_back(NonTrivialNoCtor{}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.push_back({}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.push_back(NonTrivialNoCtor{InnerType{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.push_back({InnerType{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.push_back(NonTrivialNoCtor{InnerType()}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.push_back({InnerType()}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.push_back({{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v1.emplace_back(); + + v1.emplace_back(NonTrivialNoCtor()); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.emplace_back(NonTrivialNoCtor{}); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.emplace_back(NonTrivialNoCtor{InnerType{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v1.emplace_back(); + v1.emplace_back(NonTrivialNoCtor{{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v1.emplace_back(); + + // These should not be noticed or fixed; after the correction, the code won't + // compile. + v1.push_back(NonTrivialNoCtor{""}); + v1.push_back({""}); + v1.push_back(NonTrivialNoCtor{InnerType{""}}); + v1.push_back({InnerType{""}}); + v1.emplace_back(NonTrivialNoCtor{""}); + + std::vector v2; + + v2.push_back(NonTrivialWithVector()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.push_back(NonTrivialWithVector{}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.push_back({}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.push_back(NonTrivialWithVector{std::vector{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.push_back({std::vector{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.push_back(NonTrivialWithVector{std::vector()}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.push_back({std::vector()}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.push_back({{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v2.emplace_back(); + + v2.emplace_back(NonTrivialWithVector()); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.emplace_back(NonTrivialWithVector{}); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.emplace_back(NonTrivialWithVector{std::vector{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v2.emplace_back(); + v2.emplace_back(NonTrivialWithVector{{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v2.emplace_back(); + + + // These should not be noticed or fixed; after the correction, the code won't + // compile. + v2.push_back(NonTrivialWithVector{{0}}); + v2.push_back({{0}}); + v2.push_back(NonTrivialWithVector{std::vector{0}}); + v2.push_back({std::vector{0}}); + v2.emplace_back(NonTrivialWithVector{std::vector{0}}); + + std::vector v3; + + v3.push_back(NonTrivialWithCtor()); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v3.emplace_back(); + v3.push_back(NonTrivialWithCtor{}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v3.emplace_back(); + v3.push_back({}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v3.emplace_back(); + v3.push_back(NonTrivialWithCtor{std::vector()}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v3.emplace_back(std::vector()); + v3.push_back(NonTrivialWithCtor{std::vector{0}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v3.emplace_back(std::vector{0}); + v3.push_back(NonTrivialWithCtor{std::vector{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v3.emplace_back(std::vector{}); + v3.push_back({std::vector{0}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v3.emplace_back(std::vector{0}); + v3.push_back({std::vector{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back + // CHECK-FIXES: v3.emplace_back(std::vector{}); + + v3.emplace_back(NonTrivialWithCtor()); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v3.emplace_back(); + v3.emplace_back(NonTrivialWithCtor{}); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v3.emplace_back(); + v3.emplace_back(NonTrivialWithCtor{std::vector{}}); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v3.emplace_back(std::vector{}); + v3.emplace_back(NonTrivialWithCtor{std::vector{0}}); + // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back + // CHECK-FIXES: v3.emplace_back(std::vector{0}); + + // These should not be noticed or fixed; after the correction, the code won't + // compile. + v3.push_back(NonTrivialWithCtor{{0}}); + v3.push_back(NonTrivialWithCtor{{}}); + v3.push_back({{0}}); + v3.push_back({{}}); +}