From 675a2973ee7745d1859e3b72be40a803dd349e55 Mon Sep 17 00:00:00 2001 From: Yitzhak Mandelbaum Date: Mon, 21 Dec 2020 14:49:17 +0000 Subject: [PATCH] [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 --- clang/include/clang/Tooling/Transformer/Stencil.h | 2 - clang/lib/Tooling/Transformer/Stencil.cpp | 50 ++++++++++++-- clang/unittests/Tooling/StencilTest.cpp | 81 +++++++++++++++++++++-- 3 files changed, 118 insertions(+), 15 deletions(-) diff --git a/clang/include/clang/Tooling/Transformer/Stencil.h b/clang/include/clang/Tooling/Transformer/Stencil.h index b729c82..1b7495e 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 56f1453..d46087e 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(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(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 b270e9d..2333319 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 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 class unique_ptr {}; }\n"; + StringRef Snippet = R"cc( + std::unique_ptr 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; -- 2.7.4