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 @@ -23,6 +23,7 @@ #include "clang/AST/ASTContext.h" #include "clang/AST/ASTTypeTraits.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/Refactoring/RangeSelector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include @@ -121,6 +122,7 @@ private: friend bool operator==(const Stencil &A, const Stencil &B); static StencilPart wrap(llvm::StringRef Text); + static StencilPart wrap(RangeSelector Selector); static StencilPart wrap(StencilPart Part) { return Part; } std::vector Parts; @@ -142,14 +144,24 @@ /// \returns exactly the text provided. StencilPart text(llvm::StringRef Text); +/// \returns the source corresponding to the selected range. +StencilPart selection(RangeSelector Selector); + /// \returns the source corresponding to the identified node. -StencilPart node(llvm::StringRef Id); +/// FIXME: Deprecated. Write `selection(node(Id))` instead. +inline StencilPart node(llvm::StringRef Id) { + return selection(tooling::node(Id)); +} + /// Variant of \c node() that identifies the node as a statement, for purposes /// of deciding whether to include any trailing semicolon. Only relevant for /// Expr nodes, which, by default, are *not* considered as statements. /// \returns the source corresponding to the identified node, considered as a /// statement. -StencilPart sNode(llvm::StringRef Id); +/// FIXME: Deprecated. Write `selection(statement(Id))` instead. +inline StencilPart sNode(llvm::StringRef Id) { + return selection(tooling::statement(Id)); +} /// For debug use only; semantics are not guaranteed. /// Index: cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp =================================================================== --- cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp +++ cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp @@ -55,22 +55,11 @@ explicit DebugPrintNodeOpData(std::string S) : Id(std::move(S)) {} std::string Id; }; -// Whether to associate a trailing semicolon with a node when identifying it's -// text. This flag is needed for expressions (clang::Expr), because their role -// is ambiguous when they are also complete statements. When this flag is -// `Always`, an expression node will be treated like a statement, and will -// therefore be associated with any trailing semicolon. -enum class SemiAssociation : bool { - Always, - Inferred, -}; -// A reference to a particular (bound) AST node. -struct NodeRefData { - explicit NodeRefData(std::string S, SemiAssociation SA) - : Id(std::move(S)), SemiAssoc(SA) {} - std::string Id; - SemiAssociation SemiAssoc; +// The fragment of code corresponding to the selected range. +struct SelectorOpData { + explicit SelectorOpData(RangeSelector S) : Selector(std::move(S)) {} + RangeSelector Selector; }; } // namespace @@ -82,9 +71,8 @@ return A.Id == B.Id; } -bool isEqualData(const NodeRefData &A, const NodeRefData &B) { - return A.Id == B.Id && A.SemiAssoc == B.SemiAssoc; -} +// Equality is not (yet) defined for \c RangeSelector. +bool isEqualData(const SelectorOpData &, const SelectorOpData &) { return false; } // 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 @@ -108,25 +96,12 @@ return Error::success(); } -Error evalData(const NodeRefData &Data, const MatchFinder::MatchResult &Match, +Error evalData(const SelectorOpData &Data, const MatchFinder::MatchResult &Match, std::string *Result) { - auto NodeOrErr = getNode(Match.Nodes, Data.Id); - if (auto Err = NodeOrErr.takeError()) - return Err; - auto &Node = *NodeOrErr; - switch (Data.SemiAssoc) { - case SemiAssociation::Inferred: - // Include the semicolon for non-expression statements: - *Result += Node.get() != nullptr && Node.get() == nullptr - ? getExtendedText(NodeOrErr.get(), tok::TokenKind::semi, - *Match.Context) - : getText(NodeOrErr.get(), *Match.Context); - break; - case SemiAssociation::Always: - *Result += - getExtendedText(NodeOrErr.get(), tok::TokenKind::semi, *Match.Context); - break; - } + auto Range = Data.Selector(Match); + if (!Range) + return Range.takeError(); + *Result += getText(*Range, *Match.Context); return Error::success(); } @@ -162,13 +137,17 @@ namespace { using RawText = StencilPartImpl; using DebugPrintNodeOp = StencilPartImpl; -using NodeRef = StencilPartImpl; +using SelectorOp = StencilPartImpl; } // namespace StencilPart Stencil::wrap(StringRef Text) { return stencil::text(Text); } +StencilPart Stencil::wrap(RangeSelector Selector) { + return stencil::selection(std::move(Selector)); +} + void Stencil::append(Stencil OtherStencil) { for (auto &Part : OtherStencil.Parts) Parts.push_back(std::move(Part)); @@ -187,12 +166,8 @@ return StencilPart(std::make_shared(Text)); } -StencilPart stencil::node(StringRef Id) { - return StencilPart(std::make_shared(Id, SemiAssociation::Inferred)); -} - -StencilPart stencil::sNode(StringRef Id) { - return StencilPart(std::make_shared(Id, SemiAssociation::Always)); +StencilPart stencil::selection(RangeSelector Selector) { + return StencilPart(std::make_shared(std::move(Selector))); } StencilPart stencil::dPrint(StringRef Id) { Index: cfe/trunk/unittests/Tooling/StencilTest.cpp =================================================================== --- cfe/trunk/unittests/Tooling/StencilTest.cpp +++ cfe/trunk/unittests/Tooling/StencilTest.cpp @@ -22,9 +22,9 @@ using ::testing::Eq; using ::testing::HasSubstr; using MatchResult = MatchFinder::MatchResult; -using tooling::stencil::node; -using tooling::stencil::sNode; -using tooling::stencil::text; +using stencil::cat; +using stencil::dPrint; +using stencil::text; // In tests, we can't directly match on llvm::Expected since its accessors // mutate the object. So, we collapse it to an Optional. @@ -141,8 +141,8 @@ hasThen(stmt().bind(Then)), hasElse(stmt().bind(Else)))); ASSERT_TRUE(StmtMatch); // Invert the if-then-else. - auto Stencil = Stencil::cat("if (!", node(Condition), ") ", sNode(Else), - " else ", sNode(Then)); + auto Stencil = cat("if (!", node(Condition), ") ", statement(Else), " else ", + statement(Then)); EXPECT_THAT(toOptional(Stencil.eval(StmtMatch->Result)), IsSomething(Eq("if (!true) return 0; else return 1;"))); } @@ -160,8 +160,8 @@ hasThen(stmt().bind(Then)), hasElse(stmt().bind(Else)))); ASSERT_TRUE(StmtMatch); // Invert the if-then-else. - Stencil S = Stencil::cat("if (!", node(Condition), ") ", sNode(Else), - " else ", sNode(Then)); + Stencil S = cat("if (!", node(Condition), ") ", statement(Else), " else ", + statement(Then)); EXPECT_THAT(toOptional(S(StmtMatch->Result)), IsSomething(Eq("if (!true) return 0; else return 1;"))); } @@ -176,7 +176,7 @@ auto StmtMatch = matchStmt(Snippet, ifStmt(hasCondition(stmt().bind("a1")), hasThen(stmt().bind("a2")))); ASSERT_TRUE(StmtMatch); - auto Stencil = Stencil::cat("if(!", sNode("a1"), ") ", node("UNBOUND"), ";"); + auto Stencil = cat("if(!", node("a1"), ") ", node("UNBOUND"), ";"); auto ResultOrErr = Stencil.eval(StmtMatch->Result); EXPECT_TRUE(llvm::errorToBool(ResultOrErr.takeError())) << "Expected unbound node, got " << *ResultOrErr; @@ -192,32 +192,33 @@ IsSomething(Expected)); } -TEST_F(StencilTest, NodeOp) { +TEST_F(StencilTest, SelectionOp) { StringRef Id = "id"; - testExpr(Id, "3;", Stencil::cat(node(Id)), "3"); -} - -TEST_F(StencilTest, SNodeOp) { - StringRef Id = "id"; - testExpr(Id, "3;", Stencil::cat(sNode(Id)), "3;"); + testExpr(Id, "3;", cat(node(Id)), "3"); } TEST(StencilEqualityTest, Equality) { - using stencil::dPrint; - auto Lhs = Stencil::cat("foo", node("node"), dPrint("dprint_id")); - auto Rhs = Lhs; + auto Lhs = cat("foo", dPrint("dprint_id")); + auto Rhs = cat("foo", dPrint("dprint_id")); EXPECT_EQ(Lhs, Rhs); } TEST(StencilEqualityTest, InEqualityDifferentOrdering) { - auto Lhs = Stencil::cat("foo", node("node")); - auto Rhs = Stencil::cat(node("node"), "foo"); + auto Lhs = cat("foo", dPrint("node")); + auto Rhs = cat(dPrint("node"), "foo"); EXPECT_NE(Lhs, Rhs); } TEST(StencilEqualityTest, InEqualityDifferentSizes) { - auto Lhs = Stencil::cat("foo", node("node"), "bar", "baz"); - auto Rhs = Stencil::cat("foo", node("node"), "bar"); + auto Lhs = cat("foo", dPrint("node"), "bar", "baz"); + auto Rhs = cat("foo", dPrint("node"), "bar"); EXPECT_NE(Lhs, Rhs); } + +// node is opaque and therefore cannot be examined for equality. +TEST(StencilEqualityTest, InEqualitySelection) { + auto S1 = cat(node("node")); + auto S2 = cat(node("node")); + EXPECT_NE(S1, S2); +} } // namespace