[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
llvm-svn: 271239
diff --git a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index dfaff82..e486350 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/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<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 @@
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(),