Index: cfe/trunk/include/clang/Tooling/Refactoring/Stencil.h =================================================================== --- cfe/trunk/include/clang/Tooling/Refactoring/Stencil.h +++ cfe/trunk/include/clang/Tooling/Refactoring/Stencil.h @@ -6,7 +6,7 @@ // //===----------------------------------------------------------------------===// /// -/// /file +/// \file /// This file defines the *Stencil* abstraction: a code-generating object, /// parameterized by named references to (bound) AST nodes. Given a match /// result, a stencil can be evaluated to a string of source code. @@ -154,6 +154,21 @@ return selection(tooling::node(Id)); } +/// Generates the source of the expression bound to \p Id, wrapping it in +/// parentheses if it may parse differently depending on context. For example, a +/// binary operation is always wrapped, while a variable reference is never +/// wrapped. +StencilPart expression(llvm::StringRef Id); + +/// Constructs an idiomatic dereferencing of the expression bound to \p ExprId. +/// \p ExprId is wrapped in parentheses, if needed. +StencilPart deref(llvm::StringRef ExprId); + +/// Constructs an expression that idiomatically takes the address of the +/// expression bound to \p ExprId. \p ExprId is wrapped in parentheses, if +/// needed. +StencilPart addressOf(llvm::StringRef ExprId); + /// Constructs a `MemberExpr` that accesses the named member (\p Member) of the /// object bound to \p BaseId. The access is constructed idiomatically: if \p /// BaseId is bound to `e` and \p Member identifies member `m`, then returns Index: cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp =================================================================== --- cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp +++ cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp @@ -24,6 +24,7 @@ using namespace tooling; using ast_matchers::MatchFinder; +using ast_type_traits::DynTypedNode; using llvm::errc; using llvm::Error; using llvm::Expected; @@ -37,7 +38,7 @@ return static_cast(P); } -static llvm::Expected +static llvm::Expected getNode(const ast_matchers::BoundNodes &Nodes, StringRef Id) { auto &NodesMap = Nodes.getMap(); auto It = NodesMap.find(Id); @@ -60,6 +61,21 @@ std::string Id; }; +// Operators that take a single node Id as an argument. +enum class UnaryNodeOperator { + Parens, + Deref, + Address, +}; + +// Generic container for stencil operations with a (single) node-id argument. +struct UnaryOperationData { + UnaryOperationData(UnaryNodeOperator Op, std::string Id) + : Op(Op), Id(std::move(Id)) {} + UnaryNodeOperator Op; + std::string Id; +}; + // The fragment of code corresponding to the selected range. struct SelectorData { explicit SelectorData(RangeSelector S) : Selector(std::move(S)) {} @@ -91,6 +107,10 @@ return A.Id == B.Id; } +bool isEqualData(const UnaryOperationData &A, const UnaryOperationData &B) { + return A.Op == B.Op && A.Id == B.Id; +} + // Equality is not (yet) defined for \c RangeSelector. bool isEqualData(const SelectorData &, const SelectorData &) { return false; } @@ -130,6 +150,32 @@ return Error::success(); } +Error evalData(const UnaryOperationData &Data, + const MatchFinder::MatchResult &Match, std::string *Result) { + const auto *E = Match.Nodes.getNodeAs(Data.Id); + if (E == nullptr) + return llvm::make_error( + errc::invalid_argument, "Id not bound or not Expr: " + Data.Id); + llvm::Optional Source; + switch (Data.Op) { + case UnaryNodeOperator::Parens: + Source = buildParens(*E, *Match.Context); + break; + case UnaryNodeOperator::Deref: + Source = buildDereference(*E, *Match.Context); + break; + case UnaryNodeOperator::Address: + Source = buildAddressOf(*E, *Match.Context); + break; + } + if (!Source) + return llvm::make_error( + errc::invalid_argument, + "Could not construct expression source from ID: " + Data.Id); + *Result += *Source; + return Error::success(); +} + Error evalData(const SelectorData &Data, const MatchFinder::MatchResult &Match, std::string *Result) { auto Range = Data.Selector(Match); @@ -239,6 +285,21 @@ return StencilPart(std::make_shared>(Id)); } +StencilPart stencil::expression(llvm::StringRef Id) { + return StencilPart(std::make_shared>( + UnaryNodeOperator::Parens, Id)); +} + +StencilPart stencil::deref(llvm::StringRef ExprId) { + return StencilPart(std::make_shared>( + UnaryNodeOperator::Deref, ExprId)); +} + +StencilPart stencil::addressOf(llvm::StringRef ExprId) { + return StencilPart(std::make_shared>( + UnaryNodeOperator::Address, ExprId)); +} + StencilPart stencil::access(StringRef BaseId, StencilPart Member) { return StencilPart( std::make_shared>(BaseId, std::move(Member))); Index: cfe/trunk/unittests/Tooling/StencilTest.cpp =================================================================== --- cfe/trunk/unittests/Tooling/StencilTest.cpp +++ cfe/trunk/unittests/Tooling/StencilTest.cpp @@ -20,14 +20,19 @@ using namespace ast_matchers; namespace { +using ::llvm::Failed; using ::llvm::HasValue; +using ::llvm::StringError; using ::testing::AllOf; using ::testing::Eq; using ::testing::HasSubstr; using MatchResult = MatchFinder::MatchResult; using stencil::access; +using stencil::addressOf; using stencil::cat; +using stencil::deref; using stencil::dPrint; +using stencil::expression; using stencil::ifBound; using stencil::run; using stencil::text; @@ -104,7 +109,7 @@ ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr; } else { auto Err = llvm::handleErrors(ResultOrErr.takeError(), - [&Matcher](const llvm::StringError &Err) { + [&Matcher](const StringError &Err) { EXPECT_THAT(Err.getMessage(), Matcher); }); if (Err) { @@ -183,6 +188,15 @@ EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue(Expected)); } +void testFailure(StringRef Id, StringRef Snippet, const Stencil &Stencil, + testing::Matcher MessageMatcher) { + auto StmtMatch = matchStmt(Snippet, expr().bind(Id)); + ASSERT_TRUE(StmtMatch); + EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), + Failed(testing::Property( + &StringError::getMessage, MessageMatcher))); +} + TEST_F(StencilTest, SelectionOp) { StringRef Id = "id"; testExpr(Id, "3;", cat(node(Id)), "3"); @@ -198,6 +212,56 @@ testExpr(Id, "3;", cat(ifBound("other", text("5"), text("7"))), "7"); } +TEST_F(StencilTest, ExpressionOpNoParens) { + StringRef Id = "id"; + testExpr(Id, "3;", cat(expression(Id)), "3"); +} + +// Don't parenthesize a parens expression. +TEST_F(StencilTest, ExpressionOpNoParensParens) { + StringRef Id = "id"; + testExpr(Id, "(3);", cat(expression(Id)), "(3)"); +} + +TEST_F(StencilTest, ExpressionOpBinaryOpParens) { + StringRef Id = "id"; + testExpr(Id, "3+4;", cat(expression(Id)), "(3+4)"); +} + +// `expression` shares code with other ops, so we get sufficient coverage of the +// error handling code with this test. If that changes in the future, more error +// tests should be added. +TEST_F(StencilTest, ExpressionOpUnbound) { + StringRef Id = "id"; + testFailure(Id, "3;", cat(expression("ACACA")), + AllOf(HasSubstr("ACACA"), HasSubstr("not bound"))); +} + +TEST_F(StencilTest, DerefPointer) { + StringRef Id = "id"; + testExpr(Id, "int *x; x;", cat(deref(Id)), "*x"); +} + +TEST_F(StencilTest, DerefBinOp) { + StringRef Id = "id"; + testExpr(Id, "int *x; x + 1;", cat(deref(Id)), "*(x + 1)"); +} + +TEST_F(StencilTest, DerefAddressExpr) { + StringRef Id = "id"; + testExpr(Id, "int x; &x;", cat(deref(Id)), "x"); +} + +TEST_F(StencilTest, AddressOfValue) { + StringRef Id = "id"; + testExpr(Id, "int x; x;", cat(addressOf(Id)), "&x"); +} + +TEST_F(StencilTest, AddressOfDerefExpr) { + StringRef Id = "id"; + testExpr(Id, "int *x; *x;", cat(addressOf(Id)), "x"); +} + TEST_F(StencilTest, AccessOpValue) { StringRef Snippet = R"cc( S x;