ymandel updated this revision to Diff 222889.
ymandel marked 2 inline comments as done.
ymandel added a comment.
responded to comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68315/new/
https://reviews.llvm.org/D68315
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
@@ -20,14 +20,19 @@
using namespace ast_matchers;
namespace {
+using ::llvm::Failed;
using ::llvm::HasValue;
+using ::llvm::StringError;
using ::testing::AllOf;
using ::testing::Eq;
using ::testing::HasSubstr;
using MatchResult = MatchFinder::MatchResult;
using stencil::access;
+using stencil::addressOf;
using stencil::cat;
+using stencil::deref;
using stencil::dPrint;
+using stencil::expression;
using stencil::ifBound;
using stencil::run;
using stencil::text;
@@ -104,7 +109,7 @@
ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
} else {
auto Err = llvm::handleErrors(ResultOrErr.takeError(),
- [&Matcher](const llvm::StringError &Err) {
+ [&Matcher](const StringError &Err) {
EXPECT_THAT(Err.getMessage(), Matcher);
});
if (Err) {
@@ -183,6 +188,15 @@
EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result), HasValue(Expected));
}
+void testFailure(StringRef Id, StringRef Snippet, const Stencil &Stencil,
+ testing::Matcher<std::string> MessageMatcher) {
+ auto StmtMatch = matchStmt(Snippet, expr().bind(Id));
+ ASSERT_TRUE(StmtMatch);
+ EXPECT_THAT_EXPECTED(Stencil.eval(StmtMatch->Result),
+ Failed<StringError>(testing::Property(
+ &StringError::getMessage, MessageMatcher)));
+}
+
TEST_F(StencilTest, SelectionOp) {
StringRef Id = "id";
testExpr(Id, "3;", cat(node(Id)), "3");
@@ -198,6 +212,56 @@
testExpr(Id, "3;", cat(ifBound("other", text("5"), text("7"))), "7");
}
+TEST_F(StencilTest, ExpressionOpNoParens) {
+ StringRef Id = "id";
+ testExpr(Id, "3;", cat(expression(Id)), "3");
+}
+
+// Don't parenthesize a parens expression.
+TEST_F(StencilTest, ExpressionOpNoParensParens) {
+ StringRef Id = "id";
+ testExpr(Id, "(3);", cat(expression(Id)), "(3)");
+}
+
+TEST_F(StencilTest, ExpressionOpBinaryOpParens) {
+ StringRef Id = "id";
+ testExpr(Id, "3+4;", cat(expression(Id)), "(3+4)");
+}
+
+// `expression` shares code with other ops, so we get sufficient coverage of the
+// error handling code with this test. If that changes in the future, more error
+// tests should be added.
+TEST_F(StencilTest, ExpressionOpUnbound) {
+ StringRef Id = "id";
+ testFailure(Id, "3;", cat(expression("ACACA")),
+ AllOf(HasSubstr("ACACA"), HasSubstr("not bound")));
+}
+
+TEST_F(StencilTest, DerefPointer) {
+ StringRef Id = "id";
+ testExpr(Id, "int *x; x;", cat(deref(Id)), "*x");
+}
+
+TEST_F(StencilTest, DerefBinOp) {
+ StringRef Id = "id";
+ testExpr(Id, "int *x; x + 1;", cat(deref(Id)), "*(x + 1)");
+}
+
+TEST_F(StencilTest, DerefAddressExpr) {
+ StringRef Id = "id";
+ testExpr(Id, "int x; &x;", cat(deref(Id)), "x");
+}
+
+TEST_F(StencilTest, AddressOfValue) {
+ StringRef Id = "id";
+ testExpr(Id, "int x; x;", cat(addressOf(Id)), "&x");
+}
+
+TEST_F(StencilTest, AddressOfDerefExpr) {
+ StringRef Id = "id";
+ testExpr(Id, "int *x; *x;", cat(addressOf(Id)), "x");
+}
+
TEST_F(StencilTest, AccessOpValue) {
StringRef Snippet = R"cc(
S x;
Index: clang/lib/Tooling/Refactoring/Stencil.cpp
===================================================================
--- clang/lib/Tooling/Refactoring/Stencil.cpp
+++ clang/lib/Tooling/Refactoring/Stencil.cpp
@@ -24,6 +24,7 @@
using namespace tooling;
using ast_matchers::MatchFinder;
+using ast_type_traits::DynTypedNode;
using llvm::errc;
using llvm::Error;
using llvm::Expected;
@@ -37,7 +38,7 @@
return static_cast<const D *>(P);
}
-static llvm::Expected<ast_type_traits::DynTypedNode>
+static llvm::Expected<DynTypedNode>
getNode(const ast_matchers::BoundNodes &Nodes, StringRef Id) {
auto &NodesMap = Nodes.getMap();
auto It = NodesMap.find(Id);
@@ -60,6 +61,21 @@
std::string Id;
};
+// Operators that take a single node Id as an argument.
+enum class UnaryNodeOperator {
+ Parens,
+ Deref,
+ Address,
+};
+
+// Generic container for stencil operations with a (single) node-id argument.
+struct UnaryOperationData {
+ UnaryOperationData(UnaryNodeOperator Op, std::string Id)
+ : Op(Op), Id(std::move(Id)) {}
+ UnaryNodeOperator Op;
+ std::string Id;
+};
+
// The fragment of code corresponding to the selected range.
struct SelectorData {
explicit SelectorData(RangeSelector S) : Selector(std::move(S)) {}
@@ -92,6 +108,10 @@
return A.Id == B.Id;
}
+bool isEqualData(const UnaryOperationData &A, const UnaryOperationData &B) {
+ return A.Op == B.Op && A.Id == B.Id;
+}
+
// Equality is not (yet) defined for \c RangeSelector.
bool isEqualData(const SelectorData &, const SelectorData &) { return false; }
@@ -131,6 +151,32 @@
return Error::success();
}
+Error evalData(const UnaryOperationData &Data,
+ const MatchFinder::MatchResult &Match, std::string *Result) {
+ const auto *E = Match.Nodes.getNodeAs<Expr>(Data.Id);
+ if (E == nullptr)
+ return llvm::make_error<StringError>(
+ errc::invalid_argument, "Id not bound or not Expr: " + Data.Id);
+ llvm::Optional<std::string> Source;
+ switch (Data.Op) {
+ case UnaryNodeOperator::Parens:
+ Source = buildParens(*E, *Match.Context);
+ break;
+ case UnaryNodeOperator::Deref:
+ Source = buildDereference(*E, *Match.Context);
+ break;
+ case UnaryNodeOperator::Address:
+ Source = buildAddressOf(*E, *Match.Context);
+ break;
+ }
+ if (!Source)
+ return llvm::make_error<StringError>(
+ errc::invalid_argument,
+ "Could not construct expression source from ID: " + Data.Id);
+ *Result += *Source;
+ return Error::success();
+}
+
Error evalData(const SelectorData &Data, const MatchFinder::MatchResult &Match,
std::string *Result) {
auto Range = Data.Selector(Match);
@@ -239,6 +285,21 @@
return StencilPart(std::make_shared<StencilPartImpl<DebugPrintNodeData>>(Id));
}
+StencilPart stencil::expression(llvm::StringRef Id) {
+ return StencilPart(std::make_shared<StencilPartImpl<UnaryOperationData>>(
+ UnaryNodeOperator::Parens, Id));
+}
+
+StencilPart stencil::deref(llvm::StringRef ExprId) {
+ return StencilPart(std::make_shared<StencilPartImpl<UnaryOperationData>>(
+ UnaryNodeOperator::Deref, ExprId));
+}
+
+StencilPart stencil::addressOf(llvm::StringRef ExprId) {
+ return StencilPart(std::make_shared<StencilPartImpl<UnaryOperationData>>(
+ UnaryNodeOperator::Address, ExprId));
+}
+
StencilPart stencil::access(StringRef BaseId, StencilPart Member) {
return StencilPart(
std::make_shared<StencilPartImpl<AccessData>>(BaseId, std::move(Member)));
Index: clang/include/clang/Tooling/Refactoring/Stencil.h
===================================================================
--- clang/include/clang/Tooling/Refactoring/Stencil.h
+++ clang/include/clang/Tooling/Refactoring/Stencil.h
@@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
///
-/// /file
+/// \file
/// This file defines the *Stencil* abstraction: a code-generating object,
/// parameterized by named references to (bound) AST nodes. Given a match
/// result, a stencil can be evaluated to a string of source code.
@@ -154,6 +154,21 @@
return selection(tooling::node(Id));
}
+/// Generates the source of the expression bound to \p Id, wrapping it in
+/// parentheses if it may parse differently depending on context. For example, a
+/// binary operation is always wrapped, while a variable reference is never
+/// wrapped.
+StencilPart expression(llvm::StringRef Id);
+
+/// Constructs an idiomatic dereferencing of the expression bound to \p ExprId.
+/// \p ExprId is wrapped in parentheses, if needed.
+StencilPart deref(llvm::StringRef ExprId);
+
+/// Constructs an expression that idiomatically takes the address of the
+/// expression bound to \p ExprId. \p ExprId is wrapped in parentheses, if
+/// needed.
+StencilPart addressOf(llvm::StringRef ExprId);
+
/// Constructs a `MemberExpr` that accesses the named member (\p Member) of the
/// object bound to \p BaseId. The access is constructed idiomatically: if \p
/// BaseId is bound to `e` and \p Member identifies member `m`, then returns
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
