[clang-tools-extra] r271239 - [clang-tidy] UnnecessaryCopyInitialization - Extend to trigger on non-const "this" object argument if it is not modified.
Felix Berger via cfe-commits
cfe-commits at lists.llvm.org
Mon May 30 17:25:58 PDT 2016
Author: flx
Date: Mon May 30 19:25:57 2016
New Revision: 271239
URL: http://llvm.org/viewvc/llvm-project?rev=271239&view=rev
Log:
[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
Modified:
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp
Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp?rev=271239&r1=271238&r2=271239&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp Mon May 30 19:25:57 2016
@@ -38,14 +38,15 @@ void UnnecessaryCopyInitialization::regi
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::regi
Finder->addMatcher(
localVarCopiedFrom(anyOf(ConstRefReturningFunctionCall,
- ConstRefReturningMethodCallOfConstParam)),
+ ConstRefReturningMethodCall)),
this);
Finder->addMatcher(localVarCopiedFrom(declRefExpr(
@@ -80,6 +81,7 @@ void UnnecessaryCopyInitialization::chec
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::chec
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::chec
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(),
Modified: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h?rev=271239&r1=271238&r2=271239&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h (original)
+++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h Mon May 30 19:25:57 2016
@@ -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);
Modified: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp?rev=271239&r1=271238&r2=271239&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization.cpp Mon May 30 19:25:57 2016
@@ -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 PositiveFunctionCallExpensiveTypeNo
// 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(
}
}
-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 @@ void NegativeLocalCopyWeirdNonCopy() {
WeirdCopyCtorType neg_weird_1(orig, false);
WeirdCopyCtorType neg_weird_2(orig, true);
}
-
void WarningOnlyMultiDeclStmt() {
ExpensiveToCopyType orig;
ExpensiveToCopyType copy = orig, copy2;
More information about the cfe-commits
mailing list