[clang-tools-extra] 7db780d - Fix handling of braced-init temporaries for modernize-use-emplace
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 30 11:52:57 PST 2023
Author: BigPeet
Date: 2023-01-30T14:52:20-05:00
New Revision: 7db780d51212a7ee68a9e196cfcf0737807273d6
URL: https://github.com/llvm/llvm-project/commit/7db780d51212a7ee68a9e196cfcf0737807273d6
DIFF: https://github.com/llvm/llvm-project/commit/7db780d51212a7ee68a9e196cfcf0737807273d6.diff
LOG: Fix handling of braced-init temporaries for modernize-use-emplace
Fixes #55870
Differential Revision: https://reviews.llvm.org/D135405
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 045d7fb852230..ccd07065d1573 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -165,7 +165,8 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
// 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()));
@@ -190,6 +191,16 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
.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()),
@@ -202,19 +213,35 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
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),
@@ -270,6 +297,8 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
Result.Nodes.getNodeAs<CXXMemberCallExpr>("emplacy_call");
const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
const auto *MakeCall = Result.Nodes.getNodeAs<CallExpr>("make");
+ const auto *TemporaryExpr =
+ Result.Nodes.getNodeAs<MaterializeTemporaryExpr>("temporary_expr");
const CXXMemberCallExpr *Call = [&]() {
if (PushBackCall) {
@@ -296,7 +325,9 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
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");
@@ -335,16 +366,22 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
if (CallParensRange.getBegin().isInvalid())
return;
- const SourceLocation ExprBegin =
- CtorCall ? CtorCall->getExprLoc() : MakeCall->getExprLoc();
+ // FIXME: Will there ever be a CtorCall, if there is no TemporaryExpr?
+ const SourceLocation ExprBegin = 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
index 04ff0775d285c..537263d988be3 100644
--- 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 @@ void testPossibleFalsePositives() {
};
v.emplace_back(std::make_pair<D, int>(Something(), 2));
}
+
+struct InnerType {
+ InnerType();
+ InnerType(char const*);
+};
+
+struct NonTrivialNoCtor {
+ InnerType it;
+};
+
+struct NonTrivialWithVector {
+ std::vector<int> it;
+};
+
+struct NonTrivialWithCtor {
+ NonTrivialWithCtor();
+ NonTrivialWithCtor(std::vector<int> const&);
+};
+
+void testBracedInitTemporaries() {
+ std::vector<NonTrivialNoCtor> 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<NonTrivialWithVector> 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<int>{}});
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+ // CHECK-FIXES: v2.emplace_back();
+ v2.push_back({std::vector<int>{}});
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+ // CHECK-FIXES: v2.emplace_back();
+ v2.push_back(NonTrivialWithVector{std::vector<int>()});
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+ // CHECK-FIXES: v2.emplace_back();
+ v2.push_back({std::vector<int>()});
+ // 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<int>{}});
+ // 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<int>{0}});
+ v2.push_back({std::vector<int>{0}});
+ v2.emplace_back(NonTrivialWithVector{std::vector<int>{0}});
+
+ std::vector<NonTrivialWithCtor> 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<int>()});
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+ // CHECK-FIXES: v3.emplace_back(std::vector<int>());
+ v3.push_back(NonTrivialWithCtor{std::vector<int>{0}});
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+ // CHECK-FIXES: v3.emplace_back(std::vector<int>{0});
+ v3.push_back(NonTrivialWithCtor{std::vector<int>{}});
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+ // CHECK-FIXES: v3.emplace_back(std::vector<int>{});
+ v3.push_back({std::vector<int>{0}});
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+ // CHECK-FIXES: v3.emplace_back(std::vector<int>{0});
+ v3.push_back({std::vector<int>{}});
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
+ // CHECK-FIXES: v3.emplace_back(std::vector<int>{});
+
+ 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<int>{}});
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+ // CHECK-FIXES: v3.emplace_back(std::vector<int>{});
+ v3.emplace_back(NonTrivialWithCtor{std::vector<int>{0}});
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: unnecessary temporary object created while calling emplace_back
+ // CHECK-FIXES: v3.emplace_back(std::vector<int>{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({{}});
+}
More information about the cfe-commits
mailing list