From e919320eb0f39e1ee49b59fefdd321c04810b64a Mon Sep 17 00:00:00 2001 From: wangliushuai Date: Mon, 4 Aug 2025 17:45:38 +0800 Subject: [PATCH 1/7] [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 120f7fb749493..ab6a098569487 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(OldVarDeclId); const auto *ObjectArg = Result.Nodes.getNodeAs(ObjectArgId); + const auto *FD = Result.Nodes.getNodeAs("fieldDecl"); const auto *CtorCall = Result.Nodes.getNodeAs("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 = @@ -371,6 +389,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 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 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 30f9c79f74ac78e26b44740f3478e6f86b55014d Mon Sep 17 00:00:00 2001 From: wangliushuai Date: Mon, 4 Aug 2025 18:38:57 +0800 Subject: [PATCH 2/7] 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 ab6a098569487..7559629e5006a 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 { From 04f6d42006facb8030e347421a3681be66d375d4 Mon Sep 17 00:00:00 2001 From: wangliushuai Date: Tue, 5 Aug 2025 11:04:13 +0800 Subject: [PATCH 3/7] 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 2dc5c73073cf8..9e572412a7ed4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -190,6 +190,11 @@ Changes in existing checks ` check by adding the option `IgnoreAliasing`, that allows not looking at underlying types of type aliases. +- Improved :doc:`performance-unnecessary-copy-initialization + ` check by + adding detection for the local variables initialized with the member variable + of a const object. + Removed checks ^^^^^^^^^^^^^^ From e9a49a24d50f9182ee8bcccc4e093e0faf28b527 Mon Sep 17 00:00:00 2001 From: wangliushuai Date: Tue, 5 Aug 2025 11:35:22 +0800 Subject: [PATCH 4/7] 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 9e572412a7ed4..2e618abe59964 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -172,7 +172,8 @@ Changes in existing checks - Improved :doc:`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 ` by printing @@ -190,11 +191,6 @@ Changes in existing checks ` check by adding the option `IgnoreAliasing`, that allows not looking at underlying types of type aliases. -- Improved :doc:`performance-unnecessary-copy-initialization - ` check by - adding detection for the local variables initialized with the member variable - of a const object. - Removed checks ^^^^^^^^^^^^^^ From 32cb572002e78ff536fb539aa6d96c0fa6fc2694 Mon Sep 17 00:00:00 2001 From: wangliushuai Date: Thu, 7 Aug 2025 19:13:02 +0800 Subject: [PATCH 5/7] 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 7559629e5006a..bce84a778b175 100644 --- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -274,13 +274,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); } @@ -333,7 +335,7 @@ void UnnecessaryCopyInitialization::check( handleCopyFromLocalVar(Context, *OldVar); } else { // `auto NewVar = OldVar.FD;` - handleCopyFromConstLocalVarMember(Context, *OldVar); + handleCopyFromConstLocalVarMember(Context, *OldVar, *FD); } } @@ -359,8 +361,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( @@ -390,14 +392,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 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 5c02c19fcf25a3c6210ed36f6a04cad6d9e5c2c1 Mon Sep 17 00:00:00 2001 From: wangliushuai Date: Thu, 7 Aug 2025 19:58:31 +0800 Subject: [PATCH 6/7] 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 bce84a778b175..d2b4bfee823f1 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) { LocalVarCopiedFrom( memberExpr(hasObjectExpression(anyOf(hasDescendant(DeclRefToConstVar), DeclRefToConstVar)), - member(fieldDecl().bind("fieldDecl")))), + member(fieldDecl())) + .bind("memExpr")), this); } @@ -305,7 +306,7 @@ void UnnecessaryCopyInitialization::check( IssueFix, IsVarUnused, IsVarOnlyUsedAsConst}; const auto *OldVar = Result.Nodes.getNodeAs(OldVarDeclId); const auto *ObjectArg = Result.Nodes.getNodeAs(ObjectArgId); - const auto *FD = Result.Nodes.getNodeAs("fieldDecl"); + const auto *ME = Result.Nodes.getNodeAs("memExpr"); const auto *CtorCall = Result.Nodes.getNodeAs("ctorCall"); TraversalKindScope RAII(*Result.Context, TK_AsIs); @@ -330,12 +331,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); } } @@ -360,9 +361,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( @@ -391,15 +392,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 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 d7813b1c3f90c112672e37291b58d824ce6c2f44 Mon Sep 17 00:00:00 2001 From: wangliushuai Date: Thu, 7 Aug 2025 20:02:01 +0800 Subject: [PATCH 7/7] 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 +}