Skip to content

Commit

Permalink
[LibTooling] Update Stencil to use RangeSelector
Browse files Browse the repository at this point in the history
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

llvm-svn: 361413
  • Loading branch information
ymand committed May 22, 2019
1 parent fb9b301 commit 1f46d52
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 67 deletions.
16 changes: 14 additions & 2 deletions clang/include/clang/Tooling/Refactoring/Stencil.h
Expand Up @@ -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>
Expand Down Expand Up @@ -121,6 +122,7 @@ class Stencil {
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;
Expand All @@ -142,14 +144,24 @@ template <typename... Ts> Stencil cat(Ts &&... Parts) {
/// \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.
///
Expand Down
61 changes: 18 additions & 43 deletions clang/lib/Tooling/Refactoring/Stencil.cpp
Expand Up @@ -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

Expand All @@ -82,9 +71,8 @@ bool isEqualData(const DebugPrintNodeOpData &A, const DebugPrintNodeOpData &B) {
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
Expand All @@ -108,25 +96,12 @@ Error evalData(const DebugPrintNodeOpData &Data,
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();
}

Expand Down Expand Up @@ -162,13 +137,17 @@ class StencilPartImpl : public StencilPartInterface {
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));
Expand All @@ -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) {
Expand Down
45 changes: 23 additions & 22 deletions clang/unittests/Tooling/StencilTest.cpp
Expand Up @@ -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.
Expand Down Expand Up @@ -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;")));
}
Expand All @@ -160,8 +160,8 @@ TEST_F(StencilTest, SingleStatementCallOperator) {
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;")));
}
Expand All @@ -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;
Expand All @@ -192,32 +192,33 @@ void testExpr(StringRef Id, StringRef Snippet, const Stencil &Stencil,
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

0 comments on commit 1f46d52

Please sign in to comment.