shuaiwang updated this revision to Diff 164810.
shuaiwang marked an inline comment as done.
shuaiwang added a comment.
rebase & add test case
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
@@ -724,6 +724,65 @@
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()));
+
+ AST = tooling::buildASTFromCodeWithArgs(
+ UniquePtrDef + "template <class T> void f() { UniquePtr<T> x; x->mf(); }",
+ {"-fno-delayed-template-parsing"});
+ Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("x->mf()"));
+}
+
} // 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) {
@@ -168,6 +182,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.
// An argument is assumed to be non-const-ref when the function is unresolved.
@@ -197,8 +220,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);
@@ -250,6 +273,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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
