https://github.com/movie-travel-code created https://github.com/llvm/llvm-project/pull/151936
For now, performance-unnecessary-copy-initialization skip the initialization with the `MemberExpr`, see https://github.com/llvm/llvm-project/issues/98005, which lost a large number of optimization opportunities. This check adds two new checks, as shown in the following code. Given that the analysis of `MemberExpr` is relatively complex, only the case where the old object is const is considered here. ```cpp bool CopiedFromConstRefParmVar(const Struct &crs, const Struct cs, Struct &rs, Struct s) { const auto m1 = crs.Member; // should warn const auto m2 = cs.Member; // should warn } ``` >From 02656bd505f23ad2e0b128879189f712b77e4f11 Mon Sep 17 00:00:00 2001 From: wangliushuai <[email protected]> Date: Mon, 4 Aug 2025 17:45:38 +0800 Subject: [PATCH] [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; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
