[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

llvm-svn: 341967

clang-tools-extra/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(

clang-tools-extra/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp

@@ -724,6 +724,65 @@ TEST(ExprMutationAnalyzerTest, NotUnevaluatedExpressions) {
   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