[clang-tidy] Enhance the check for unnecessary copy initialization in the scenario of MemberExpr initialization.

Author: movie-travel-code

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) {

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;

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