shuaiwang created this revision.
shuaiwang added a reviewer: hokein.
Herald added subscribers: cfe-commits, Szelethus, a.sidorin, xazax.hun.
Herald added a reviewer: george.karpenkov.
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`
}
}
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50883
Files:
clang-tidy/utils/ExprMutationAnalyzer.cpp
unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
Index: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
===================================================================
--- unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -606,6 +606,59 @@
EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x.f()"));
}
+TEST(ExprMutationAnalyzerTest, UniquePtr) {
+ const std::string UniquePtrDef =
+ "template <class T> struct UniquePtr {"
+ " UniquePtr();"
+ " UniquePtr(const UniquePtr&) = delete;"
+ " UniquePtr(UniquePtr&&);"
+ " UniquePtr& operator=(const UniquePtr&) = delete;"
+ " UniquePtr& operator=(UniquePtr&&);"
+ " T& operator*() const;"
+ " T* operator->() const;"
+ "};";
+
+ auto AST = tooling::buildASTFromCode(
+ UniquePtrDef + "void f() { UniquePtr<int> x; *x = 10; }");
+ auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("* x = 10"));
+
+ AST = tooling::buildASTFromCode(UniquePtrDef +
+ "void f() { UniquePtr<int> x; *x; }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+
+ AST = tooling::buildASTFromCode(UniquePtrDef +
+ "void f() { UniquePtr<const int> x; *x; }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+
+ AST = tooling::buildASTFromCode(UniquePtrDef +
+ "struct S { int v; };"
+ "void f() { UniquePtr<S> x; x->v; }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
+
+ AST = tooling::buildASTFromCode(UniquePtrDef +
+ "struct S { int v; };"
+ "void f() { UniquePtr<const S> x; x->v; }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+
+ AST = tooling::buildASTFromCode(UniquePtrDef +
+ "struct S { void mf(); };"
+ "void f() { UniquePtr<S> x; x->mf(); }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x"));
+
+ AST = tooling::buildASTFromCode(
+ UniquePtrDef + "struct S { void mf() const; };"
+ "void f() { UniquePtr<const S> x; x->mf(); }");
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
Index: clang-tidy/utils/ExprMutationAnalyzer.cpp
===================================================================
--- clang-tidy/utils/ExprMutationAnalyzer.cpp
+++ clang-tidy/utils/ExprMutationAnalyzer.cpp
@@ -51,6 +51,20 @@
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) {
@@ -163,6 +177,15 @@
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.
const auto NonConstRefParam = forEachArgumentWithParam(
@@ -186,8 +209,8 @@
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);
@@ -236,6 +259,21 @@
}
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(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
