https://github.com/ccotter updated
https://github.com/llvm/llvm-project/pull/66169
>From d8c40ccc6744d350b8bc530917accd1d8d87118e Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotte...@bloomberg.net>
Date: Mon, 11 Sep 2023 00:05:54 -0400
Subject: [PATCH 1/3] [clang-tidy] Fix bug in modernize-use-emplace
emplace_back cannot construct an aggregate with arguments used to
initialize the aggregate.
Closes #62387
Test plan: Added test case from #62387 which contains code that should
not be replaced by the check.
---
.../clang-tidy/modernize/UseEmplaceCheck.cpp | 16 +++++++++++-----
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++++
.../checkers/modernize/use-emplace.cpp | 16 ++++++++++++++++
3 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 554abcd900e329c..e4455d6f9c1feec 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, initCountIs, 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
@@ -207,11 +211,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(anyOf(initCountIs(0), initCountIs(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 19c977977f9044c..694beeb98a54e36 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -247,6 +247,10 @@ Changes in existing checks
<clang-tidy/checks/modernize/loop-convert>` to support for-loops with
iterators initialized by free functions like ``begin``, ``end``, or ``size``.
+- Improved :doc:`modernize-use-emplace
+ <clang-tidy/checks/modernize/use-emplace>` to not replace aggregates that
+ ``emplace_back`` 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 fead2b6151d0218..74edf0760bb324d 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,17 @@ 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, {}});
+ // CHECK-FIXES: v4.push_back(NonTrivialWithIntAndVector{1, {}});
+ v4.push_back(NonTrivialWithIntAndVector{});
+ // CHECK-FIXES: v4.push_back(NonTrivialWithIntAndVector{});
+ v4.push_back({});
+ // CHECK-FIXES: v4.push_back({});
}
void testWithPointerTypes() {
>From 9e96a3b60c132ac94e58f1e76e1ae7fe4d2238fb Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotte...@bloomberg.net>
Date: Wed, 13 Sep 2023 09:29:27 -0400
Subject: [PATCH 2/3] Use emplace in ReleaseNotes
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 694beeb98a54e36..fb6034caca06e30 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -249,7 +249,7 @@ Changes in existing checks
- Improved :doc:`modernize-use-emplace
<clang-tidy/checks/modernize/use-emplace>` to not replace aggregates that
- ``emplace_back`` cannot construct with aggregate initialization.
+ ``emplace`` cannot construct with aggregate initialization.
- Improved :doc:`modernize-use-equals-delete
<clang-tidy/checks/modernize/use-equals-delete>` check to ignore
>From 13928c216d7cbf96700d0658d513f8197c26ca9a Mon Sep 17 00:00:00 2001
From: Chris Cotter <ccotte...@bloomberg.net>
Date: Sun, 1 Oct 2023 17:23:14 -0400
Subject: [PATCH 3/3] Simplify matcher, remove CHECK-FIXES
---
clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp | 6 +++---
.../test/clang-tidy/checkers/modernize/use-emplace.cpp | 3 ---
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index e4455d6f9c1feec..d203005fb55e39e 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -13,8 +13,8 @@ using namespace clang::ast_matchers;
namespace clang::tidy::modernize {
namespace {
-AST_MATCHER_P(InitListExpr, initCountIs, unsigned, N) {
- return Node.getNumInits() == N;
+AST_MATCHER_P(InitListExpr, initCountLeq, unsigned, N) {
+ return Node.getNumInits() <= N;
}
// Identical to hasAnyName, except it does not take template specifiers into
@@ -212,7 +212,7 @@ void UseEmplaceCheck::registerMatchers(MatchFinder *Finder)
{
// allow for T{} to be replaced, even if no CTOR is declared
auto HasConstructInitListExpr =
- has(initListExpr(anyOf(initCountIs(0), initCountIs(1)),
+ has(initListExpr(initCountLeq(1),
anyOf(allOf(has(SoughtConstructExpr),
has(cxxConstructExpr(argumentCountIs(0)))),
has(cxxBindTemporaryExpr(
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 74edf0760bb324d..f7b1ad55f5df51e 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
@@ -1343,11 +1343,8 @@ void testBracedInitTemporaries() {
// These should not be noticed or fixed; after the correction, the code won't
// compile.
v4.push_back(NonTrivialWithIntAndVector{1, {}});
- // CHECK-FIXES: v4.push_back(NonTrivialWithIntAndVector{1, {}});
v4.push_back(NonTrivialWithIntAndVector{});
- // CHECK-FIXES: v4.push_back(NonTrivialWithIntAndVector{});
v4.push_back({});
- // CHECK-FIXES: v4.push_back({});
}
void testWithPointerTypes() {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
