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 @@ -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,8 @@ /// \returns exactly the text provided. StencilPart text(llvm::StringRef Text); -/// \returns the source corresponding to the identified node. -StencilPart node(llvm::StringRef 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); +/// \returns the source corresponding to the selected range. +StencilPart selection(RangeSelector Selector); /// For debug use only; semantics are not guaranteed. /// 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 @@ -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) { 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 @@ -22,8 +22,6 @@ using ::testing::Eq; using ::testing::HasSubstr; using MatchResult = MatchFinder::MatchResult; -using tooling::stencil::node; -using tooling::stencil::sNode; using tooling::stencil::text; // In tests, we can't directly match on llvm::Expected since its accessors @@ -141,8 +139,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 = 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 +158,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 = Stencil::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 +174,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 = 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 +190,36 @@ 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;"); -} - TEST(StencilEqualityTest, Equality) { using stencil::dPrint; - auto Lhs = Stencil::cat("foo", node("node"), dPrint("dprint_id")); + auto Lhs = Stencil::cat("foo", dPrint("dprint_id")); auto Rhs = Lhs; EXPECT_EQ(Lhs, Rhs); } TEST(StencilEqualityTest, InEqualityDifferentOrdering) { - auto Lhs = Stencil::cat("foo", node("node")); - auto Rhs = Stencil::cat(node("node"), "foo"); + using stencil::dPrint; + auto Lhs = Stencil::cat("foo", dPrint("node")); + auto Rhs = Stencil::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"); + using stencil::dPrint; + auto Lhs = Stencil::cat("foo", dPrint("node"), "bar", "baz"); + auto Rhs = Stencil::cat("foo", dPrint("node"), "bar"); EXPECT_NE(Lhs, Rhs); } + +// node is opaque and therefore cannot be examined for equality. +TEST(StencilEqualityTest, InEqualitySelection) { + using stencil::dPrint; + auto S = Stencil::cat(node("node")); + EXPECT_NE(S, S); +} } // namespace