[clang-tidy] Handle sugared reference types in ExprMutationAnalyzer

Summary:
This handles cases like this:
```
typedef int& IntRef;
void mutate(IntRef);
void f() {
  int x;
  mutate(x);
}
```
where the param type is a sugared type (`TypedefType`) instead of a
reference type directly.

Note that another category of similar but different cases are already
handled properly before:
```
typedef int Int;
void mutate(Int&);
void f() {
  int x;
  mutate(x);
}
```

Reviewers: aaron.ballman, alexfh, george.karpenkov

Subscribers: xazax.hun, a.sidorin, Szelethus, cfe-commits

Differential Revision: https://reviews.llvm.org/D50953

llvm-svn: 341986

2 files changed, 184 insertions, 24 deletions

diff --git a/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.cpp
index 1ec1f7270f3..e04ac724c84 100644
--- a/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ExprMutationAnalyzer.cpp
@@ -48,11 +48,13 @@ AST_MATCHER_P(GenericSelectionExpr, hasControllingExpr,
 }
 
 const auto nonConstReferenceType = [] {
-  return referenceType(pointee(unless(isConstQualified())));
+  return hasUnqualifiedDesugaredType(
+      referenceType(pointee(unless(isConstQualified()))));
 };
 
 const auto nonConstPointerType = [] {
-  return pointerType(pointee(unless(isConstQualified())));
+  return hasUnqualifiedDesugaredType(
+      pointerType(pointee(unless(isConstQualified()))));
 };
 
 const auto isMoveOnly = [] {
@@ -185,12 +187,11 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const Expr *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)));
+  const auto AsOperatorArrowThis =
+      cxxOperatorCallExpr(hasOverloadedOperatorName("->"),
+                          callee(cxxMethodDecl(ofClass(isMoveOnly()),
+                                               returns(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.
diff --git a/clang-tools-extra/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp b/clang-tools-extra/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
index 14878ed119f..7f4a4cc545b 100644
--- a/clang-tools-extra/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/ExprMutationAnalyzerTest.cpp
@@ -168,6 +168,15 @@ TEST(ExprMutationAnalyzerTest, ByValueArgument) {
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
+  AST = tooling::buildASTFromCode("void g(int*); void f() { int* x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode("typedef int* IntPtr;"
+                                  "void g(IntPtr); void f() { int* x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
   AST = tooling::buildASTFromCode(
       "void f() { struct A {}; A operator+(A, int); A x; x + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
@@ -184,6 +193,40 @@ TEST(ExprMutationAnalyzerTest, ByValueArgument) {
   EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
+TEST(ExprMutationAnalyzerTest, ByConstValueArgument) {
+  auto AST =
+      tooling::buildASTFromCode("void g(const int); void f() { int x; g(x); }");
+  auto Results =
+      match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "void g(int* const); void f() { int* x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST =
+      tooling::buildASTFromCode("typedef int* const CIntPtr;"
+                                "void g(CIntPtr); void f() { int* x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "void f() { struct A {}; A operator+(const A, int); A x; x + 1; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "void f() { struct A { A(const int); }; int x; A y(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "void f() { struct A { A(); A(const A); }; A x; A y(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
 TEST(ExprMutationAnalyzerTest, ByNonConstRefArgument) {
   auto AST =
       tooling::buildASTFromCode("void g(int&); void f() { int x; g(x); }");
@@ -191,6 +234,36 @@ TEST(ExprMutationAnalyzerTest, ByNonConstRefArgument) {
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
 
+  AST = tooling::buildASTFromCode("typedef int& IntRef;"
+                                  "void g(IntRef); void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+  AST =
+      tooling::buildASTFromCode("template <class T> using TRef = T&;"
+                                "void g(TRef<int>); void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+  AST = tooling::buildASTFromCode(
+      "template <class T> struct identity { using type = T; };"
+      "template <class T, class U = T&> void g(typename identity<U>::type);"
+      "void f() { int x; g<int>(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g<int>(x)"));
+
+  AST =
+      tooling::buildASTFromCode("typedef int* IntPtr;"
+                                "void g(IntPtr&); void f() { int* x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
+  AST = tooling::buildASTFromCode(
+      "typedef int* IntPtr; typedef IntPtr& IntPtrRef;"
+      "void g(IntPtrRef); void f() { int* x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(x)"));
+
   AST = tooling::buildASTFromCode(
       "void f() { struct A {}; A operator+(A&, int); A x; x + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
@@ -214,6 +287,25 @@ TEST(ExprMutationAnalyzerTest, ByConstRefArgument) {
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
 
+  AST = tooling::buildASTFromCode("typedef const int& CIntRef;"
+                                  "void g(CIntRef); void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "template <class T> using CTRef = const T&;"
+      "void g(CTRef<int>); void f() { int x; g(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "template <class T> struct identity { using type = T; };"
+      "template <class T, class U = const T&>"
+      "void g(typename identity<U>::type);"
+      "void f() { int x; g<int>(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
   AST = tooling::buildASTFromCode(
       "void f() { struct A {}; A operator+(const A&, int); A x; x + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
@@ -369,10 +461,19 @@ TEST(ExprMutationAnalyzerTest, CallUnresolved) {
 }
 
 TEST(ExprMutationAnalyzerTest, ReturnAsValue) {
-  const auto AST = tooling::buildASTFromCode("int f() { int x; return x; }");
-  const auto Results =
+  auto AST = tooling::buildASTFromCode("int f() { int x; return x; }");
+  auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode("int* f() { int* x; return x; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode("typedef int* IntPtr;"
+                                  "IntPtr f() { int* x; return x; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, ReturnAsNonConstRef) {
@@ -440,22 +541,44 @@ TEST(ExprMutationAnalyzerTest, TemplateWithArrayToPointerDecay) {
 }
 
 TEST(ExprMutationAnalyzerTest, FollowRefModified) {
-  const auto AST = tooling::buildASTFromCode(
+  auto AST = tooling::buildASTFromCode(
       "void f() { int x; int& r0 = x; int& r1 = r0; int& r2 = r1; "
       "int& r3 = r2; r3 = 10; }");
-  const auto Results =
+  auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()),
               ElementsAre("r0", "r1", "r2", "r3", "r3 = 10"));
+
+  AST = tooling::buildASTFromCode(
+      "typedef int& IntRefX;"
+      "using IntRefY = int&;"
+      "void f() { int x; IntRefX r0 = x; IntRefY r1 = r0;"
+      "decltype((x)) r2 = r1; r2 = 10; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()),
+              ElementsAre("r0", "r1", "r2", "r2 = 10"));
 }
 
 TEST(ExprMutationAnalyzerTest, FollowRefNotModified) {
-  const auto AST = tooling::buildASTFromCode(
+  auto AST = tooling::buildASTFromCode(
       "void f() { int x; int& r0 = x; int& r1 = r0; int& r2 = r1; "
       "int& r3 = r2; int& r4 = r3; int& r5 = r4;}");
-  const auto Results =
+  auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "void f() { int x; int& r0 = x; const int& r1 = r0;}");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "typedef const int& CIntRefX;"
+      "using CIntRefY = const int&;"
+      "void f() { int x; int& r0 = x; CIntRefX r1 = r0;"
+      "CIntRefY r2 = r1; decltype((r1)) r3 = r2;}");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, FollowConditionalRefModified) {
@@ -542,12 +665,19 @@ TEST(ExprMutationAnalyzerTest, CastToValue) {
 }
 
 TEST(ExprMutationAnalyzerTest, CastToRefModified) {
-  const auto AST = tooling::buildASTFromCode(
+  auto AST = tooling::buildASTFromCode(
       "void f() { int x; static_cast<int &>(x) = 10; }");
-  const auto Results =
+  auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()),
               ElementsAre("static_cast<int &>(x) = 10"));
+
+  AST = tooling::buildASTFromCode(
+      "typedef int& IntRef;"
+      "void f() { int x; static_cast<IntRef>(x) = 10; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()),
+              ElementsAre("static_cast<IntRef>(x) = 10"));
 }
 
 TEST(ExprMutationAnalyzerTest, CastToRefNotModified) {
@@ -559,11 +689,17 @@ TEST(ExprMutationAnalyzerTest, CastToRefNotModified) {
 }
 
 TEST(ExprMutationAnalyzerTest, CastToConstRef) {
-  const auto AST = tooling::buildASTFromCode(
+  auto AST = tooling::buildASTFromCode(
       "void f() { int x; static_cast<const int&>(x); }");
-  const auto Results =
+  auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST =
+      tooling::buildASTFromCode("typedef const int& CIntRef;"
+                                "void f() { int x; static_cast<CIntRef>(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, LambdaDefaultCaptureByValue) {
@@ -601,11 +737,17 @@ TEST(ExprMutationAnalyzerTest, LambdaExplicitCaptureByRef) {
 }
 
 TEST(ExprMutationAnalyzerTest, RangeForArrayByRefModified) {
-  const auto AST = tooling::buildASTFromCode(
+  auto AST = tooling::buildASTFromCode(
       "void f() { int x[2]; for (int& e : x) e = 10; }");
-  const auto Results =
+  auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10"));
+
+  AST = tooling::buildASTFromCode(
+      "typedef int& IntRef;"
+      "void f() { int x[2]; for (IntRef e : x) e = 10; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("e", "e = 10"));
 }
 
 TEST(ExprMutationAnalyzerTest, RangeForArrayByRefNotModified) {
@@ -617,19 +759,36 @@ TEST(ExprMutationAnalyzerTest, RangeForArrayByRefNotModified) {
 }
 
 TEST(ExprMutationAnalyzerTest, RangeForArrayByValue) {
-  const auto AST = tooling::buildASTFromCode(
+  auto AST = tooling::buildASTFromCode(
       "void f() { int x[2]; for (int e : x) e = 10; }");
-  const auto Results =
+  auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "void f() { int* x[2]; for (int* e : x) e = nullptr; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "typedef int* IntPtr;"
+      "void f() { int* x[2]; for (IntPtr e : x) e = nullptr; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, RangeForArrayByConstRef) {
-  const auto AST = tooling::buildASTFromCode(
+  auto AST = tooling::buildASTFromCode(
       "void f() { int x[2]; for (const int& e : x) e; }");
-  const auto Results =
+  auto Results =
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+      "typedef const int& CIntRef;"
+      "void f() { int x[2]; for (CIntRef e : x) e; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, RangeForNonArrayByRefModified) {