[clang-tools-extra] 03ef103 - [clang-tidy] Fix bug in modernize-use-emplace (#66169)

[via cfe-commits]

Author: Chris Cotter
Date: 2024-01-04T21:47:14+01:00
New Revision: 03ef103235752830da7b9ce5e825c0e3ddf7f45a

URL: https://github.com/llvm/llvm-project/commit/03ef103235752830da7b9ce5e825c0e3ddf7f45a
DIFF: https://github.com/llvm/llvm-project/commit/03ef103235752830da7b9ce5e825c0e3ddf7f45a.diff

LOG: [clang-tidy] Fix bug in modernize-use-emplace (#66169)

emplace_back cannot construct an aggregate with arguments used to
initialize the aggregate.
Closes #62387

Test plan: Added test test from #62387 which contains code that should
not be replaced by the check.

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    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 4438f0b22063f5..07e296cc26528d 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -13,6 +13,10 @@ using namespace clang::ast_matchers;
 namespace clang::tidy::modernize {
 
 namespace {
+AST_MATCHER_P(InitListExpr, initCountLeq, unsigned, N) {
+  return Node.getNumInits() <= N;
+}
+
 // Identical to hasAnyName, except it does not take template specifiers into
 // account. This is used to match the functions names as in
 // DefaultEmplacyFunctions below without caring about the template types of the
@@ -205,11 +209,13 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder) {
   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 HasConstructInitListExpr =
+      has(initListExpr(initCountLeq(1),
+                       anyOf(allOf(has(SoughtConstructExpr),
+                                   has(cxxConstructExpr(argumentCountIs(0)))),
+                             has(cxxBindTemporaryExpr(
+                                 has(SoughtConstructExpr),
+                                 has(cxxConstructExpr(argumentCountIs(0))))))));
   auto HasBracedInitListExpr =
       anyOf(has(cxxBindTemporaryExpr(HasConstructInitListExpr)),
             HasConstructInitListExpr);

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 571808a51596a8..4d25e2ebe85f5f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -394,6 +394,10 @@ Changes in existing checks
   false-positives when constructing the container with ``count`` copies of
   elements with value ``value``.
 
+- Improved :doc:`modernize-use-emplace
+  <clang-tidy/checks/modernize/use-emplace>` to not replace aggregates that
+  ``emplace`` cannot construct with aggregate initialization.
+
 - Improved :doc:`modernize-use-equals-delete
   <clang-tidy/checks/modernize/use-equals-delete>` check to ignore
   false-positives when special member function is actually used or implicit.

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 fead2b6151d021..f7b1ad55f5df51 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
@@ -1183,6 +1183,11 @@ struct NonTrivialWithVector {
   std::vector<int> it;
 };
 
+struct NonTrivialWithIntAndVector {
+  int x;
+  std::vector<int> it;
+};
+
 struct NonTrivialWithCtor {
   NonTrivialWithCtor();
   NonTrivialWithCtor(std::vector<int> const&);
@@ -1332,6 +1337,14 @@ void testBracedInitTemporaries() {
   v3.push_back(NonTrivialWithCtor{{}});
   v3.push_back({{0}});
   v3.push_back({{}});
+
+  std::vector<NonTrivialWithIntAndVector> v4;
+
+  // These should not be noticed or fixed; after the correction, the code won't
+  // compile.
+  v4.push_back(NonTrivialWithIntAndVector{1, {}});
+  v4.push_back(NonTrivialWithIntAndVector{});
+  v4.push_back({});
 }
 
 void testWithPointerTypes() {