Index: clang-tidy/performance/UnnecessaryCopyInitialization.h =================================================================== --- clang-tidy/performance/UnnecessaryCopyInitialization.h +++ clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -33,7 +33,8 @@ private: void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt, - bool IssueFix, ASTContext &Context); + bool IssueFix, const VarDecl *ObjectArg, + ASTContext &Context); void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar, const Stmt &BlockStmt, bool IssueFix, ASTContext &Context); Index: clang-tidy/performance/UnnecessaryCopyInitialization.cpp =================================================================== --- clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -38,14 +38,15 @@ unless(allOf(pointerType(), unless(pointerType(pointee( qualType(isConstQualified()))))))); - // Match method call expressions where the this argument is a const - // type or const reference. This returned const reference is highly likely to - // outlive the local const reference of the variable being declared. - // The assumption is that the const reference being returned either points - // to a global static variable or to a member of the called object. - auto ConstRefReturningMethodCallOfConstParam = cxxMemberCallExpr( + // Match method call expressions where the `this` argument is only used as + // const, this will be checked in `check()` part. This returned const + // reference is highly likely to outlive the local const reference of the + // variable being declared. The assumption is that the const reference being + // returned either points to a global static variable or to a member of the + // called object. + auto ConstRefReturningMethodCall = cxxMemberCallExpr( callee(cxxMethodDecl(returns(ConstReference))), - on(declRefExpr(to(varDecl(hasType(qualType(ConstOrConstReference))))))); + on(declRefExpr(to(varDecl().bind("objectArg"))))); auto ConstRefReturningFunctionCall = callExpr(callee(functionDecl(returns(ConstReference))), unless(callee(cxxMethodDecl()))); @@ -68,7 +69,7 @@ Finder->addMatcher( localVarCopiedFrom(anyOf(ConstRefReturningFunctionCall, - ConstRefReturningMethodCallOfConstParam)), + ConstRefReturningMethodCall)), this); Finder->addMatcher(localVarCopiedFrom(declRefExpr( @@ -80,6 +81,7 @@ const MatchFinder::MatchResult &Result) { const auto *NewVar = Result.Nodes.getNodeAs("newVarDecl"); const auto *OldVar = Result.Nodes.getNodeAs("oldVarDecl"); + const auto *ObjectArg = Result.Nodes.getNodeAs("objectArg"); 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 @@ -96,7 +98,8 @@ return; if (OldVar == nullptr) { - handleCopyFromMethodReturn(*NewVar, *BlockStmt, IssueFix, *Result.Context); + handleCopyFromMethodReturn(*NewVar, *BlockStmt, IssueFix, ObjectArg, + *Result.Context); } else { handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, IssueFix, *Result.Context); @@ -105,10 +108,13 @@ void UnnecessaryCopyInitialization::handleCopyFromMethodReturn( const VarDecl &Var, const Stmt &BlockStmt, bool IssueFix, - ASTContext &Context) { + const VarDecl *ObjectArg, ASTContext &Context) { bool IsConstQualified = Var.getType().isConstQualified(); if (!IsConstQualified && !isOnlyUsedAsConst(Var, BlockStmt, Context)) return; + if (ObjectArg != nullptr && + !isOnlyUsedAsConst(*ObjectArg, BlockStmt, Context)) + return; auto Diagnostic = diag(Var.getLocation(), Index: test/clang-tidy/performance-unnecessary-copy-initialization.cpp =================================================================== --- test/clang-tidy/performance-unnecessary-copy-initialization.cpp +++ test/clang-tidy/performance-unnecessary-copy-initialization.cpp @@ -36,65 +36,65 @@ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization] // CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference(); const auto AutoCopyConstructed(ExpensiveTypeReference()); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed' // CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveTypeReference()); const ExpensiveToCopyType VarAssigned = ExpensiveTypeReference(); - // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned' // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = ExpensiveTypeReference(); const ExpensiveToCopyType VarCopyConstructed(ExpensiveTypeReference()); - // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed' // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(ExpensiveTypeReference()); } void PositiveMethodCallConstReferenceParam(const ExpensiveToCopyType &Obj) { const auto AutoAssigned = Obj.reference(); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' // CHECK-FIXES: const auto& AutoAssigned = Obj.reference(); const auto AutoCopyConstructed(Obj.reference()); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed' // CHECK-FIXES: const auto& AutoCopyConstructed(Obj.reference()); const ExpensiveToCopyType VarAssigned = Obj.reference(); - // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned' // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj.reference(); const ExpensiveToCopyType VarCopyConstructed(Obj.reference()); - // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed' // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj.reference()); } void PositiveMethodCallConstParam(const ExpensiveToCopyType Obj) { const auto AutoAssigned = Obj.reference(); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' // CHECK-FIXES: const auto& AutoAssigned = Obj.reference(); const auto AutoCopyConstructed(Obj.reference()); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed' // CHECK-FIXES: const auto& AutoCopyConstructed(Obj.reference()); const ExpensiveToCopyType VarAssigned = Obj.reference(); - // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned' // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj.reference(); const ExpensiveToCopyType VarCopyConstructed(Obj.reference()); - // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed' // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj.reference()); } void PositiveMethodCallConstPointerParam(const ExpensiveToCopyType *const Obj) { const auto AutoAssigned = Obj->reference(); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' // CHECK-FIXES: const auto& AutoAssigned = Obj->reference(); const auto AutoCopyConstructed(Obj->reference()); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoCopyConstructed' // CHECK-FIXES: const auto& AutoCopyConstructed(Obj->reference()); const ExpensiveToCopyType VarAssigned = Obj->reference(); - // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarAssigned' // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = Obj->reference(); const ExpensiveToCopyType VarCopyConstructed(Obj->reference()); - // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: the const qualified variable 'VarCopyConstructed' // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(Obj->reference()); } void PositiveLocalConstValue() { const ExpensiveToCopyType Obj; const auto UnnecessaryCopy = Obj.reference(); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'UnnecessaryCopy' // CHECK-FIXES: const auto& UnnecessaryCopy = Obj.reference(); } @@ -102,7 +102,7 @@ const ExpensiveToCopyType Obj; const ExpensiveToCopyType &ConstReference = Obj.reference(); const auto UnnecessaryCopy = ConstReference.reference(); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'UnnecessaryCopy' // CHECK-FIXES: const auto& UnnecessaryCopy = ConstReference.reference(); } @@ -110,7 +110,7 @@ const ExpensiveToCopyType Obj; const ExpensiveToCopyType *const ConstPointer = &Obj; const auto UnnecessaryCopy = ConstPointer->reference(); - // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'UnnecessaryCopy' // CHECK-FIXES: const auto& UnnecessaryCopy = ConstPointer->reference(); } @@ -130,20 +130,20 @@ // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'AutoAssigned' is copy-constructed from a const reference but is only used as const reference; consider making it a const reference [performance-unnecessary-copy-initialization] // CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference(); auto AutoCopyConstructed(ExpensiveTypeReference()); - // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'AutoCopyConstructed' // CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveTypeReference()); ExpensiveToCopyType VarAssigned = ExpensiveTypeReference(); - // CHECK-MESSAGES: [[@LINE-1]]:23: warning: the variable + // CHECK-MESSAGES: [[@LINE-1]]:23: warning: the variable 'VarAssigned' // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = ExpensiveTypeReference(); ExpensiveToCopyType VarCopyConstructed(ExpensiveTypeReference()); - // CHECK-MESSAGES: [[@LINE-1]]:23: warning: the variable + // CHECK-MESSAGES: [[@LINE-1]]:23: warning: the variable 'VarCopyConstructed' // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(ExpensiveTypeReference()); } void positiveNonConstVarInCodeBlock(const ExpensiveToCopyType &Obj) { { auto Assigned = Obj.reference(); - // CHECK-MESSAGES: [[@LINE-1]]:10: warning: the variable + // CHECK-MESSAGES: [[@LINE-1]]:10: warning: the variable 'Assigned' // CHECK-FIXES: const auto& Assigned = Obj.reference(); Assigned.reference(); useAsConstReference(Assigned); @@ -174,33 +174,57 @@ } } -void NegativeMethodCallNonConstRef(ExpensiveToCopyType &Obj) { +void PositiveMethodCallNonConstRefNotModified(ExpensiveToCopyType &Obj) { const auto AutoAssigned = Obj.reference(); - const auto AutoCopyConstructed(Obj.reference()); - const ExpensiveToCopyType VarAssigned = Obj.reference(); - const ExpensiveToCopyType VarCopyConstructed(Obj.reference()); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' + // CHECK-FIXES: const auto& AutoAssigned = Obj.reference(); } -void NegativeMethodCallNonConst(ExpensiveToCopyType Obj) { +void NegativeMethodCallNonConstRefIsModified(ExpensiveToCopyType &Obj) { const auto AutoAssigned = Obj.reference(); const auto AutoCopyConstructed(Obj.reference()); const ExpensiveToCopyType VarAssigned = Obj.reference(); const ExpensiveToCopyType VarCopyConstructed(Obj.reference()); + mutate(&Obj); +} + +void PositiveMethodCallNonConstNotModified(ExpensiveToCopyType Obj) { + const auto AutoAssigned = Obj.reference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' + // CHECK-FIXES: const auto& AutoAssigned = Obj.reference(); } -void NegativeMethodCallNonConstPointer(ExpensiveToCopyType *const Obj) { +void NegativeMethodCallNonConstValueArgumentIsModified(ExpensiveToCopyType Obj) { + Obj.nonConstMethod(); + const auto AutoAssigned = Obj.reference(); +} + +void PositiveMethodCallNonConstPointerNotModified(ExpensiveToCopyType *const Obj) { + const auto AutoAssigned = Obj->reference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' + // CHECK-FIXES: const auto& AutoAssigned = Obj->reference(); + Obj->constMethod(); +} + +void NegativeMethodCallNonConstPointerIsModified(ExpensiveToCopyType *const Obj) { const auto AutoAssigned = Obj->reference(); const auto AutoCopyConstructed(Obj->reference()); const ExpensiveToCopyType VarAssigned = Obj->reference(); const ExpensiveToCopyType VarCopyConstructed(Obj->reference()); + mutate(Obj); } -void NegativeObjIsNotParam() { +void PositiveLocalVarIsNotModified() { + ExpensiveToCopyType LocalVar; + const auto AutoAssigned = LocalVar.reference(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' + // CHECK-FIXES: const auto& AutoAssigned = LocalVar.reference(); +} + +void NegativeLocalVarIsModified() { ExpensiveToCopyType Obj; const auto AutoAssigned = Obj.reference(); - const auto AutoCopyConstructed(Obj.reference()); - ExpensiveToCopyType VarAssigned = Obj.reference(); - ExpensiveToCopyType VarCopyConstructed(Obj.reference()); + Obj = AutoAssigned; } struct NegativeConstructor { @@ -338,7 +362,6 @@ WeirdCopyCtorType neg_weird_1(orig, false); WeirdCopyCtorType neg_weird_2(orig, true); } - void WarningOnlyMultiDeclStmt() { ExpensiveToCopyType orig; ExpensiveToCopyType copy = orig, copy2;