r361413 - [LibTooling] Update Stencil to use RangeSelector
Yitzhak Mandelbaum via cfe-commits
cfe-commits at lists.llvm.org
Wed May 22 11:03:00 PDT 2019
Author: ymandel
Date: Wed May 22 11:03:00 2019
New Revision: 361413
URL: http://llvm.org/viewvc/llvm-project?rev=361413&view=rev
Log:
[LibTooling] Update Stencil to use RangeSelector
Add support for creating a `StencilPart` from any `RangeSelector`, which
broadens the scope of `Stencil`.
Correspondingly, deprecate Stencil's specialized combinators `node` and `sNode`
in favor of using the new `selection` combinator directly (with the appropriate
range selector).
Reviewers: sbenza
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D62160
Modified:
cfe/trunk/include/clang/Tooling/Refactoring/Stencil.h
cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp
cfe/trunk/unittests/Tooling/StencilTest.cpp
Modified: cfe/trunk/include/clang/Tooling/Refactoring/Stencil.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/Stencil.h?rev=361413&r1=361412&r2=361413&view=diff
==============================================================================
--- cfe/trunk/include/clang/Tooling/Refactoring/Stencil.h (original)
+++ cfe/trunk/include/clang/Tooling/Refactoring/Stencil.h Wed May 22 11:03:00 2019
@@ -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 <string>
@@ -121,6 +122,7 @@ public:
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<StencilPart> Parts;
@@ -142,14 +144,24 @@ template <typename... Ts> Stencil cat(Ts
/// \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.
///
Modified: cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp?rev=361413&r1=361412&r2=361413&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Stencil.cpp Wed May 22 11:03:00 2019
@@ -55,22 +55,11 @@ struct DebugPrintNodeOpData {
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 @@ bool isEqualData(const DebugPrintNodeOpD
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 @@ Error evalData(const DebugPrintNodeOpDat
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<Stmt>() != nullptr && Node.get<Expr>() == 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 @@ public:
namespace {
using RawText = StencilPartImpl<RawTextData>;
using DebugPrintNodeOp = StencilPartImpl<DebugPrintNodeOpData>;
-using NodeRef = StencilPartImpl<NodeRefData>;
+using SelectorOp = StencilPartImpl<SelectorOpData>;
} // 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 @@ StencilPart stencil::text(StringRef Text
return StencilPart(std::make_shared<RawText>(Text));
}
-StencilPart stencil::node(StringRef Id) {
- return StencilPart(std::make_shared<NodeRef>(Id, SemiAssociation::Inferred));
-}
-
-StencilPart stencil::sNode(StringRef Id) {
- return StencilPart(std::make_shared<NodeRef>(Id, SemiAssociation::Always));
+StencilPart stencil::selection(RangeSelector Selector) {
+ return StencilPart(std::make_shared<SelectorOp>(std::move(Selector)));
}
StencilPart stencil::dPrint(StringRef Id) {
Modified: cfe/trunk/unittests/Tooling/StencilTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/StencilTest.cpp?rev=361413&r1=361412&r2=361413&view=diff
==============================================================================
--- cfe/trunk/unittests/Tooling/StencilTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/StencilTest.cpp Wed May 22 11:03:00 2019
@@ -22,9 +22,9 @@ using ::testing::AllOf;
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 @@ TEST_F(StencilTest, SingleStatement) {
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 @@ TEST_F(StencilTest, SingleStatementCallO
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 @@ TEST_F(StencilTest, UnboundNode) {
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 @@ void testExpr(StringRef Id, StringRef Sn
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
More information about the cfe-commits
mailing list