[clang-tools-extra] [clang-tidy] performance-unnecessary-copy-initialization: Enhance the check for the scenario with MemberExpr initialization. (PR #151936)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 4 03:39:21 PDT 2025
https://github.com/movie-travel-code updated https://github.com/llvm/llvm-project/pull/151936
>From 02656bd505f23ad2e0b128879189f712b77e4f11 Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Mon, 4 Aug 2025 17:45:38 +0800
Subject: [PATCH 1/2] [clang-tidy] Enhance the check for unnecessary copy
initialization in the scenario of MemberExpr initialization.
---
.../UnnecessaryCopyInitialization.cpp | 32 ++++++++++++++++++-
.../UnnecessaryCopyInitialization.h | 5 +++
.../unnecessary-copy-initialization.cpp | 25 +++++++++++++++
3 files changed, 61 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index b6f19811f5e5c..c8a46f27000e7 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -273,6 +273,15 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(LocalVarCopiedFrom(declRefExpr(
to(varDecl(hasLocalStorage()).bind(OldVarDeclId)))),
this);
+
+ Finder->addMatcher(
+ LocalVarCopiedFrom(memberExpr(
+ hasObjectExpression(expr(ignoringParenImpCasts(declRefExpr(
+ to(varDecl(anyOf(hasType(isConstQualified()),
+ hasType(references(isConstQualified()))))
+ .bind(OldVarDeclId)))))),
+ member(fieldDecl().bind("fieldDecl")))),
+ this);
}
void UnnecessaryCopyInitialization::check(
@@ -294,6 +303,7 @@ void UnnecessaryCopyInitialization::check(
IssueFix, IsVarUnused, IsVarOnlyUsedAsConst};
const auto *OldVar = Result.Nodes.getNodeAs<VarDecl>(OldVarDeclId);
const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>(ObjectArgId);
+ const auto *FD = Result.Nodes.getNodeAs<FieldDecl>("fieldDecl");
const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
TraversalKindScope RAII(*Result.Context, TK_AsIs);
@@ -318,9 +328,12 @@ void UnnecessaryCopyInitialization::check(
if (OldVar == nullptr) {
// `auto NewVar = functionCall();`
handleCopyFromMethodReturn(Context, ObjectArg);
- } else {
+ } else if (FD == nullptr){
// `auto NewVar = OldVar;`
handleCopyFromLocalVar(Context, *OldVar);
+ } else {
+ // `auto NewVar = OldVar.FD;`
+ handleCopyFromConstLocalVarMember(Context, *OldVar);
}
}
@@ -345,6 +358,11 @@ void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
diagnoseCopyFromLocalVar(Ctx, OldVar);
}
+void UnnecessaryCopyInitialization::handleCopyFromConstLocalVarMember(
+ const CheckContext &Ctx, const VarDecl &OldVar) {
+ diagnoseCopyFromConstLocalVarMember(Ctx, OldVar);
+}
+
void UnnecessaryCopyInitialization::diagnoseCopyFromMethodReturn(
const CheckContext &Ctx) {
auto Diagnostic =
@@ -369,6 +387,18 @@ void UnnecessaryCopyInitialization::diagnoseCopyFromLocalVar(
maybeIssueFixes(Ctx, Diagnostic);
}
+void UnnecessaryCopyInitialization::diagnoseCopyFromConstLocalVarMember(
+ const CheckContext &Ctx, const VarDecl &OldVar) {
+ auto Diagnostic =
+ diag(Ctx.Var.getLocation(),
+ "local copy %1 of the field of the variable %0 is never "
+ "modified%select{"
+ "| and never used}2; consider %select{avoiding the copy|removing "
+ "the statement}2")
+ << &OldVar << &Ctx.Var << Ctx.IsVarUnused;
+ maybeIssueFixes(Ctx, Diagnostic);
+}
+
void UnnecessaryCopyInitialization::maybeIssueFixes(
const CheckContext &Ctx, DiagnosticBuilder &Diagnostic) {
if (Ctx.IssueFix) {
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
index 38f756f9b452f..1dade991cfd5f 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -50,12 +50,17 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck {
virtual void diagnoseCopyFromMethodReturn(const CheckContext &Ctx);
virtual void diagnoseCopyFromLocalVar(const CheckContext &Ctx,
const VarDecl &OldVar);
+ virtual void diagnoseCopyFromConstLocalVarMember(const CheckContext &Ctx,
+ const VarDecl &OldVar);
private:
void handleCopyFromMethodReturn(const CheckContext &Ctx,
const VarDecl *ObjectArg);
void handleCopyFromLocalVar(const CheckContext &Ctx, const VarDecl &OldVar);
+ void handleCopyFromConstLocalVarMember(const CheckContext &Ctx,
+ const VarDecl &OldVar);
+
void maybeIssueFixes(const CheckContext &Ctx, DiagnosticBuilder &Diagnostic);
const std::vector<StringRef> AllowedTypes;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
index c0f1fb9c0f6d2..8f263ddde57b1 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp
@@ -939,3 +939,28 @@ template<typename T> bool OperatorWithNoDirectCallee(T t) {
return a1 == t;
}
+bool CopiedFromConstRefParmVar(const Struct &crs, const Struct cs, Struct &rs, Struct s) {
+ const auto m1 = crs.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the field of the variable 'crs' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m1 = crs.Member;
+ const auto m2 = cs.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm2' of the field of the variable 'cs' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m2 = cs.Member;
+ const auto m3 = rs.Member;
+ const auto m4 = s.Member;
+ return m1 == m2 || m3 == m4;
+}
+
+const Struct GlobalS;
+bool CopiedFromConstLocalVar() {
+ const Struct crs;
+ Struct s;
+ const auto m1 = crs.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the field of the variable 'crs' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m1 = crs.Member;
+ const auto m2 = s.Member;
+ const auto m3 = GlobalS.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm3' of the field of the variable 'GlobalS' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m3 = GlobalS.Member;
+ return m1 == m2 || m2 == m3;
+}
>From 7eacdd2735e70d555979b0c2315336b52d62bdae Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Mon, 4 Aug 2025 18:38:57 +0800
Subject: [PATCH 2/2] format the PR.
---
.../clang-tidy/performance/UnnecessaryCopyInitialization.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index c8a46f27000e7..aae78be9ff7fe 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -328,7 +328,7 @@ void UnnecessaryCopyInitialization::check(
if (OldVar == nullptr) {
// `auto NewVar = functionCall();`
handleCopyFromMethodReturn(Context, ObjectArg);
- } else if (FD == nullptr){
+ } else if (FD == nullptr) {
// `auto NewVar = OldVar;`
handleCopyFromLocalVar(Context, *OldVar);
} else {
More information about the cfe-commits
mailing list