diff options
author | Shuai Wang <shuaiwang@google.com> | 2018-09-11 17:33:12 +0000 |
---|---|---|
committer | Shuai Wang <shuaiwang@google.com> | 2018-09-11 17:33:12 +0000 |
commit | 4fac53f8cbca67a03c5079b45febe04b8e20f926 (patch) | |
tree | 7c8533a87cdb7f60fc76e76b9e8bdceef99a0fd9 /clang-tidy/utils | |
parent | dff75f4977889614ff85f7eb1f7d1b492f00ebd0 (diff) |
[clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer
Summary:
For smart pointers like std::unique_ptr which uniquely owns the
underlying object, treat the mutation of the pointee as mutation of the
smart pointer itself.
This gives better behavior for cases like this:
```
void f(std::vector<std::unique_ptr<Foo>> v) { // undesirable analyze result of `v` as not mutated.
for (auto& p : v) {
p->mutate(); // only const member function `operator->` is invoked on `p`
}
}
```
Reviewers: hokein, george.karpenkov
Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D50883
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@341967 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'clang-tidy/utils')
-rw-r--r-- | clang-tidy/utils/ExprMutationAnalyzer.cpp | 42 |
1 files changed, 40 insertions, 2 deletions
diff --git a/clang-tidy/utils/ExprMutationAnalyzer.cpp b/clang-tidy/utils/ExprMutationAnalyzer.cpp index f979e97a..1ec1f727 100644 --- a/clang-tidy/utils/ExprMutationAnalyzer.cpp +++ b/clang-tidy/utils/ExprMutationAnalyzer.cpp @@ -51,6 +51,20 @@ const auto nonConstReferenceType = [] { return referenceType(pointee(unless(isConstQualified()))); }; +const auto nonConstPointerType = [] { + return pointerType(pointee(unless(isConstQualified()))); +}; + +const auto isMoveOnly = [] { + return cxxRecordDecl( + hasMethod(cxxConstructorDecl(isMoveConstructor(), unless(isDeleted()))), + hasMethod(cxxMethodDecl(isMoveAssignmentOperator(), unless(isDeleted()))), + unless(anyOf(hasMethod(cxxConstructorDecl(isCopyConstructor(), + unless(isDeleted()))), + hasMethod(cxxMethodDecl(isCopyAssignmentOperator(), + unless(isDeleted())))))); +}; + } // namespace const Stmt *ExprMutationAnalyzer::findMutation(const Expr *Exp) { @@ -168,6 +182,15 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) { const auto AsPointerFromArrayDecay = castExpr(hasCastKind(CK_ArrayToPointerDecay), unless(hasParent(arraySubscriptExpr())), has(equalsNode(Exp))); + // Treat calling `operator->()` of move-only classes as taking address. + // These are typically smart pointers with unique ownership so we treat + // mutation of pointee as mutation of the smart pointer itself. + const auto AsOperatorArrowThis = cxxOperatorCallExpr( + hasOverloadedOperatorName("->"), + callee(cxxMethodDecl( + ofClass(isMoveOnly()), + returns(hasUnqualifiedDesugaredType(nonConstPointerType())))), + argumentCountIs(1), hasArgument(0, equalsNode(Exp))); // Used as non-const-ref argument when calling a function. // An argument is assumed to be non-const-ref when the function is unresolved. @@ -197,8 +220,8 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *Exp) { const auto Matches = match(findAll(stmt(anyOf(AsAssignmentLhs, AsIncDecOperand, AsNonConstThis, AsAmpersandOperand, AsPointerFromArrayDecay, - AsNonConstRefArg, AsLambdaRefCaptureInit, - AsNonConstRefReturn)) + AsOperatorArrowThis, AsNonConstRefArg, + AsLambdaRefCaptureInit, AsNonConstRefReturn)) .bind("stmt")), Stm, Context); return selectFirst<Stmt>("stmt", Matches); @@ -250,6 +273,21 @@ const Stmt *ExprMutationAnalyzer::findRangeLoopMutation(const Expr *Exp) { } const Stmt *ExprMutationAnalyzer::findReferenceMutation(const Expr *Exp) { + // Follow non-const reference returned by `operator*()` of move-only classes. + // These are typically smart pointers with unique ownership so we treat + // mutation of pointee as mutation of the smart pointer itself. + const auto Ref = match( + findAll(cxxOperatorCallExpr( + hasOverloadedOperatorName("*"), + callee(cxxMethodDecl(ofClass(isMoveOnly()), + returns(hasUnqualifiedDesugaredType( + nonConstReferenceType())))), + argumentCountIs(1), hasArgument(0, equalsNode(Exp))) + .bind("expr")), + Stm, Context); + if (const Stmt *S = findExprMutation(Ref)) + return S; + // If 'Exp' is bound to a non-const reference, check all declRefExpr to that. const auto Refs = match( stmt(forEachDescendant( |