Index: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -33,9 +33,10 @@ private: void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt, - ASTContext &Context); + bool IssueFix, ASTContext &Context); void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar, - const Stmt &BlockStmt, ASTContext &Context); + const Stmt &BlockStmt, bool IssueFix, + ASTContext &Context); }; } // namespace performance Index: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -20,10 +20,6 @@ void recordFixes(const VarDecl &Var, ASTContext &Context, DiagnosticBuilder &Diagnostic) { - // Do not propose fixes in macros since we cannot place them correctly. - if (Var.getLocation().isMacroID()) - return; - Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context); if (!Var.getType().isLocalConstQualified()) Diagnostic << utils::fixit::changeVarDeclToConst(Var); @@ -57,14 +53,16 @@ auto localVarCopiedFrom = [](const internal::Matcher &CopyCtorArg) { return compoundStmt( forEachDescendant( - varDecl(hasLocalStorage(), - hasType(matchers::isExpensiveToCopy()), - hasInitializer(cxxConstructExpr( - hasDeclaration(cxxConstructorDecl( - isCopyConstructor())), - hasArgument(0, CopyCtorArg)) - .bind("ctorCall"))) - .bind("newVarDecl"))) + declStmt( + has(varDecl(hasLocalStorage(), + hasType(matchers::isExpensiveToCopy()), + hasInitializer( + cxxConstructExpr( + hasDeclaration(cxxConstructorDecl( + isCopyConstructor())), + hasArgument(0, CopyCtorArg)) + .bind("ctorCall"))) + .bind("newVarDecl"))).bind("declStmt"))) .bind("blockStmt"); }; @@ -84,6 +82,11 @@ const auto *OldVar = Result.Nodes.getNodeAs("oldVarDecl"); const auto *BlockStmt = Result.Nodes.getNodeAs("blockStmt"); const auto *CtorCall = Result.Nodes.getNodeAs("ctorCall"); + // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros + // since we cannot place them correctly. + bool IssueFix = + Result.Nodes.getNodeAs("declStmt")->isSingleDecl() && + !NewVar->getLocation().isMacroID(); // A constructor that looks like T(const T& t, bool arg = false) counts as a // copy only when it is called with default arguments for the arguments after @@ -93,14 +96,16 @@ return; if (OldVar == nullptr) { - handleCopyFromMethodReturn(*NewVar, *BlockStmt, *Result.Context); + handleCopyFromMethodReturn(*NewVar, *BlockStmt, IssueFix, *Result.Context); } else { - handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, *Result.Context); + handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, IssueFix, + *Result.Context); } } void UnnecessaryCopyInitialization::handleCopyFromMethodReturn( - const VarDecl &Var, const Stmt &BlockStmt, ASTContext &Context) { + const VarDecl &Var, const Stmt &BlockStmt, bool IssueFix, + ASTContext &Context) { bool IsConstQualified = Var.getType().isConstQualified(); if (!IsConstQualified && !isOnlyUsedAsConst(Var, BlockStmt, Context)) return; @@ -114,12 +119,13 @@ "const reference but is only used as const " "reference; consider making it a const reference") << &Var; - recordFixes(Var, Context, Diagnostic); + if (IssueFix) + recordFixes(Var, Context, Diagnostic); } void UnnecessaryCopyInitialization::handleCopyFromLocalVar( const VarDecl &NewVar, const VarDecl &OldVar, const Stmt &BlockStmt, - ASTContext &Context) { + bool IssueFix, ASTContext &Context) { if (!isOnlyUsedAsConst(NewVar, BlockStmt, Context) || !isOnlyUsedAsConst(OldVar, BlockStmt, Context)) return; @@ -128,7 +134,8 @@ "local copy %0 of the variable %1 is never modified; " "consider avoiding the copy") << &NewVar << &OldVar; - recordFixes(NewVar, Context, Diagnostic); + if (IssueFix) + recordFixes(NewVar, Context, Diagnostic); } } // namespace performance Index: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp @@ -338,3 +338,10 @@ WeirdCopyCtorType neg_weird_1(orig, false); WeirdCopyCtorType neg_weird_2(orig, true); } + +void WarningOnlyMultiDeclStmt() { + ExpensiveToCopyType orig; + ExpensiveToCopyType copy = orig, copy2; + // CHECK-MESSAGES: [[@LINE-1]]:23: warning: local copy 'copy' of the variable 'orig' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization] + // CHECK-FIXES: ExpensiveToCopyType copy = orig, copy2; +}