[clang-tidy] performance-unnecessary-copy-initialization: Enhance the check for the scenario with MemberExpr initialization. #151936

movie-travel-code · 2025-08-04T10:01:25Z

For now, performance-unnecessary-copy-initialization skip the check of the initialization with MemberExpr, see #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 var object is const-qualified is considered here.

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
}

Fixes #98005

llvmbot · 2025-08-04T10:01:56Z

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: henrywong (movie-travel-code)

Changes

For now, performance-unnecessary-copy-initialization skip the initialization with the MemberExpr, see #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.

bool CopiedFromConstRefParmVar(const Struct &amp;crs, const Struct cs, Struct &amp;rs, Struct s) {
  const auto m1 = crs.Member; // should warn
  const auto m2 = cs.Member; // should warn
}

Full diff: https://github.com/llvm/llvm-project/pull/151936.diff

3 Files Affected:

(modified) clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp (+31-1)
(modified) clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h (+5)
(modified) clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp (+25)

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;
+}

github-actions · 2025-08-04T10:03:45Z

✅ With the latest revision this PR passed the C/C++ code formatter.

EugeneZelenko · 2025-08-04T14:31:19Z

I think changes should be mentioned in Release Notes.

movie-travel-code · 2025-08-05T03:11:23Z

Hi EugeneZelenko, appreciate your reminding me of that. I have added it.

clang-tools-extra/docs/ReleaseNotes.rst

localspook · 2025-08-06T03:54:10Z

clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp

+    const CheckContext &Ctx, const VarDecl &OldVar) {
+  auto Diagnostic =
+      diag(Ctx.Var.getLocation(),
+           "local copy %1 of the field of the variable %0 is never "


I think the error message would be more understandable if it mentioned the name of the field. Something like:

local copy 'foo' of the field 'bar' of the variable 'baz' is never modified [...] ^^^^^

or:

local copy 'foo' of the subobject 'baz.bar' is never modified [...] ^^^^^^^^^^^^^^^^^^^

Also, with the latest trunk type information of copied variables are emitted, please adjust your message.

Thanks all for all your detailed comments, I have improved the diag message based on the trunk commit.

@localspook thank you for your professional advice and I took your second suggestion, which seems more concise, especially when expressing chain member expressions.

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp

… the scenario of MemberExpr initialization.

EugeneZelenko · 2025-08-07T14:54:02Z

clang-tools-extra/docs/ReleaseNotes.rst

@@ -172,7 +172,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.


Suggested change

initialized with a member variable of a const object.

initialized with a member variable of a ``const`` object.

localspook · 2025-08-07T15:08:06Z

All my comments above this message are resolved, but it seems I can't mark them resolved (because I don't have commit permissions, I think?). Could someone else resolve them for me?

localspook · 2025-08-07T15:10:45Z

clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp

+           "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;


Suggested change

"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;

"local copy %0 of the subobject '%1' of type %2 is never "

"modified%select{"

"| and never used}3; consider %select{avoiding the copy|removing "

"the statement}3")

<< &Ctx.Var << MEStr << Ctx.Var.getType() << Ctx.IsVarUnused;

Then, the OldVar parameter can be removed from this function and from handleCopyFromConstVarMember.

Thank you for your patient review. I will make the modification, and resolve the previous comments.

localspook · 2025-08-07T15:11:11Z

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-copy-initialization.cpp

+
+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


Suggested change

// 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

movie-travel-code requested a review from legrosbuffle August 4, 2025 10:01

llvmbot added clang-tools-extra clang-tidy labels Aug 4, 2025

EugeneZelenko requested review from PiotrZSL, carlosgalvezp, 5chmidti, vbvictor and HerrCai0907 August 4, 2025 14:30

EugeneZelenko reviewed Aug 5, 2025

View reviewed changes

clang-tools-extra/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved

Sockke self-requested a review August 5, 2025 03:38

localspook reviewed Aug 6, 2025

View reviewed changes

movie-travel-code added 5 commits August 6, 2025 19:59

[clang-tidy] Enhance the check for unnecessary copy initialization in…

e919320

… the scenario of MemberExpr initialization.

format the PR.

30f9c79

supplement the release notes about the change in this PR.

04f6d42

keep the release notes in alphabetical order.

e9a49a2

support nested member expr, improve the diag message and rebase main

32cb572

movie-travel-code force-pushed the enhance_unnecessary_copy_init_check branch from a201f8a to 32cb572 Compare August 7, 2025 11:13

put the source text of the 'MemberExpr' in the diag message.

5c02c19

movie-travel-code force-pushed the enhance_unnecessary_copy_init_check branch from 90a760a to 5c02c19 Compare August 7, 2025 11:58

add a new line at the end of the file.

d7813b1

EugeneZelenko reviewed Aug 7, 2025

View reviewed changes

localspook reviewed Aug 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang-tidy] performance-unnecessary-copy-initialization: Enhance the check for the scenario with MemberExpr initialization. #151936

[clang-tidy] performance-unnecessary-copy-initialization: Enhance the check for the scenario with MemberExpr initialization. #151936

movie-travel-code commented Aug 4, 2025 •

edited by vbvictor

Loading

Uh oh!

llvmbot commented Aug 4, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 4, 2025 •

edited

Loading

Uh oh!

EugeneZelenko commented Aug 4, 2025

Uh oh!

movie-travel-code commented Aug 5, 2025

Uh oh!

Uh oh!

Uh oh!

localspook Aug 6, 2025

Uh oh!

vbvictor Aug 6, 2025

Uh oh!

movie-travel-code Aug 7, 2025

Uh oh!

movie-travel-code Aug 7, 2025

Uh oh!

Uh oh!

EugeneZelenko Aug 7, 2025

Uh oh!

localspook commented Aug 7, 2025

Uh oh!

localspook Aug 7, 2025 •

edited

Loading

Uh oh!

movie-travel-code Aug 8, 2025

Uh oh!

localspook Aug 7, 2025

Uh oh!

Uh oh!

	initialized with a member variable of a const object.
	initialized with a member variable of a ``const`` object.

	// 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

[clang-tidy] performance-unnecessary-copy-initialization: Enhance the check for the scenario with MemberExpr initialization. #151936

Are you sure you want to change the base?

[clang-tidy] performance-unnecessary-copy-initialization: Enhance the check for the scenario with MemberExpr initialization. #151936

Conversation

movie-travel-code commented Aug 4, 2025 • edited by vbvictor Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EugeneZelenko commented Aug 4, 2025

Uh oh!

movie-travel-code commented Aug 5, 2025

Uh oh!

Uh oh!

Uh oh!

localspook Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

vbvictor Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

movie-travel-code Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

movie-travel-code Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

EugeneZelenko Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

localspook commented Aug 7, 2025

Uh oh!

localspook Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

movie-travel-code Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

localspook Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

movie-travel-code commented Aug 4, 2025 •

edited by vbvictor

Loading

llvmbot commented Aug 4, 2025 •

edited

Loading

github-actions bot commented Aug 4, 2025 •

edited

Loading

localspook Aug 7, 2025 •

edited

Loading