[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
Fri Aug 29 04:51:48 PDT 2025
https://github.com/movie-travel-code updated https://github.com/llvm/llvm-project/pull/151936
>From 52c52e599132d438d4e9d1585a7027a7c6a56abb Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Mon, 4 Aug 2025 17:45:38 +0800
Subject: [PATCH 01/13] [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 | 26 +++++++++++++++
3 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index c413090b3a0a4..07f04f05934f5 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -274,6 +274,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(
@@ -295,6 +304,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);
@@ -319,9 +329,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);
}
}
@@ -346,6 +359,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 =
@@ -372,6 +390,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 0168aeefc4583..606615bb1e75a 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
@@ -938,3 +938,29 @@ template<typename T> bool OperatorWithNoDirectCallee(T t) {
ExpensiveToCopyType a2 = a1;
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 53343f77bd435e628a82fd3bf38e0df803a031fd Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Mon, 4 Aug 2025 18:38:57 +0800
Subject: [PATCH 02/13] 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 07f04f05934f5..43e5295e65221 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -329,7 +329,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 {
>From a15fb0994e435628663bdc39ed4eec2116f86b67 Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Tue, 5 Aug 2025 11:04:13 +0800
Subject: [PATCH 03/13] supplement the release notes about the change in this
PR.
---
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 06641a602e28f..ead584062cb23 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -244,6 +244,11 @@ Changes in existing checks
<clang-tidy/checks/readability/qualified-auto>` check by adding the option
`IgnoreAliasing`, that allows not looking at underlying types of type aliases.
+- Improved :doc:`performance-unnecessary-copy-initialization
+ <clang-tidy/checks/performance/unnecessary-copy-initialization>` check by
+ adding detection for the local variables initialized with the member variable
+ of a const object.
+
Removed checks
^^^^^^^^^^^^^^
>From 60e4ca1c615329ff9715f7b8d8f8fc1ad2512a90 Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Tue, 5 Aug 2025 11:35:22 +0800
Subject: [PATCH 04/13] keep the release notes in alphabetical order.
---
clang-tools-extra/docs/ReleaseNotes.rst | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ead584062cb23..510fb468f7984 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -220,7 +220,8 @@ Changes in existing checks
- Improved :doc:`performance-unnecessary-copy-initialization
<clang-tidy/checks/performance/unnecessary-copy-initialization>` by printing
- the type of the diagnosed variable.
+ the type of the diagnosed variable and adding detection for local variables
+ initialized with a member variable of a const object.
- Improved :doc:`performance-unnecessary-value-param
<clang-tidy/checks/performance/unnecessary-value-param>` by printing
@@ -244,11 +245,6 @@ Changes in existing checks
<clang-tidy/checks/readability/qualified-auto>` check by adding the option
`IgnoreAliasing`, that allows not looking at underlying types of type aliases.
-- Improved :doc:`performance-unnecessary-copy-initialization
- <clang-tidy/checks/performance/unnecessary-copy-initialization>` check by
- adding detection for the local variables initialized with the member variable
- of a const object.
-
Removed checks
^^^^^^^^^^^^^^
>From 046f928147a0ad83077d09ec112ce38a0453791f Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Thu, 7 Aug 2025 19:13:02 +0800
Subject: [PATCH 05/13] support nested member expr, improve the diag message
and rebase main
---
.../UnnecessaryCopyInitialization.cpp | 30 ++++++-------
.../UnnecessaryCopyInitialization.h | 6 ++-
.../unnecessary-copy-initialization.cpp | 42 ++++++++++++++++---
3 files changed, 56 insertions(+), 22 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index 43e5295e65221..22949b6814ab5 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -275,13 +275,15 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
to(varDecl(hasLocalStorage()).bind(OldVarDeclId)))),
this);
+ auto DeclRefToConstVar =
+ declRefExpr(to(varDecl(anyOf(hasType(isConstQualified()),
+ hasType(references(isConstQualified()))))
+ .bind(OldVarDeclId)));
Finder->addMatcher(
- LocalVarCopiedFrom(memberExpr(
- hasObjectExpression(expr(ignoringParenImpCasts(declRefExpr(
- to(varDecl(anyOf(hasType(isConstQualified()),
- hasType(references(isConstQualified()))))
- .bind(OldVarDeclId)))))),
- member(fieldDecl().bind("fieldDecl")))),
+ LocalVarCopiedFrom(
+ memberExpr(hasObjectExpression(anyOf(hasDescendant(DeclRefToConstVar),
+ DeclRefToConstVar)),
+ member(fieldDecl().bind("fieldDecl")))),
this);
}
@@ -334,7 +336,7 @@ void UnnecessaryCopyInitialization::check(
handleCopyFromLocalVar(Context, *OldVar);
} else {
// `auto NewVar = OldVar.FD;`
- handleCopyFromConstLocalVarMember(Context, *OldVar);
+ handleCopyFromConstLocalVarMember(Context, *OldVar, *FD);
}
}
@@ -360,8 +362,8 @@ void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
}
void UnnecessaryCopyInitialization::handleCopyFromConstLocalVarMember(
- const CheckContext &Ctx, const VarDecl &OldVar) {
- diagnoseCopyFromConstLocalVarMember(Ctx, OldVar);
+ const CheckContext &Ctx, const VarDecl &OldVar, const FieldDecl &FD) {
+ diagnoseCopyFromConstLocalVarMember(Ctx, OldVar, FD);
}
void UnnecessaryCopyInitialization::diagnoseCopyFromMethodReturn(
@@ -391,14 +393,14 @@ void UnnecessaryCopyInitialization::diagnoseCopyFromLocalVar(
}
void UnnecessaryCopyInitialization::diagnoseCopyFromConstLocalVarMember(
- const CheckContext &Ctx, const VarDecl &OldVar) {
+ const CheckContext &Ctx, const VarDecl &OldVar, const FieldDecl &FD) {
auto Diagnostic =
diag(Ctx.Var.getLocation(),
- "local copy %1 of the field of the variable %0 is never "
+ "local copy %0 of the field %1 of type %2 in object %3 is never "
"modified%select{"
- "| and never used}2; consider %select{avoiding the copy|removing "
- "the statement}2")
- << &OldVar << &Ctx.Var << Ctx.IsVarUnused;
+ "| and never used}4; consider %select{avoiding the copy|removing "
+ "the statement}4")
+ << &Ctx.Var << &FD << Ctx.Var.getType() << &OldVar << Ctx.IsVarUnused;
maybeIssueFixes(Ctx, Diagnostic);
}
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
index 1dade991cfd5f..865c21366c9f7 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -51,7 +51,8 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck {
virtual void diagnoseCopyFromLocalVar(const CheckContext &Ctx,
const VarDecl &OldVar);
virtual void diagnoseCopyFromConstLocalVarMember(const CheckContext &Ctx,
- const VarDecl &OldVar);
+ const VarDecl &OldVar,
+ const FieldDecl &FD);
private:
void handleCopyFromMethodReturn(const CheckContext &Ctx,
@@ -59,7 +60,8 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck {
void handleCopyFromLocalVar(const CheckContext &Ctx, const VarDecl &OldVar);
void handleCopyFromConstLocalVarMember(const CheckContext &Ctx,
- const VarDecl &OldVar);
+ const VarDecl &OldVar,
+ const FieldDecl &FD);
void maybeIssueFixes(const CheckContext &Ctx, DiagnosticBuilder &Diagnostic);
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 606615bb1e75a..826c1175e9340 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,12 +939,12 @@ template<typename T> bool OperatorWithNoDirectCallee(T t) {
return a1 == t;
}
-bool CopiedFromConstRefParmVar(const Struct &crs, const Struct cs, Struct &rs, Struct s) {
+bool CopiedFromParmVarField(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-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the field 'Member' of type 'const ExpensiveToCopyType' in object '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-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm2' of the field 'Member' of type 'const ExpensiveToCopyType' in object '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;
@@ -952,15 +952,45 @@ bool CopiedFromConstRefParmVar(const Struct &crs, const Struct cs, Struct &rs, S
}
const Struct GlobalS;
-bool CopiedFromConstLocalVar() {
+bool CopiedFromVarField() {
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-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the field 'Member' of type 'const ExpensiveToCopyType' in object '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-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm3' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'GlobalS' is never modified; consider avoiding the copy
// CHECK-FIXES: const auto& m3 = GlobalS.Member;
return m1 == m2 || m2 == m3;
}
+
+struct NestedStruct {
+ Struct s;
+};
+
+bool CopiedFromParmVarNestedField(const NestedStruct &ncrs, const NestedStruct ncs, NestedStruct &nrs, NestedStruct ns) {
+ const auto m1 = ncrs.s.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'ncrs' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m1 = ncrs.s.Member;
+ const auto m2 = ncs.s.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm2' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'ncs' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m2 = ncs.s.Member;
+ const auto m3 = nrs.s.Member;
+ const auto m4 = ns.s.Member;
+ return m1 == m2 || m3 == m4;
+}
+
+const NestedStruct GlobalNS;
+bool CopiedFromVarNestedField() {
+ const NestedStruct ncrs;
+ NestedStruct ns;
+ const auto m1 = ncrs.s.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'ncrs' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m1 = ncrs.s.Member;
+ const auto m2 = ns.s.Member;
+ const auto m3 = GlobalNS.s.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm3' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'GlobalNS' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m3 = GlobalNS.s.Member;
+ return m1 == m2 || m2 == m3;
+}
\ No newline at end of file
>From 3a03bfd66cd88e791498b12d62579bb5d3c22055 Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Thu, 7 Aug 2025 19:58:31 +0800
Subject: [PATCH 06/13] put the source text of the 'MemberExpr' in the diag
message.
---
.../UnnecessaryCopyInitialization.cpp | 26 +++++++++++--------
.../UnnecessaryCopyInitialization.h | 12 ++++-----
.../unnecessary-copy-initialization.cpp | 16 ++++++------
3 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index 22949b6814ab5..ae9acce45b3f7 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -283,7 +283,8 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
LocalVarCopiedFrom(
memberExpr(hasObjectExpression(anyOf(hasDescendant(DeclRefToConstVar),
DeclRefToConstVar)),
- member(fieldDecl().bind("fieldDecl")))),
+ member(fieldDecl()))
+ .bind("memExpr")),
this);
}
@@ -306,7 +307,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 *ME = Result.Nodes.getNodeAs<MemberExpr>("memExpr");
const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
TraversalKindScope RAII(*Result.Context, TK_AsIs);
@@ -331,12 +332,12 @@ void UnnecessaryCopyInitialization::check(
if (OldVar == nullptr) {
// `auto NewVar = functionCall();`
handleCopyFromMethodReturn(Context, ObjectArg);
- } else if (FD == nullptr) {
+ } else if (ME == nullptr) {
// `auto NewVar = OldVar;`
handleCopyFromLocalVar(Context, *OldVar);
} else {
// `auto NewVar = OldVar.FD;`
- handleCopyFromConstLocalVarMember(Context, *OldVar, *FD);
+ handleCopyFromConstVarMember(Context, *OldVar, *ME);
}
}
@@ -361,9 +362,9 @@ void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
diagnoseCopyFromLocalVar(Ctx, OldVar);
}
-void UnnecessaryCopyInitialization::handleCopyFromConstLocalVarMember(
- const CheckContext &Ctx, const VarDecl &OldVar, const FieldDecl &FD) {
- diagnoseCopyFromConstLocalVarMember(Ctx, OldVar, FD);
+void UnnecessaryCopyInitialization::handleCopyFromConstVarMember(
+ const CheckContext &Ctx, const VarDecl &OldVar, const MemberExpr &ME) {
+ diagnoseCopyFromConstVarMember(Ctx, OldVar, ME);
}
void UnnecessaryCopyInitialization::diagnoseCopyFromMethodReturn(
@@ -392,15 +393,18 @@ void UnnecessaryCopyInitialization::diagnoseCopyFromLocalVar(
maybeIssueFixes(Ctx, Diagnostic);
}
-void UnnecessaryCopyInitialization::diagnoseCopyFromConstLocalVarMember(
- const CheckContext &Ctx, const VarDecl &OldVar, const FieldDecl &FD) {
+void UnnecessaryCopyInitialization::diagnoseCopyFromConstVarMember(
+ const CheckContext &Ctx, const VarDecl &OldVar, const MemberExpr &ME) {
+ std::string MEStr(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(ME.getSourceRange()),
+ Ctx.ASTCtx.getSourceManager(), Ctx.ASTCtx.getLangOpts()));
auto Diagnostic =
diag(Ctx.Var.getLocation(),
- "local copy %0 of the field %1 of type %2 in object %3 is never "
+ "local copy %0 of the subobject '%1' of type %2 is never "
"modified%select{"
"| and never used}4; consider %select{avoiding the copy|removing "
"the statement}4")
- << &Ctx.Var << &FD << Ctx.Var.getType() << &OldVar << Ctx.IsVarUnused;
+ << &Ctx.Var << MEStr << Ctx.Var.getType() << &OldVar << Ctx.IsVarUnused;
maybeIssueFixes(Ctx, Diagnostic);
}
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
index 865c21366c9f7..7154ad1d89856 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -50,18 +50,18 @@ 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,
- const FieldDecl &FD);
+ virtual void diagnoseCopyFromConstVarMember(const CheckContext &Ctx,
+ const VarDecl &OldVar,
+ const MemberExpr &ME);
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,
- const FieldDecl &FD);
+ void handleCopyFromConstVarMember(const CheckContext &Ctx,
+ const VarDecl &OldVar,
+ const MemberExpr &ME);
void maybeIssueFixes(const CheckContext &Ctx, DiagnosticBuilder &Diagnostic);
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 826c1175e9340..8dbdb6f554322 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
@@ -941,10 +941,10 @@ template<typename T> bool OperatorWithNoDirectCallee(T t) {
bool CopiedFromParmVarField(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 'Member' of type 'const ExpensiveToCopyType' in object 'crs' is never modified; consider avoiding the copy
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'crs.Member' of type 'const ExpensiveToCopyType' 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 'Member' of type 'const ExpensiveToCopyType' in object 'cs' is never modified; consider avoiding the copy
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm2' of the subobject 'cs.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
// CHECK-FIXES: const auto& m2 = cs.Member;
const auto m3 = rs.Member;
const auto m4 = s.Member;
@@ -956,11 +956,11 @@ bool CopiedFromVarField() {
const Struct crs;
Struct s;
const auto m1 = crs.Member;
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'crs' is never modified; consider avoiding the copy
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'crs.Member' of type 'const ExpensiveToCopyType' 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 'Member' of type 'const ExpensiveToCopyType' in object 'GlobalS' is never modified; consider avoiding the copy
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm3' of the subobject 'GlobalS.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
// CHECK-FIXES: const auto& m3 = GlobalS.Member;
return m1 == m2 || m2 == m3;
}
@@ -971,10 +971,10 @@ struct NestedStruct {
bool CopiedFromParmVarNestedField(const NestedStruct &ncrs, const NestedStruct ncs, NestedStruct &nrs, NestedStruct ns) {
const auto m1 = ncrs.s.Member;
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'ncrs' is never modified; consider avoiding the copy
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'ncrs.s.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
// CHECK-FIXES: const auto& m1 = ncrs.s.Member;
const auto m2 = ncs.s.Member;
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm2' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'ncs' is never modified; consider avoiding the copy
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm2' of the subobject 'ncs.s.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
// CHECK-FIXES: const auto& m2 = ncs.s.Member;
const auto m3 = nrs.s.Member;
const auto m4 = ns.s.Member;
@@ -986,11 +986,11 @@ bool CopiedFromVarNestedField() {
const NestedStruct ncrs;
NestedStruct ns;
const auto m1 = ncrs.s.Member;
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'ncrs' is never modified; consider avoiding the copy
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'ncrs.s.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
// CHECK-FIXES: const auto& m1 = ncrs.s.Member;
const auto m2 = ns.s.Member;
const auto m3 = GlobalNS.s.Member;
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm3' of the field 'Member' of type 'const ExpensiveToCopyType' in object 'GlobalNS' is never modified; consider avoiding the copy
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm3' of the subobject 'GlobalNS.s.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
// CHECK-FIXES: const auto& m3 = GlobalNS.s.Member;
return m1 == m2 || m2 == m3;
}
\ No newline at end of file
>From e7eec49847a1ad105c9353b8eb5204c4fc2472fc Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Thu, 7 Aug 2025 20:02:01 +0800
Subject: [PATCH 07/13] add a new line at the end of the file.
---
.../checkers/performance/unnecessary-copy-initialization.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 8dbdb6f554322..046fa406cc665 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
@@ -993,4 +993,4 @@ bool CopiedFromVarNestedField() {
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm3' of the subobject 'GlobalNS.s.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
// CHECK-FIXES: const auto& m3 = GlobalNS.s.Member;
return m1 == m2 || m2 == m3;
-}
\ No newline at end of file
+}
>From 9d904e249e93ab7a294185285c2f30ab86a3686c Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Fri, 8 Aug 2025 14:28:54 +0800
Subject: [PATCH 08/13] do some cleanups.
---
.../performance/UnnecessaryCopyInitialization.cpp | 14 +++++++-------
.../performance/UnnecessaryCopyInitialization.h | 2 --
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
.../unnecessary-copy-initialization.cpp | 2 +-
4 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index ae9acce45b3f7..f2c569f32f474 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -337,7 +337,7 @@ void UnnecessaryCopyInitialization::check(
handleCopyFromLocalVar(Context, *OldVar);
} else {
// `auto NewVar = OldVar.FD;`
- handleCopyFromConstVarMember(Context, *OldVar, *ME);
+ handleCopyFromConstVarMember(Context, *ME);
}
}
@@ -363,8 +363,8 @@ void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
}
void UnnecessaryCopyInitialization::handleCopyFromConstVarMember(
- const CheckContext &Ctx, const VarDecl &OldVar, const MemberExpr &ME) {
- diagnoseCopyFromConstVarMember(Ctx, OldVar, ME);
+ const CheckContext &Ctx, const MemberExpr &ME) {
+ diagnoseCopyFromConstVarMember(Ctx, ME);
}
void UnnecessaryCopyInitialization::diagnoseCopyFromMethodReturn(
@@ -394,7 +394,7 @@ void UnnecessaryCopyInitialization::diagnoseCopyFromLocalVar(
}
void UnnecessaryCopyInitialization::diagnoseCopyFromConstVarMember(
- const CheckContext &Ctx, const VarDecl &OldVar, const MemberExpr &ME) {
+ const CheckContext &Ctx, const MemberExpr &ME) {
std::string MEStr(Lexer::getSourceText(
CharSourceRange::getTokenRange(ME.getSourceRange()),
Ctx.ASTCtx.getSourceManager(), Ctx.ASTCtx.getLangOpts()));
@@ -402,9 +402,9 @@ void UnnecessaryCopyInitialization::diagnoseCopyFromConstVarMember(
diag(Ctx.Var.getLocation(),
"local copy %0 of the subobject '%1' of type %2 is never "
"modified%select{"
- "| and never used}4; consider %select{avoiding the copy|removing "
- "the statement}4")
- << &Ctx.Var << MEStr << Ctx.Var.getType() << &OldVar << Ctx.IsVarUnused;
+ "| and never used}3; consider %select{avoiding the copy|removing "
+ "the statement}3")
+ << &Ctx.Var << MEStr << Ctx.Var.getType() << Ctx.IsVarUnused;
maybeIssueFixes(Ctx, Diagnostic);
}
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
index 7154ad1d89856..ca569a8de492f 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -51,7 +51,6 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck {
virtual void diagnoseCopyFromLocalVar(const CheckContext &Ctx,
const VarDecl &OldVar);
virtual void diagnoseCopyFromConstVarMember(const CheckContext &Ctx,
- const VarDecl &OldVar,
const MemberExpr &ME);
private:
@@ -60,7 +59,6 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck {
void handleCopyFromLocalVar(const CheckContext &Ctx, const VarDecl &OldVar);
void handleCopyFromConstVarMember(const CheckContext &Ctx,
- const VarDecl &OldVar,
const MemberExpr &ME);
void maybeIssueFixes(const CheckContext &Ctx, DiagnosticBuilder &Diagnostic);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 510fb468f7984..dd724bef49f2b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -221,7 +221,7 @@ Changes in existing checks
- Improved :doc:`performance-unnecessary-copy-initialization
<clang-tidy/checks/performance/unnecessary-copy-initialization>` by printing
the type of the diagnosed variable and adding detection for local variables
- initialized with a member variable of a const object.
+ initialized with a member variable of a ``const`` object.
- Improved :doc:`performance-unnecessary-value-param
<clang-tidy/checks/performance/unnecessary-value-param>` by printing
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 046fa406cc665..762497043a3d7 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
@@ -971,7 +971,7 @@ struct NestedStruct {
bool CopiedFromParmVarNestedField(const NestedStruct &ncrs, const NestedStruct ncs, NestedStruct &nrs, NestedStruct ns) {
const auto m1 = ncrs.s.Member;
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'ncrs.s.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'ncrs.s.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
// CHECK-FIXES: const auto& m1 = ncrs.s.Member;
const auto m2 = ncs.s.Member;
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm2' of the subobject 'ncs.s.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
>From ed641ade7a91a833a867a5f70f9b15f60cd5d520 Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Mon, 11 Aug 2025 19:12:25 +0800
Subject: [PATCH 09/13] add the test about different template cases.
---
.../unnecessary-copy-initialization.cpp | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
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 762497043a3d7..6423c06c8f56a 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
@@ -994,3 +994,41 @@ bool CopiedFromVarNestedField() {
// CHECK-FIXES: const auto& m3 = GlobalNS.s.Member;
return m1 == m2 || m2 == m3;
}
+
+template <typename A, typename B>
+bool negativeTemplateTypesCopiedFromVarField(const A &neg_tcrs, A &neg_trs) {
+ const B m1 = neg_tcrs.Member; // should not warn
+ // CHECK-NOT-FIXES: const auto m1 = neg_tcrs.Member;
+ const B m2 = neg_trs.Member; // should not warn
+ // CHECK-NOT-FIXES: const auto m2 = neg_trs.Member;
+ return m1 == m2;
+}
+
+template <typename A>
+bool positiveTemplateTypesCopiedFromVarField(const A &tcrs, A &trs) {
+ const auto m1 = tcrs.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'tcrs.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
+ // CHECK-NOT-FIXES: const auto m1 = tcrs.Member;
+ const auto m2 = trs.Member;
+ // CHECK-NOT-FIXES: const auto m2 = trs.Member;
+ return m1 == m2;
+}
+
+template <typename A, typename B>
+bool negativeTemplateTypesCopiedFromVarNestedField(const A &neg_tncrs, A &neg_tnrs) {
+ const B m1 = neg_tncrs.s.Member; // should not warn
+ // CHECK-NOT-FIXES: const auto m1 = neg_tncrs.s.Member;
+ const B m2 = neg_tnrs.s.Member; // should not warn
+ // CHECK-NOT-FIXES: const auto m2 = neg_tnrs.s.Member;
+ return m1 == m2;
+}
+
+void callNegativeAllTemplateTypesCopiedFromVarField() {
+ Struct s;
+ negativeTemplateTypesCopiedFromVarField<Struct, ExpensiveToCopyType>(s, s);
+
+ positiveTemplateTypesCopiedFromVarField(s, s);
+
+ NestedStruct ns;
+ negativeTemplateTypesCopiedFromVarNestedField<NestedStruct, ExpensiveToCopyType>(ns, ns);
+}
>From 30ab6c64ba4332d3d8faee2623646445dfae2f10 Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Wed, 20 Aug 2025 10:36:06 +0800
Subject: [PATCH 10/13] support method call and add the tests about method
call.
---
.../performance/UnnecessaryCopyInitialization.cpp | 3 +--
.../performance/unnecessary-copy-initialization.cpp | 11 ++++++++++-
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index f2c569f32f474..4456f668b587d 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -282,8 +282,7 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
LocalVarCopiedFrom(
memberExpr(hasObjectExpression(anyOf(hasDescendant(DeclRefToConstVar),
- DeclRefToConstVar)),
- member(fieldDecl()))
+ DeclRefToConstVar)))
.bind("memExpr")),
this);
}
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 6423c06c8f56a..f2590cde67ff3 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
@@ -967,6 +967,7 @@ bool CopiedFromVarField() {
struct NestedStruct {
Struct s;
+ const Struct& getS() const { return s; }
};
bool CopiedFromParmVarNestedField(const NestedStruct &ncrs, const NestedStruct ncs, NestedStruct &nrs, NestedStruct ns) {
@@ -1026,9 +1027,17 @@ bool negativeTemplateTypesCopiedFromVarNestedField(const A &neg_tncrs, A &neg_tn
void callNegativeAllTemplateTypesCopiedFromVarField() {
Struct s;
negativeTemplateTypesCopiedFromVarField<Struct, ExpensiveToCopyType>(s, s);
-
positiveTemplateTypesCopiedFromVarField(s, s);
NestedStruct ns;
negativeTemplateTypesCopiedFromVarNestedField<NestedStruct, ExpensiveToCopyType>(ns, ns);
}
+
+ExpensiveToCopyType CopiedFromMethodCall(const NestedStruct &ns) {
+ const auto m = ns.getS().Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm' of the subobject 'ns.getS().Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m = ns.getS().Member;
+ const auto s = ns.getS();
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 's' of type 'const Struct' is copy-constructed from a const reference but is never used; consider removing the statement
+ return m;
+}
>From 41017a0c6bfa0e568c9d52fddff245514bce7583 Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Sat, 23 Aug 2025 10:16:57 +0800
Subject: [PATCH 11/13] add the missing constness check.
---
.../performance/UnnecessaryCopyInitialization.cpp | 3 +++
.../performance/unnecessary-copy-initialization.cpp | 11 ++++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index 4456f668b587d..da0a8096aec1c 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -363,6 +363,9 @@ void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
void UnnecessaryCopyInitialization::handleCopyFromConstVarMember(
const CheckContext &Ctx, const MemberExpr &ME) {
+ bool IsConstQualified = Ctx.Var.getType().isConstQualified();
+ if (!IsConstQualified && !Ctx.IsVarOnlyUsedAsConst)
+ return;
diagnoseCopyFromConstVarMember(Ctx, ME);
}
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 f2590cde67ff3..33128ec769392 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
@@ -948,7 +948,16 @@ bool CopiedFromParmVarField(const Struct &crs, const Struct cs, Struct &rs, Stru
// CHECK-FIXES: const auto& m2 = cs.Member;
const auto m3 = rs.Member;
const auto m4 = s.Member;
- return m1 == m2 || m3 == m4;
+
+ auto m5 = crs.Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: local copy 'm5' of the subobject 'crs.Member' of type 'ExpensiveToCopyType' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m5 = crs.Member;
+
+ auto m6 = cs.Member;
+ // CHECK-NOT-FIXES: const auto& m6 = cs.Member;
+
+ m6 = ExpensiveToCopyType();
+ return m1 == m2 || m3 == m4 || m5 == m6;
}
const Struct GlobalS;
>From 8b5e14f77b4d34a90a6f3a3dee7e4b6c365fa566 Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Sat, 23 Aug 2025 10:48:21 +0800
Subject: [PATCH 12/13] add more method call checks.
---
.../unnecessary-copy-initialization.cpp | 28 +++++++++++++------
1 file changed, 19 insertions(+), 9 deletions(-)
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 33128ec769392..a4302851ea691 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
@@ -954,7 +954,7 @@ bool CopiedFromParmVarField(const Struct &crs, const Struct cs, Struct &rs, Stru
// CHECK-FIXES: const auto& m5 = crs.Member;
auto m6 = cs.Member;
- // CHECK-NOT-FIXES: const auto& m6 = cs.Member;
+ // CHECK-NOT-FIXES: const auto m6 = cs.Member;
m6 = ExpensiveToCopyType();
return m1 == m2 || m3 == m4 || m5 == m6;
@@ -976,7 +976,9 @@ bool CopiedFromVarField() {
struct NestedStruct {
Struct s;
- const Struct& getS() const { return s; }
+ const Struct& getConstRefS() const { return s; }
+ const Struct getConstS() const { return s; }
+ Struct getS() const { return s; }
};
bool CopiedFromParmVarNestedField(const NestedStruct &ncrs, const NestedStruct ncs, NestedStruct &nrs, NestedStruct ns) {
@@ -1042,11 +1044,19 @@ void callNegativeAllTemplateTypesCopiedFromVarField() {
negativeTemplateTypesCopiedFromVarNestedField<NestedStruct, ExpensiveToCopyType>(ns, ns);
}
-ExpensiveToCopyType CopiedFromMethodCall(const NestedStruct &ns) {
- const auto m = ns.getS().Member;
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm' of the subobject 'ns.getS().Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
- // CHECK-FIXES: const auto& m = ns.getS().Member;
- const auto s = ns.getS();
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 's' of type 'const Struct' is copy-constructed from a const reference but is never used; consider removing the statement
- return m;
+bool CopiedFromMethodCall(const NestedStruct &ns) {
+ const auto m1 = ns.getConstRefS().Member;
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'ns.getConstRefS().Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
+ // CHECK-FIXES: const auto& m1 = ns.getConstRefS().Member;
+
+ const auto m2 = ns.getConstS().Member;
+ // CHECK-NOT-FIXES: const auto m2 = ns.getConstS().Member;
+
+ const auto m3 = ns.getS().Member;
+ // CHECK-NOT-FIXES: const auto m3 = ns.getConstS().Member;
+
+ const auto s1 = ns.getConstRefS();
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 's1' of type 'const Struct' is copy-constructed from a const reference but is never used; consider removing the statement
+
+ return m1 == m2 || m2 == m3;
}
>From 9a617420a103a843ddbbc5d28da735c7cfba38cb Mon Sep 17 00:00:00 2001
From: wangliushuai <wangliushuai at bytedance.com>
Date: Fri, 29 Aug 2025 19:35:10 +0800
Subject: [PATCH 13/13] filter the MemberExprs which contain cxxMemberCallExpr.
---
.../UnnecessaryCopyInitialization.cpp | 3 ++-
.../unnecessary-copy-initialization.cpp | 24 ++++++++-----------
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index da0a8096aec1c..3003910a21023 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -282,7 +282,8 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
LocalVarCopiedFrom(
memberExpr(hasObjectExpression(anyOf(hasDescendant(DeclRefToConstVar),
- DeclRefToConstVar)))
+ DeclRefToConstVar)),
+ unless(hasDescendant(cxxMemberCallExpr())))
.bind("memExpr")),
this);
}
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 a4302851ea691..c1b8bd7f7120d 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
@@ -954,7 +954,7 @@ bool CopiedFromParmVarField(const Struct &crs, const Struct cs, Struct &rs, Stru
// CHECK-FIXES: const auto& m5 = crs.Member;
auto m6 = cs.Member;
- // CHECK-NOT-FIXES: const auto m6 = cs.Member;
+ // CHECK-NOT-FIXES: const auto& m6 = cs.Member;
m6 = ExpensiveToCopyType();
return m1 == m2 || m3 == m4 || m5 == m6;
@@ -1010,9 +1010,9 @@ bool CopiedFromVarNestedField() {
template <typename A, typename B>
bool negativeTemplateTypesCopiedFromVarField(const A &neg_tcrs, A &neg_trs) {
const B m1 = neg_tcrs.Member; // should not warn
- // CHECK-NOT-FIXES: const auto m1 = neg_tcrs.Member;
+ // CHECK-NOT-FIXES: const auto& m1 = neg_tcrs.Member;
const B m2 = neg_trs.Member; // should not warn
- // CHECK-NOT-FIXES: const auto m2 = neg_trs.Member;
+ // CHECK-NOT-FIXES: const auto& m2 = neg_trs.Member;
return m1 == m2;
}
@@ -1020,18 +1020,18 @@ template <typename A>
bool positiveTemplateTypesCopiedFromVarField(const A &tcrs, A &trs) {
const auto m1 = tcrs.Member;
// CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'tcrs.Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
- // CHECK-NOT-FIXES: const auto m1 = tcrs.Member;
+ // CHECK-NOT-FIXES: const auto& m1 = tcrs.Member;
const auto m2 = trs.Member;
- // CHECK-NOT-FIXES: const auto m2 = trs.Member;
+ // CHECK-NOT-FIXES: const auto& m2 = trs.Member;
return m1 == m2;
}
template <typename A, typename B>
bool negativeTemplateTypesCopiedFromVarNestedField(const A &neg_tncrs, A &neg_tnrs) {
const B m1 = neg_tncrs.s.Member; // should not warn
- // CHECK-NOT-FIXES: const auto m1 = neg_tncrs.s.Member;
+ // CHECK-NOT-FIXES: const auto& m1 = neg_tncrs.s.Member;
const B m2 = neg_tnrs.s.Member; // should not warn
- // CHECK-NOT-FIXES: const auto m2 = neg_tnrs.s.Member;
+ // CHECK-NOT-FIXES: const auto& m2 = neg_tnrs.s.Member;
return m1 == m2;
}
@@ -1046,17 +1046,13 @@ void callNegativeAllTemplateTypesCopiedFromVarField() {
bool CopiedFromMethodCall(const NestedStruct &ns) {
const auto m1 = ns.getConstRefS().Member;
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: local copy 'm1' of the subobject 'ns.getConstRefS().Member' of type 'const ExpensiveToCopyType' is never modified; consider avoiding the copy
- // CHECK-FIXES: const auto& m1 = ns.getConstRefS().Member;
+ // CHECK-NOT-FIXES: const auto& m1 = ns.getConstRefS().Member;
const auto m2 = ns.getConstS().Member;
- // CHECK-NOT-FIXES: const auto m2 = ns.getConstS().Member;
+ // CHECK-NOT-FIXES: const auto& m2 = ns.getConstS().Member;
const auto m3 = ns.getS().Member;
- // CHECK-NOT-FIXES: const auto m3 = ns.getConstS().Member;
-
- const auto s1 = ns.getConstRefS();
- // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 's1' of type 'const Struct' is copy-constructed from a const reference but is never used; consider removing the statement
+ // CHECK-NOT-FIXES: const auto& m3 = ns.getConstS().Member;
return m1 == m2 || m2 == m3;
}
More information about the cfe-commits
mailing list