[clang] 675a297 - [libTooling] Add support for smart pointers to relevant Transformer `Stencil`s.

Yitzhak Mandelbaum via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 5 09:57:52 PST 2021


Author: Yitzhak Mandelbaum
Date: 2021-01-05T17:57:41Z
New Revision: 675a2973ee7745d1859e3b72be40a803dd349e55

URL: https://github.com/llvm/llvm-project/commit/675a2973ee7745d1859e3b72be40a803dd349e55
DIFF: https://github.com/llvm/llvm-project/commit/675a2973ee7745d1859e3b72be40a803dd349e55.diff

LOG: [libTooling] Add support for smart pointers to relevant Transformer `Stencil`s.

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Tooling/Transformer/Stencil.h b/clang/include/clang/Tooling/Transformer/Stencil.h
index b729c826c808..1b7495eb0262 100644
--- a/clang/include/clang/Tooling/Transformer/Stencil.h
+++ b/clang/include/clang/Tooling/Transformer/Stencil.h
@@ -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

diff  --git a/clang/lib/Tooling/Transformer/Stencil.cpp b/clang/lib/Tooling/Transformer/Stencil.cpp
index 56f145393691..d46087e4b04b 100644
--- a/clang/lib/Tooling/Transformer/Stencil.cpp
+++ b/clang/lib/Tooling/Transformer/Stencil.cpp
@@ -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();
     }

diff  --git a/clang/unittests/Tooling/StencilTest.cpp b/clang/unittests/Tooling/StencilTest.cpp
index b270e9d2f572..233331933901 100644
--- a/clang/unittests/Tooling/StencilTest.cpp
+++ b/clang/unittests/Tooling/StencilTest.cpp
@@ -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;


        


More information about the cfe-commits mailing list