https://github.com/RiverDave updated
https://github.com/llvm/llvm-project/pull/131969
>From 9972aa8e1720f7e6378c63551c853ee504193000 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 | 105 +++++++++++++++---
clang-tools-extra/docs/ReleaseNotes.rst | 4 +
.../checkers/modernize/use-emplace.cpp | 48 ++++++--
3 files changed, 132 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..56c5a73a0c8b1 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,11 +202,26 @@ 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(
@@ -305,6 +320,36 @@ void UseEmplaceCheck::registerMatchers(MatchFinder
*Finder) {
this);
}
+static const CXXConstructExpr *unwrapConstructorExpr(const Expr *E) {
+
+ while (E) {
+
+ if (const auto *ConstructorExpr = llvm::dyn_cast<CXXConstructExpr>(E)) {
+ return ConstructorExpr;
+ }
+
+ 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 +358,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 +378,34 @@ 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 *InnerConstructorExpr = unwrapConstructorExpr(Init)) {
+ // consume all args if it's an empty constructor call so that we can ->
+ // T{} -> emplace_back()
+ if (InnerConstructorExpr && InnerConstructorExpr->getNumArgs() == 0) {
+
+ CtorCall = InnerConstructorExpr;
+ AggInitCall = nullptr;
+ break;
+ }
+ }
+ }
+ }
+
const auto FunctionNameSourceRange = CharSourceRange::getCharRange(
Call->getExprLoc(), Call->getArg(0)->getExprLoc());
@@ -345,6 +413,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 +445,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 +457,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..fdb7b54f63acd 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', \
@@ -1237,10 +1245,6 @@ void testBracedInitTemporaries() {
// 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;
@@ -1285,11 +1289,9 @@ void testBracedInitTemporaries() {
// These should not be noticed or fixed; after the correction, the code won't
- // compile.
+ // compile in version previous to C++20.
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;
@@ -1434,3 +1436,33 @@ void testWithPointerTypes() {
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: unnecessary temporary object
created while calling emplace
// CHECK-FIXES: sp->emplace();
}
+
+namespace GH1225740 {
+
+void CXX20testBracedInitTemporaries(){
+
+ std::vector<NonTrivialNoCtor> v1;
+
+ 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{""}});
+ // CHECK-MESSAGES-CPP20: :[[@LINE-1]]:6: warning: use emplace_back instead
of push_back
+ // CHECK-FIXES-CPP20: v1.emplace_back(InnerType{""});
+
+ std::vector<NonTrivialWithVector> v2;
+
+ 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});
+ }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
