flx created this revision.
flx added a subscriber: cfe-commits.
flx set the repository for this revision to rL LLVM.
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.
Repository:
rL LLVM
http://reviews.llvm.org/D20010
Files:
clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tidy/performance/UnnecessaryCopyInitialization.h
test/clang-tidy/performance-unnecessary-copy-initialization.cpp
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
@@ -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
+ // 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
+ // CHECK-FIXES: const auto& AutoAssigned = Obj.reference();
+}
+
+void NegativeMethodCallNonConstValueArgumentIsModified(ExpensiveToCopyType Obj) {
+ Obj.nonConstMethod();
+ const auto AutoAssigned = Obj.reference();
}
-void NegativeMethodCallNonConstPointer(ExpensiveToCopyType *const Obj) {
+void PositiveMethodCallNonConstPointerNotModified(ExpensiveToCopyType *const Obj) {
+ const auto AutoAssigned = Obj->reference();
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const
+ // 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 PositiveLocalVarIsNotModified() {
+ ExpensiveToCopyType LocalVar;
+ const auto AutoAssigned = LocalVar.reference();
+ // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const
+ // CHECK-FIXES: const auto& AutoAssigned = LocalVar.reference();
}
-void NegativeObjIsNotParam() {
+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 {
Index: clang-tidy/performance/UnnecessaryCopyInitialization.h
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -33,6 +33,7 @@
private:
void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt,
+ const VarDecl *ObjectArg,
ASTContext &Context);
void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar,
const Stmt &BlockStmt, ASTContext &Context);
Index: clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -42,14 +42,14 @@
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())));
@@ -70,7 +70,7 @@
Finder->addMatcher(
localVarCopiedFrom(anyOf(ConstRefReturningFunctionCall,
- ConstRefReturningMethodCallOfConstParam)),
+ ConstRefReturningMethodCall)),
this);
Finder->addMatcher(localVarCopiedFrom(declRefExpr(
@@ -82,6 +82,7 @@
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");
@@ -93,17 +94,21 @@
return;
if (OldVar == nullptr) {
- handleCopyFromMethodReturn(*NewVar, *BlockStmt, *Result.Context);
+ handleCopyFromMethodReturn(*NewVar, *BlockStmt, ObjectArg, *Result.Context);
} else {
handleCopyFromLocalVar(*NewVar, *OldVar, *BlockStmt, *Result.Context);
}
}
void UnnecessaryCopyInitialization::handleCopyFromMethodReturn(
- const VarDecl &Var, const Stmt &BlockStmt, ASTContext &Context) {
+ const VarDecl &Var, const Stmt &BlockStmt, 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(),
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
