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

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

@@ -273,6 +273,18 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(LocalVarCopiedFrom(declRefExpr(
                          to(varDecl(hasLocalStorage()).bind(OldVarDeclId)))),
                      this);
+
+  auto DeclRefToConstVar =
+      declRefExpr(to(varDecl(anyOf(hasType(isConstQualified()),
+                                   hasType(references(isConstQualified()))))
+                         .bind(OldVarDeclId)));
+  Finder->addMatcher(
+      LocalVarCopiedFrom(
+          memberExpr(hasObjectExpression(anyOf(hasDescendant(DeclRefToConstVar),
+                                               DeclRefToConstVar)),
+                     member(fieldDecl()))
+              .bind("memExpr")),
+      this);
 }
 
 void UnnecessaryCopyInitialization::check(
@@ -294,6 +306,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 *ME = Result.Nodes.getNodeAs<MemberExpr>("memExpr");
   const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
 
   TraversalKindScope RAII(*Result.Context, TK_AsIs);
@@ -318,9 +331,12 @@ void UnnecessaryCopyInitialization::check(
   if (OldVar == nullptr) {
     // `auto NewVar = functionCall();`
     handleCopyFromMethodReturn(Context, ObjectArg);
-  } else {
+  } else if (ME == nullptr) {
     // `auto NewVar = OldVar;`
     handleCopyFromLocalVar(Context, *OldVar);
+  } else {
+    // `auto NewVar = OldVar.FD;`
+    handleCopyFromConstVarMember(Context, *OldVar, *ME);
   }
 }
 
@@ -345,6 +361,11 @@ void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
   diagnoseCopyFromLocalVar(Ctx, OldVar);
 }
 
+void UnnecessaryCopyInitialization::handleCopyFromConstVarMember(
+    const CheckContext &Ctx, const VarDecl &OldVar, const MemberExpr &ME) {
+  diagnoseCopyFromConstVarMember(Ctx, OldVar, ME);
+}
+
 void UnnecessaryCopyInitialization::diagnoseCopyFromMethodReturn(
     const CheckContext &Ctx) {
   auto Diagnostic =
@@ -371,6 +392,21 @@ void UnnecessaryCopyInitialization::diagnoseCopyFromLocalVar(
   maybeIssueFixes(Ctx, Diagnostic);
 }
 
+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 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;
+  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,19 @@ class UnnecessaryCopyInitialization : public ClangTidyCheck {
   virtual void diagnoseCopyFromMethodReturn(const CheckContext &Ctx);
   virtual void diagnoseCopyFromLocalVar(const CheckContext &Ctx,
                                         const VarDecl &OldVar);
+  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 handleCopyFromConstVarMember(const CheckContext &Ctx,
+                                    const VarDecl &OldVar,
+                                    const MemberExpr &ME);
+
   void maybeIssueFixes(const CheckContext &Ctx, DiagnosticBuilder &Diagnostic);
 
   const std::vector<StringRef> AllowedTypes;

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.
 
 - Improved :doc:`performance-unnecessary-value-param
   <clang-tidy/checks/performance/unnecessary-value-param>` by printing

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

@@ -938,3 +938,59 @@ template<typename T> bool OperatorWithNoDirectCallee(T t) {
   ExpensiveToCopyType a2 = a1;
   return a1 == 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 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 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;
+  return m1 == m2 || m3 == m4;
+}
+
+const Struct GlobalS;
+bool CopiedFromVarField() {
+  const Struct crs;
+  Struct s;
+  const auto m1 = crs.Member;
+  // 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 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;
+}
+
+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 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
+  // 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 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 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;
+}