ymandel updated this revision to Diff 200747.
ymandel added a comment.
Add back `node` and `sNode`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62160/new/
https://reviews.llvm.org/D62160
Files:
clang/include/clang/Tooling/Refactoring/Stencil.h
clang/lib/Tooling/Refactoring/Stencil.cpp
clang/unittests/Tooling/StencilTest.cpp
Index: clang/unittests/Tooling/StencilTest.cpp
===================================================================
--- clang/unittests/Tooling/StencilTest.cpp
+++ 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
Index: clang/lib/Tooling/Refactoring/Stencil.cpp
===================================================================
--- clang/lib/Tooling/Refactoring/Stencil.cpp
+++ 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<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 @@
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 @@
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) {
Index: clang/include/clang/Tooling/Refactoring/Stencil.h
===================================================================
--- clang/include/clang/Tooling/Refactoring/Stencil.h
+++ 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 <string>
@@ -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<StencilPart> 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(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(statement(Id));
+}
/// For debug use only; semantics are not guaranteed.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
