Fix handling of braced-init temporaries for modernize-use-emplace

Fixes #55870

Differential Revision: https://reviews.llvm.org/D135405

diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 045d7fb852230d0b7478f5ea5e8941936bc31a1c..ccd07065d15737204761ca327506aad405d05198 100644 (file)
--- 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 04ff0775d285c61119ad201fcbb3cea755c7ff55..537263d988be3bf7e5b50b56b4a69aa6f9042c3f 100644 (file)
--- 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({{}});
+}