https://github.com/RiverDave updated
https://github.com/llvm/llvm-project/pull/131969
>From fa0f1079587e6e26279c293fe4467ff0388f6a43 Mon Sep 17 00:00:00 2001
From: David Rivera <davidriv...@gmail.com>
Date: Sun, 16 Mar 2025 16:20:16 -0400
Subject: [PATCH] [clang-tidy] Add support for Initialization Forwarding in
Nested Objects within modernize-use-emplace
---
.../clang-tidy/modernize/UseEmplaceCheck.cpp | 120 ++++++++++++++----
clang-tools-extra/docs/ReleaseNotes.rst | 4 +
.../checkers/modernize/use-emplace.cpp | 26 +++-
3 files changed, 125 insertions(+), 25 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
index 430455a38f395..277994bbb6082 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -98,8 +98,8 @@ auto hasWantedType(llvm::ArrayRef<StringRef> TypeNames) {
// Matches member call expressions of the named method on the listed container
// types.
-auto cxxMemberCallExprOnContainer(
- StringRef MethodName, llvm::ArrayRef<StringRef> ContainerNames) {
+auto cxxMemberCallExprOnContainer(StringRef MethodName,
+ llvm::ArrayRef<StringRef> ContainerNames) {
return cxxMemberCallExpr(
hasDeclaration(functionDecl(hasName(MethodName))),
on(hasTypeOrPointeeType(hasWantedType(ContainerNames))));
@@ -174,19 +174,19 @@ void UseEmplaceCheck::registerMatchers(MatchFinder
*Finder) {
// passed pointer because smart pointer won't be constructed
// (and destructed) as in push_back case.
auto IsCtorOfSmartPtr =
- hasDeclaration(cxxConstructorDecl(ofClass(hasAnyName(SmartPointers))));
+ cxxConstructorDecl(ofClass(hasAnyName(SmartPointers)));
// Bitfields binds only to consts and emplace_back take it by universal ref.
- auto BitFieldAsArgument = hasAnyArgument(
- ignoringImplicit(memberExpr(hasDeclaration(fieldDecl(isBitField())))));
+ auto BitFieldAsArgument =
+ ignoringImplicit(memberExpr(hasDeclaration(fieldDecl(isBitField()))));
// Initializer list can't be passed to universal reference.
- auto InitializerListAsArgument = hasAnyArgument(
+ auto InitializerListAsArgument =
ignoringImplicit(allOf(cxxConstructExpr(isListInitialization()),
- unless(cxxTemporaryObjectExpr()))));
+ unless(cxxTemporaryObjectExpr())));
// We could have leak of resource.
- auto NewExprAsArgument = hasAnyArgument(ignoringImplicit(cxxNewExpr()));
+ auto NewExprAsArgument = ignoringImplicit(cxxNewExpr());
// We would call another constructor.
auto ConstructingDerived =
hasParent(implicitCastExpr(hasCastKind(CastKind::CK_DerivedToBase)));
@@ -202,19 +202,35 @@ void UseEmplaceCheck::registerMatchers(MatchFinder
*Finder) {
// overloaded functions and template names.
auto SoughtConstructExpr =
cxxConstructExpr(
- unless(anyOf(IsCtorOfSmartPtr, HasInitList, BitFieldAsArgument,
- InitializerListAsArgument, NewExprAsArgument,
- ConstructingDerived, IsPrivateOrProtectedCtor)))
+ unless(anyOf(hasDeclaration(IsCtorOfSmartPtr), HasInitList,
+ hasAnyArgument(BitFieldAsArgument),
+ hasAnyArgument(InitializerListAsArgument),
+ hasAnyArgument(NewExprAsArgument), ConstructingDerived,
+ IsPrivateOrProtectedCtor)))
.bind("ctor");
- auto HasConstructExpr = has(ignoringImplicit(SoughtConstructExpr));
+
+ auto IsPrimitiveType = hasType(builtinType());
+
+ auto AggregateInitExpr =
+ getLangOpts().CPlusPlus20
+ ? initListExpr(unless(anyOf(HasInitList, has(IsCtorOfSmartPtr),
+ has(BitFieldAsArgument),
+ has(InitializerListAsArgument),
+ has(NewExprAsArgument),
IsPrimitiveType)))
+ .bind("agg_init")
+ : unless(anything());
+
+ auto HasConstructExpr =
+ has(ignoringImplicit(anyOf(SoughtConstructExpr, AggregateInitExpr)));
// allow for T{} to be replaced, even if no CTOR is declared
auto HasConstructInitListExpr = has(initListExpr(
- initCountLeq(1), anyOf(allOf(has(SoughtConstructExpr),
- has(cxxConstructExpr(argumentCountIs(0)))),
- has(cxxBindTemporaryExpr(
- has(SoughtConstructExpr),
-
has(cxxConstructExpr(argumentCountIs(0))))))));
+ 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);
@@ -305,6 +321,38 @@ void UseEmplaceCheck::registerMatchers(MatchFinder
*Finder) {
this);
}
+const Expr *unwrapInnerExpression(const Expr *E) {
+
+ while (true) {
+ if (!E)
+ break;
+
+ if (llvm::isa<CXXConstructExpr>(E)) {
+ return E;
+ }
+
+ if (const auto *BindTemp = llvm::dyn_cast<CXXBindTemporaryExpr>(E)) {
+ E = BindTemp->getSubExpr();
+ continue;
+ }
+
+ if (const auto *MaterialTemp =
+ llvm::dyn_cast<MaterializeTemporaryExpr>(E)) {
+ E = MaterialTemp->getSubExpr();
+ continue;
+ }
+
+ if (const auto *Cast = llvm::dyn_cast<ImplicitCastExpr>(E)) {
+ E = Cast->getSubExpr();
+ continue;
+ }
+
+ break;
+ }
+
+ return nullptr; // No relevant sub-expression found
+}
+
void UseEmplaceCheck::check(const MatchFinder::MatchResult &Result) {
const auto *PushBackCall =
Result.Nodes.getNodeAs<CXXMemberCallExpr>("push_back_call");
@@ -313,7 +361,8 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult
&Result) {
Result.Nodes.getNodeAs<CXXMemberCallExpr>("push_front_call");
const auto *EmplacyCall =
Result.Nodes.getNodeAs<CXXMemberCallExpr>("emplacy_call");
- const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
+ auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctor");
+ auto *AggInitCall = Result.Nodes.getNodeAs<InitListExpr>("agg_init");
const auto *MakeCall = Result.Nodes.getNodeAs<CallExpr>("make");
const auto *TemporaryExpr =
Result.Nodes.getNodeAs<MaterializeTemporaryExpr>("temporary_expr");
@@ -332,12 +381,36 @@ void UseEmplaceCheck::check(const
MatchFinder::MatchResult &Result) {
}();
assert(Call && "No call matched");
- assert((CtorCall || MakeCall) && "No push_back parameter matched");
+ assert((CtorCall || MakeCall || AggInitCall) &&
+ "No push_back parameter matched");
if (IgnoreImplicitConstructors && CtorCall && CtorCall->getNumArgs() >= 1 &&
CtorCall->getArg(0)->getSourceRange() == CtorCall->getSourceRange())
return;
+ if (IgnoreImplicitConstructors && AggInitCall &&
+ AggInitCall->getNumInits() >= 1 &&
+ AggInitCall->getInit(0)->getSourceRange() ==
+ AggInitCall->getSourceRange())
+ return;
+
+ if (getLangOpts().LangStd >= LangStandard::lang_cxx20 && AggInitCall) {
+ for (const auto *Init : AggInitCall->inits()) {
+ if (const auto *InnermostInit = unwrapInnerExpression(Init)) {
+ // consume all args if it's an empty constructor call so that we can ->
+ // T{} -> emplace_back()
+ if (const auto *CtorExpr =
+ llvm::dyn_cast<CXXConstructExpr>(InnermostInit);
+ CtorExpr && CtorExpr->getNumArgs() == 0) {
+
+ CtorCall = CtorExpr;
+ AggInitCall = nullptr;
+ break;
+ }
+ }
+ }
+ }
+
const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
Call->getExprLoc(), Call->getArg(0)->getExprLoc());
@@ -345,6 +418,7 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult
&Result) {
EmplacyCall
? diag(TemporaryExpr ? TemporaryExpr->getBeginLoc()
: CtorCall ? CtorCall->getBeginLoc()
+ : AggInitCall ? AggInitCall->getBeginLoc()
: MakeCall->getBeginLoc(),
"unnecessary temporary object created while calling %0")
: diag(Call->getExprLoc(), "use emplace%select{|_back|_front}0 "
@@ -376,9 +450,10 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult
&Result) {
}
const SourceRange CallParensRange =
- MakeCall ? SourceRange(MakeCall->getCallee()->getEndLoc(),
- MakeCall->getRParenLoc())
- : CtorCall->getParenOrBraceRange();
+ MakeCall ? SourceRange(MakeCall->getCallee()->getEndLoc(),
+ MakeCall->getRParenLoc())
+ : CtorCall ? CtorCall->getParenOrBraceRange()
+ : AggInitCall->getSourceRange();
// Finish if there is no explicit constructor call.
if (CallParensRange.getBegin().isInvalid())
@@ -387,6 +462,7 @@ void UseEmplaceCheck::check(const MatchFinder::MatchResult
&Result) {
// FIXME: Will there ever be a CtorCall, if there is no TemporaryExpr?
const SourceLocation ExprBegin = TemporaryExpr ? TemporaryExpr->getExprLoc()
: CtorCall ? CtorCall->getExprLoc()
+ : AggInitCall ? AggInitCall->getExprLoc()
: MakeCall->getExprLoc();
// Range for constructor name and opening brace.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 72aa05eb4dcd1..8e0c71225ff58 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,10 @@ Changes in existing checks
``constexpr`` and ``static``` values on member initialization and by
detecting
explicit casting of built-in types within member list initialization.
+- Improved :doc:`modernize-use-emplace
+ <clang-tidy/checks/modernize/use-emplace>` check by adding support for C++20
+ Argument forwarding inside ``struct`` type objects.
+
- Improved :doc:`modernize-use-ranges
<clang-tidy/checks/modernize/use-ranges>` check by updating suppress
warnings logic for ``nullptr`` in ``std::find``.
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 e6562cd18dbab..4ad690cde2f94 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
@@ -1,4 +1,12 @@
-// RUN: %check_clang_tidy %s modernize-use-emplace %t -- \
+// RUN: %check_clang_tidy %s -std=c++11 modernize-use-emplace %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: {modernize-use-emplace.ContainersWithPushBack: \
+// RUN: '::std::vector; ::std::list; ::std::deque;
llvm::LikeASmallVector', \
+// RUN: modernize-use-emplace.TupleTypes: \
+// RUN: '::std::pair; std::tuple; ::test::Single', \
+// RUN: modernize-use-emplace.TupleMakeFunctions: \
+// RUN: '::std::make_pair; ::std::make_tuple;
::test::MakeSingle'}}"
+// RUN: %check_clang_tidy -std=c++20 %s modernize-use-emplace
-check-suffixes=,CPP20 %t -- \
// RUN: -config="{CheckOptions: \
// RUN: {modernize-use-emplace.ContainersWithPushBack: \
// RUN: '::std::vector; ::std::list; ::std::deque;
llvm::LikeASmallVector', \
@@ -1236,12 +1244,20 @@ void testBracedInitTemporaries() {
// CHECK-FIXES: v1.emplace_back();
// These should not be noticed or fixed; after the correction, the code won't
- // compile.
+ // compile in version previous to C++20.
v1.push_back(NonTrivialNoCtor{""});
+ // CHECK-MESSAGES-CPP20: :[[@LINE-1]]:6: warning: use emplace_back instead
of push_back
+ // CHECK-FIXES-CPP20: v1.emplace_back("");
v1.push_back({""});
+ // CHECK-MESSAGES-CPP20: :[[@LINE-1]]:6: warning: use emplace_back instead
of push_back
+ // CHECK-FIXES-CPP20: v1.emplace_back("");
v1.push_back(NonTrivialNoCtor{InnerType{""}});
+ // CHECK-MESSAGES-CPP20: :[[@LINE-1]]:6: warning: use emplace_back instead
of push_back
+ // CHECK-FIXES-CPP20: v1.emplace_back(InnerType{""});
v1.push_back({InnerType{""}});
- v1.emplace_back(NonTrivialNoCtor{""});
+ // CHECK-MESSAGES-CPP20: :[[@LINE-1]]:6: warning: use emplace_back instead
of push_back
+ // CHECK-FIXES-CPP20: v1.emplace_back(InnerType{""});
+ v1.emplace_back(NonTrivialNoCtor{""}); //TODO: Unsure if this should be
covered in cxx20
std::vector<NonTrivialWithVector> v2;
@@ -1289,7 +1305,11 @@ void testBracedInitTemporaries() {
v2.push_back(NonTrivialWithVector{{0}});
v2.push_back({{0}});
v2.push_back(NonTrivialWithVector{std::vector<int>{0}});
+ // CHECK-MESSAGES-CPP20: :[[@LINE-1]]:6: warning: use emplace_back instead
of push_back
+ // CHECK-FIXES-CPP20: v2.emplace_back(std::vector<int>{0});
v2.push_back({std::vector<int>{0}});
+ // CHECK-MESSAGES-CPP20: :[[@LINE-1]]:6: warning: use emplace_back instead
of push_back
+ // CHECK-FIXES-CPP20: v2.emplace_back(std::vector<int>{0});
v2.emplace_back(NonTrivialWithVector{std::vector<int>{0}});
std::vector<NonTrivialWithCtor> v3;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
