[clang-tidy] UnnecessaryCopyInitialization - Extend to trigger on non-const "this" object argument if it is not modified.
Summary: Also trigger the check in the following case: void foo() { ExpensiveToCopy Obj; const auto UnnecessaryCopy = Obj.constReference(); Obj.onlyUsedAsConst(); } i.e. when the object the method is called on is not const but is never modified. Reviewers: alexfh, fowles Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D20010 git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@271239 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Родитель
d3a1e8c51d
Коммит
f506f18103
|
@ -38,14 +38,15 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
|
|||
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 @@ void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) {
|
|||
|
||||
Finder->addMatcher(
|
||||
localVarCopiedFrom(anyOf(ConstRefReturningFunctionCall,
|
||||
ConstRefReturningMethodCallOfConstParam)),
|
||||
ConstRefReturningMethodCall)),
|
||||
this);
|
||||
|
||||
Finder->addMatcher(localVarCopiedFrom(declRefExpr(
|
||||
|
@ -80,6 +81,7 @@ void UnnecessaryCopyInitialization::check(
|
|||
const MatchFinder::MatchResult &Result) {
|
||||
const auto *NewVar = Result.Nodes.getNodeAs<VarDecl>("newVarDecl");
|
||||
const auto *OldVar = Result.Nodes.getNodeAs<VarDecl>("oldVarDecl");
|
||||
const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>("objectArg");
|
||||
const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
|
||||
const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
|
||||
// Do not propose fixes if the DeclStmt has multiple VarDecls or in macros
|
||||
|
@ -96,7 +98,8 @@ void UnnecessaryCopyInitialization::check(
|
|||
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::check(
|
|||
|
||||
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(),
|
||||
|
|
|
@ -33,7 +33,8 @@ public:
|
|||
|
||||
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);
|
||||
|
|
|
@ -36,65 +36,65 @@ void PositiveFunctionCall() {
|
|||
// 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 @@ void PositiveLocalConstRef() {
|
|||
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 @@ void PositiveLocalConstPointer() {
|
|||
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 @@ void PositiveFunctionCallExpensiveTypeNonConstVariable() {
|
|||
// 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 negativeNonConstVarWithNonConstUse(const ExpensiveToCopyType &Obj) {
|
|||
}
|
||||
}
|
||||
|
||||
void NegativeMethodCallNonConstRef(ExpensiveToCopyType &Obj) {
|
||||
void PositiveMethodCallNonConstRefNotModified(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 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 NegativeMethodCallNonConst(ExpensiveToCopyType Obj) {
|
||||
void PositiveMethodCallNonConstNotModified(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 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 @@ void NegativeLocalCopyWeirdNonCopy() {
|
|||
WeirdCopyCtorType neg_weird_1(orig, false);
|
||||
WeirdCopyCtorType neg_weird_2(orig, true);
|
||||
}
|
||||
|
||||
void WarningOnlyMultiDeclStmt() {
|
||||
ExpensiveToCopyType orig;
|
||||
ExpensiveToCopyType copy = orig, copy2;
|
||||
|
|
Загрузка…
Ссылка в новой задаче