[libTooling] Add support for smart pointers to relevant Transformer `Stencil`s.
authorYitzhak Mandelbaum <yitzhakm@google.com>
Mon, 21 Dec 2020 14:49:17 +0000 (14:49 +0000)
committerYitzhak Mandelbaum <yitzhakm@google.com>
Tue, 5 Jan 2021 17:57:41 +0000 (17:57 +0000)
Stencils `maybeDeref` and `maybeAddressOf` are designed to handle nodes that may
be pointers. Currently, they only handle native pointers. This patch extends the
support to recognize smart pointers and handle them as well.

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

clang/include/clang/Tooling/Transformer/Stencil.h
clang/lib/Tooling/Transformer/Stencil.cpp
clang/unittests/Tooling/StencilTest.cpp

index b729c82..1b7495e 100644 (file)
@@ -82,7 +82,6 @@ Stencil deref(llvm::StringRef ExprId);
 /// If \p ExprId is of pointer type, constructs an idiomatic dereferencing of
 /// the expression bound to \p ExprId, including wrapping it in parentheses, if
 /// needed. Otherwise, generates the original expression source.
-/// FIXME: Identify smart-pointers as pointer types.
 Stencil maybeDeref(llvm::StringRef ExprId);
 
 /// Constructs an expression that idiomatically takes the address of the
@@ -94,7 +93,6 @@ Stencil addressOf(llvm::StringRef ExprId);
 /// idiomatically takes the address of the expression bound to \p ExprId,
 /// including wrapping \p ExprId in parentheses, if needed. Otherwise, generates
 /// the original expression source.
-/// FIXME: Identify smart-pointers as pointer types.
 Stencil maybeAddressOf(llvm::StringRef ExprId);
 
 /// Constructs a `MemberExpr` that accesses the named member (\p Member) of the
index 56f1453..d46087e 100644 (file)
@@ -195,6 +195,24 @@ Error evalData(const DebugPrintNodeData &Data,
   return printNode(Data.Id, Match, Result);
 }
 
+// FIXME: Consider memoizing this function using the `ASTContext`.
+static bool isSmartPointerType(QualType Ty, ASTContext &Context) {
+  using namespace ::clang::ast_matchers;
+
+  // Optimization: hard-code common smart-pointer types. This can/should be
+  // removed if we start caching the results of this function.
+  auto KnownSmartPointer =
+      cxxRecordDecl(hasAnyName("::std::unique_ptr", "::std::shared_ptr"));
+  const auto QuacksLikeASmartPointer = cxxRecordDecl(
+      hasMethod(cxxMethodDecl(hasOverloadedOperatorName("->"),
+                              returns(qualType(pointsTo(type()))))),
+      hasMethod(cxxMethodDecl(hasOverloadedOperatorName("*"),
+                              returns(qualType(references(type()))))));
+  const auto SmartPointer = qualType(hasDeclaration(
+      cxxRecordDecl(anyOf(KnownSmartPointer, QuacksLikeASmartPointer))));
+  return match(SmartPointer, Ty, Context).size() > 0;
+}
+
 Error evalData(const UnaryOperationData &Data,
                const MatchFinder::MatchResult &Match, std::string *Result) {
   // The `Describe` operation can be applied to any node, not just expressions,
@@ -215,17 +233,37 @@ Error evalData(const UnaryOperationData &Data,
     Source = tooling::buildDereference(*E, *Match.Context);
     break;
   case UnaryNodeOperator::MaybeDeref:
-    if (!E->getType()->isAnyPointerType()) {
-      *Result += tooling::getText(*E, *Match.Context);
-      return Error::success();
+    if (E->getType()->isAnyPointerType() ||
+        isSmartPointerType(E->getType(), *Match.Context)) {
+      // Strip off any operator->. This can only occur inside an actual arrow
+      // member access, so we treat it as equivalent to an actual object
+      // expression.
+      if (const auto *OpCall = dyn_cast<clang::CXXOperatorCallExpr>(E)) {
+        if (OpCall->getOperator() == clang::OO_Arrow &&
+            OpCall->getNumArgs() == 1) {
+          E = OpCall->getArg(0);
+        }
+      }
+      Source = tooling::buildDereference(*E, *Match.Context);
+      break;
     }
-    Source = tooling::buildDereference(*E, *Match.Context);
-    break;
+    *Result += tooling::getText(*E, *Match.Context);
+    return Error::success();
   case UnaryNodeOperator::AddressOf:
     Source = tooling::buildAddressOf(*E, *Match.Context);
     break;
   case UnaryNodeOperator::MaybeAddressOf:
-    if (E->getType()->isAnyPointerType()) {
+    if (E->getType()->isAnyPointerType() ||
+        isSmartPointerType(E->getType(), *Match.Context)) {
+      // Strip off any operator->. This can only occur inside an actual arrow
+      // member access, so we treat it as equivalent to an actual object
+      // expression.
+      if (const auto *OpCall = dyn_cast<clang::CXXOperatorCallExpr>(E)) {
+        if (OpCall->getOperator() == clang::OO_Arrow &&
+            OpCall->getNumArgs() == 1) {
+          E = OpCall->getArg(0);
+        }
+      }
       *Result += tooling::getText(*E, *Match.Context);
       return Error::success();
     }
index b270e9d..2333319 100644 (file)
@@ -8,6 +8,7 @@
 
 #include "clang/Tooling/Transformer/Stencil.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/FixIt.h"
 #include "clang/Tooling/Tooling.h"
@@ -29,10 +30,18 @@ using ::testing::HasSubstr;
 using MatchResult = MatchFinder::MatchResult;
 
 // Create a valid translation-unit from a statement.
-static std::string wrapSnippet(StringRef StatementCode) {
-  return ("namespace N { class C {}; } "
-          "namespace { class AnonC {}; } "
-          "struct S { int field; }; auto stencil_test_snippet = []{" +
+static std::string wrapSnippet(StringRef ExtraPreface,
+                               StringRef StatementCode) {
+  constexpr char Preface[] = R"cc(
+    namespace N { class C {}; }
+    namespace { class AnonC {}; }
+    struct S { int Field; };
+    struct Smart {
+      S* operator->() const;
+      S& operator*() const;
+    };
+  )cc";
+  return (Preface + ExtraPreface + "auto stencil_test_snippet = []{" +
           StatementCode + "};")
       .str();
 }
@@ -55,10 +64,12 @@ struct TestMatch {
 // matcher correspondingly. `Matcher` should match one of the statements in
 // `StatementCode` exactly -- that is, produce exactly one match. However,
 // `StatementCode` may contain other statements not described by `Matcher`.
+// `ExtraPreface` (optionally) adds extra decls to the TU, before the code.
 static llvm::Optional<TestMatch> matchStmt(StringRef StatementCode,
-                                           StatementMatcher Matcher) {
-  auto AstUnit = tooling::buildASTFromCodeWithArgs(wrapSnippet(StatementCode),
-                                                   {"-Wno-unused-value"});
+                                           StatementMatcher Matcher,
+                                           StringRef ExtraPreface = "") {
+  auto AstUnit = tooling::buildASTFromCodeWithArgs(
+      wrapSnippet(ExtraPreface, StatementCode), {"-Wno-unused-value"});
   if (AstUnit == nullptr) {
     ADD_FAILURE() << "AST construction failed";
     return llvm::None;
@@ -260,6 +271,42 @@ TEST_F(StencilTest, MaybeDerefAddressExpr) {
   testExpr(Id, "int x; &x;", maybeDeref(Id), "x");
 }
 
+TEST_F(StencilTest, MaybeDerefSmartPointer) {
+  StringRef Id = "id";
+  std::string Snippet = R"cc(
+    Smart x;
+    x;
+  )cc";
+  testExpr(Id, Snippet, maybeDeref(Id), "*x");
+}
+
+// Tests that unique_ptr specifically is handled.
+TEST_F(StencilTest, MaybeDerefSmartPointerUniquePtr) {
+  StringRef Id = "id";
+  // We deliberately specify `unique_ptr` as empty to verify that it matches
+  // because of its name, rather than its contents.
+  StringRef ExtraPreface =
+      "namespace std { template <typename T> class unique_ptr {}; }\n";
+  StringRef Snippet = R"cc(
+    std::unique_ptr<int> x;
+    x;
+  )cc";
+  auto StmtMatch = matchStmt(Snippet, expr().bind(Id), ExtraPreface);
+  ASSERT_TRUE(StmtMatch);
+  EXPECT_THAT_EXPECTED(maybeDeref(Id)->eval(StmtMatch->Result),
+                       HasValue(std::string("*x")));
+}
+
+TEST_F(StencilTest, MaybeDerefSmartPointerFromMemberExpr) {
+  StringRef Id = "id";
+  std::string Snippet = "Smart x; x->Field;";
+  auto StmtMatch =
+      matchStmt(Snippet, memberExpr(hasObjectExpression(expr().bind(Id))));
+  ASSERT_TRUE(StmtMatch);
+  const Stencil Stencil = maybeDeref(Id);
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("*x"));
+}
+
 TEST_F(StencilTest, MaybeAddressOfPointer) {
   StringRef Id = "id";
   testExpr(Id, "int *x; x;", maybeAddressOf(Id), "x");
@@ -280,6 +327,26 @@ TEST_F(StencilTest, MaybeAddressOfDerefExpr) {
   testExpr(Id, "int *x; *x;", addressOf(Id), "x");
 }
 
+TEST_F(StencilTest, MaybeAddressOfSmartPointer) {
+  StringRef Id = "id";
+  testExpr(Id, "Smart x; x;", maybeAddressOf(Id), "x");
+}
+
+TEST_F(StencilTest, MaybeAddressOfSmartPointerFromMemberCall) {
+  StringRef Id = "id";
+  std::string Snippet = "Smart x; x->Field;";
+  auto StmtMatch =
+      matchStmt(Snippet, memberExpr(hasObjectExpression(expr().bind(Id))));
+  ASSERT_TRUE(StmtMatch);
+  const Stencil Stencil = maybeAddressOf(Id);
+  EXPECT_THAT_EXPECTED(Stencil->eval(StmtMatch->Result), HasValue("x"));
+}
+
+TEST_F(StencilTest, MaybeAddressOfSmartPointerDerefNoCancel) {
+  StringRef Id = "id";
+  testExpr(Id, "Smart x; *x;", maybeAddressOf(Id), "&*x");
+}
+
 TEST_F(StencilTest, AccessOpValue) {
   StringRef Snippet = R"cc(
     S x;