[clang-tools-extra] r341967 - [clang-tidy] Handle unique owning smart pointers in ExprMutationAnalyzer
Shuai Wang via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 11 10:33:12 PDT 2018
Author: shuaiwang
Date: Tue Sep 11 10:33:12 2018
New Revision: 341967
URL: http://llvm.org/viewvc/llvm-project?rev=341967&view=rev
Log:
[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
Modified:
clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
Modified: clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp?rev=341967&r1=341966&r2=341967&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExprMutationAnalyzer.cpp Tue Sep 11 10:33:12 2018
@@ -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::findDi
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::findDi
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::findRa
}
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(
Modified: clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp?rev=341967&r1=341966&r2=341967&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp Tue Sep 11 10:33:12 2018
@@ -724,6 +724,65 @@ TEST(ExprMutationAnalyzerTest, NotUneval
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
More information about the cfe-commits
mailing list