diff --git a/clang/include/clang/Tooling/Refactoring/Stencil.h b/clang/include/clang/Tooling/Refactoring/Stencil.h --- a/clang/include/clang/Tooling/Refactoring/Stencil.h +++ b/clang/include/clang/Tooling/Refactoring/Stencil.h @@ -153,6 +153,28 @@ return selection(tooling::node(Id)); } +/// 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 +/// `e->m`, when e is a pointer, `e2->m` when e = `*e2` and `e.m` otherwise. +/// Additionally, `e` is wrapped in parentheses, if needed. +StencilPart access(llvm::StringRef BaseId, StencilPart Member); +inline StencilPart access(llvm::StringRef BaseId, llvm::StringRef Member) { + return access(BaseId, text(Member)); +} + +/// Chooses between the two stencil parts, based on whether \p ID is bound in +/// the match. +StencilPart ifBound(llvm::StringRef Id, StencilPart TruePart, + StencilPart FalsePart); + +/// Chooses between the two strings, based on whether \p ID is bound in the +/// match. +inline StencilPart ifBound(llvm::StringRef Id, llvm::StringRef TrueText, + llvm::StringRef FalseText) { + return ifBound(Id, text(TrueText), text(FalseText)); +} + /// For debug use only; semantics are not guaranteed. /// /// \returns the string resulting from calling the node's print() method. diff --git a/clang/lib/Tooling/Refactoring/Stencil.cpp b/clang/lib/Tooling/Refactoring/Stencil.cpp --- a/clang/lib/Tooling/Refactoring/Stencil.cpp +++ b/clang/lib/Tooling/Refactoring/Stencil.cpp @@ -14,6 +14,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Lex/Lexer.h" #include "clang/Tooling/Refactoring/SourceCode.h" +#include "clang/Tooling/Refactoring/SourceCodeBuilders.h" #include "llvm/Support/Errc.h" #include #include @@ -23,7 +24,9 @@ using namespace tooling; using ast_matchers::MatchFinder; +using llvm::errc; using llvm::Error; +using llvm::StringError; // A down_cast function to safely down cast a StencilPartInterface to a subclass // D. Returns nullptr if P is not an instance of D. @@ -51,28 +54,53 @@ }; // A debugging operation to dump the AST for a particular (bound) AST node. -struct DebugPrintNodeOpData { - explicit DebugPrintNodeOpData(std::string S) : Id(std::move(S)) {} +struct DebugPrintNodeData { + explicit DebugPrintNodeData(std::string S) : Id(std::move(S)) {} std::string Id; }; // The fragment of code corresponding to the selected range. -struct SelectorOpData { - explicit SelectorOpData(RangeSelector S) : Selector(std::move(S)) {} +struct SelectorData { + explicit SelectorData(RangeSelector S) : Selector(std::move(S)) {} RangeSelector Selector; }; + +// A stencil operation to build a member access `e.m` or `e->m`, as appropriate. +struct AccessData { + AccessData(StringRef BaseId, StencilPart Member) + : BaseId(BaseId), Member(std::move(Member)) {} + std::string BaseId; + StencilPart Member; +}; + +struct IfBoundData { + IfBoundData(StringRef Id, StencilPart TruePart, StencilPart FalsePart) + : Id(Id), TruePart(std::move(TruePart)), FalsePart(std::move(FalsePart)) { + } + std::string Id; + StencilPart TruePart; + StencilPart FalsePart; +}; } // namespace bool isEqualData(const RawTextData &A, const RawTextData &B) { return A.Text == B.Text; } -bool isEqualData(const DebugPrintNodeOpData &A, const DebugPrintNodeOpData &B) { +bool isEqualData(const DebugPrintNodeData &A, const DebugPrintNodeData &B) { return A.Id == B.Id; } // Equality is not (yet) defined for \c RangeSelector. -bool isEqualData(const SelectorOpData &, const SelectorOpData &) { return false; } +bool isEqualData(const SelectorData &, const SelectorData &) { return false; } + +bool isEqualData(const AccessData &A, const AccessData &B) { + return A.BaseId == B.BaseId && A.Member == B.Member; +} + +bool isEqualData(const IfBoundData &A, const IfBoundData &B) { + return A.Id == B.Id && A.TruePart == B.TruePart && A.FalsePart == B.FalsePart; +} // The `evalData()` overloads evaluate the given stencil data to a string, given // the match result, and append it to `Result`. We define an overload for each @@ -84,7 +112,7 @@ return Error::success(); } -Error evalData(const DebugPrintNodeOpData &Data, +Error evalData(const DebugPrintNodeData &Data, const MatchFinder::MatchResult &Match, std::string *Result) { std::string Output; llvm::raw_string_ostream Os(Output); @@ -96,7 +124,7 @@ return Error::success(); } -Error evalData(const SelectorOpData &Data, const MatchFinder::MatchResult &Match, +Error evalData(const SelectorData &Data, const MatchFinder::MatchResult &Match, std::string *Result) { auto Range = Data.Selector(Match); if (!Range) @@ -105,6 +133,32 @@ return Error::success(); } +Error evalData(const AccessData &Data, const MatchFinder::MatchResult &Match, + std::string *Result) { + const auto *E = Match.Nodes.getNodeAs(Data.BaseId); + if (E == nullptr) + return llvm::make_error(errc::invalid_argument, + "Id not bound: " + Data.BaseId); + if (!E->isImplicitCXXThis()) { + if (llvm::Optional S = E->getType()->isAnyPointerType() + ? buildArrow(*E, *Match.Context) + : buildDot(*E, *Match.Context)) + *Result += *S; + else + return llvm::make_error( + errc::invalid_argument, + "Could not construct object text from ID: " + Data.BaseId); + } + return Data.Member.eval(Match, Result); +} + +Error evalData(const IfBoundData &Data, const MatchFinder::MatchResult &Match, + std::string *Result) { + auto &M = Match.Nodes.getMap(); + return (M.find(Data.Id) != M.end() ? Data.TruePart : Data.FalsePart) + .eval(Match, Result); +} + template class StencilPartImpl : public StencilPartInterface { T Data; @@ -134,12 +188,6 @@ } }; -namespace { -using RawText = StencilPartImpl; -using DebugPrintNodeOp = StencilPartImpl; -using SelectorOp = StencilPartImpl; -} // namespace - StencilPart Stencil::wrap(StringRef Text) { return stencil::text(Text); } @@ -163,13 +211,25 @@ } StencilPart stencil::text(StringRef Text) { - return StencilPart(std::make_shared(Text)); + return StencilPart(std::make_shared>(Text)); } StencilPart stencil::selection(RangeSelector Selector) { - return StencilPart(std::make_shared(std::move(Selector))); + return StencilPart( + std::make_shared>(std::move(Selector))); } StencilPart stencil::dPrint(StringRef Id) { - return StencilPart(std::make_shared(Id)); + return StencilPart(std::make_shared>(Id)); +} + +StencilPart stencil::access(StringRef BaseId, StencilPart Member) { + return StencilPart( + std::make_shared>(BaseId, std::move(Member))); +} + +StencilPart stencil::ifBound(StringRef Id, StencilPart TruePart, + StencilPart FalsePart) { + return StencilPart(std::make_shared>( + Id, std::move(TruePart), std::move(FalsePart))); } diff --git a/clang/unittests/Tooling/StencilTest.cpp b/clang/unittests/Tooling/StencilTest.cpp --- a/clang/unittests/Tooling/StencilTest.cpp +++ b/clang/unittests/Tooling/StencilTest.cpp @@ -10,6 +10,8 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Tooling/FixIt.h" #include "clang/Tooling/Tooling.h" +#include "llvm/Support/Error.h" +#include "llvm/Testing/Support/Error.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -18,12 +20,15 @@ using namespace ast_matchers; namespace { +using ::llvm::HasValue; using ::testing::AllOf; using ::testing::Eq; using ::testing::HasSubstr; using MatchResult = MatchFinder::MatchResult; +using stencil::access; using stencil::cat; using stencil::dPrint; +using stencil::ifBound; using stencil::text; // In tests, we can't directly match on llvm::Expected since its accessors @@ -44,8 +49,10 @@ } // Create a valid translation-unit from a statement. -static std::string wrapSnippet(llvm::Twine StatementCode) { - return ("auto stencil_test_snippet = []{" + StatementCode + "};").str(); +static std::string wrapSnippet(StringRef StatementCode) { + return ("struct S { int field; }; auto stencil_test_snippet = []{" + + StatementCode + "};") + .str(); } static DeclarationMatcher wrapMatcher(const StatementMatcher &Matcher) { @@ -63,9 +70,10 @@ // Matches `Matcher` against the statement `StatementCode` and returns the // result. Handles putting the statement inside a function and modifying the -// matcher correspondingly. `Matcher` should match `StatementCode` exactly -- -// that is, produce exactly one match. -static llvm::Optional matchStmt(llvm::Twine StatementCode, +// 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`. +static llvm::Optional matchStmt(StringRef StatementCode, StatementMatcher Matcher) { auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode)); if (AstUnit == nullptr) { @@ -123,7 +131,7 @@ // Tests failures caused by references to unbound nodes. `unbound_id` is the // id that will cause the failure. - void testUnboundNodeError(const Stencil &Stencil, llvm::StringRef UnboundId) { + void testUnboundNodeError(const Stencil &Stencil, StringRef UnboundId) { testError(Stencil, AllOf(HasSubstr(UnboundId), HasSubstr("not bound"))); } }; @@ -188,8 +196,7 @@ StringRef Expected) { auto StmtMatch = matchStmt(Snippet, expr().bind(Id)); ASSERT_TRUE(StmtMatch); - EXPECT_THAT(toOptional(Stencil.eval(StmtMatch->Result)), - IsSomething(Expected)); + EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue(Expected)); } TEST_F(StencilTest, SelectionOp) { @@ -197,6 +204,102 @@ testExpr(Id, "3;", cat(node(Id)), "3"); } +TEST_F(StencilTest, IfBoundOpBound) { + StringRef Id = "id"; + testExpr(Id, "3;", cat(ifBound(Id, text("5"), text("7"))), "5"); +} + +TEST_F(StencilTest, IfBoundOpUnbound) { + StringRef Id = "id"; + testExpr(Id, "3;", cat(ifBound("other", text("5"), text("7"))), "7"); +} + +TEST_F(StencilTest, AccessOpValue) { + StringRef Snippet = R"cc( + S x; + x; + )cc"; + StringRef Id = "id"; + testExpr(Id, Snippet, cat(access(Id, "field")), "x.field"); +} + +TEST_F(StencilTest, AccessOpValueExplicitText) { + StringRef Snippet = R"cc( + S x; + x; + )cc"; + StringRef Id = "id"; + testExpr(Id, Snippet, cat(access(Id, text("field"))), "x.field"); +} + +TEST_F(StencilTest, AccessOpValueAddress) { + StringRef Snippet = R"cc( + S x; + &x; + )cc"; + StringRef Id = "id"; + testExpr(Id, Snippet, cat(access(Id, "field")), "x.field"); +} + +TEST_F(StencilTest, AccessOpPointer) { + StringRef Snippet = R"cc( + S *x; + x; + )cc"; + StringRef Id = "id"; + testExpr(Id, Snippet, cat(access(Id, "field")), "x->field"); +} + +TEST_F(StencilTest, AccessOpPointerDereference) { + StringRef Snippet = R"cc( + S *x; + *x; + )cc"; + StringRef Id = "id"; + testExpr(Id, Snippet, cat(access(Id, "field")), "x->field"); +} + +TEST_F(StencilTest, AccessOpExplicitThis) { + using clang::ast_matchers::hasObjectExpression; + using clang::ast_matchers::memberExpr; + + // Set up the code so we can bind to a use of this. + StringRef Snippet = R"cc( + class C { + public: + int x; + int foo() { return this->x; } + }; + )cc"; + auto StmtMatch = + matchStmt(Snippet, returnStmt(hasReturnValue(ignoringImplicit(memberExpr( + hasObjectExpression(expr().bind("obj"))))))); + ASSERT_TRUE(StmtMatch); + const Stencil Stencil = cat(access("obj", "field")); + EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), + HasValue("this->field")); +} + +TEST_F(StencilTest, AccessOpImplicitThis) { + using clang::ast_matchers::hasObjectExpression; + using clang::ast_matchers::memberExpr; + + // Set up the code so we can bind to a use of (implicit) this. + StringRef Snippet = R"cc( + class C { + public: + int x; + int foo() { return x; } + }; + )cc"; + auto StmtMatch = + matchStmt(Snippet, returnStmt(hasReturnValue(ignoringImplicit(memberExpr( + hasObjectExpression(expr().bind("obj"))))))); + ASSERT_TRUE(StmtMatch); + const Stencil Stencil = cat(access("obj", "field")); + EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue("field")); +} + TEST(StencilEqualityTest, Equality) { auto Lhs = cat("foo", dPrint("dprint_id")); auto Rhs = cat("foo", dPrint("dprint_id"));