[libTooling] Add stencil combinators for nodes that may be pointers or values.

[ymandel]

Summary

Adds combinators maybeDeref and maybeAddressOf to provide a uniform way to handle
nodes which may be bound to either a pointer or a value (most often in the
context of member expressions). Such polymorphism is already supported by
access; these combinators extend it to more general uses.

[gribozavr2]

clang/include/clang/Tooling/Transformer/Stencil.h

95

Three slashes, please.

96

Need more explanation to cover the cases when the expression is a pointer to a pointer (do we deref once or twice?), or a smart pointer (do we take care of that at all?)

WDYT about calling it maybeDeref() or derefIfPointer()?

99

Three slashes, please.

101

Similarly, maybeAddressOf()?

[ymandel]

renamed combinators; reordered cases to pair x and maybeX.

[ymandel]

fix typo

[ymandel]

Thanks for the review!

clang/include/clang/Tooling/Transformer/Stencil.h

96

I went with maybe... Given that, I didn't elaborate on pointer-to-pointer case because it never claims to result in a value. WDYT?

also, I kind of like derefIfPointer better, but addressOfIfValue just seemed to clunky. So, went w/ maybe...

[gribozavr2]

clang/include/clang/Tooling/Transformer/Stencil.h

90

"If the expression \p ExprId has pointer type ..."

95

"If the expression \p ExprId is not a pointer, ..."

[gribozavr2]

clang/include/clang/Tooling/Transformer/Stencil.h

96

Given that, I didn't elaborate on pointer-to-pointer case because it never claims to result in a value. WDYT?

I think it is clear what happens with pointers to pointers given the current doc comment.

clang/include/clang/Tooling/Transformer/Stencil.h

 /// binary operation is always wrapped, while a variable reference is never
 /// wrapped.
 Stencil expression(llvm::StringRef Id);
 
 /// Constructs an idiomatic dereferencing of the expression bound to \p ExprId.
 /// \p ExprId is wrapped in parentheses, if needed.
 Stencil 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.
 Stencil addressOf(llvm::StringRef ExprId);
 
+// Constructs an expression that idiomatically represents a value, taking into
+// account whether `ExprId` is a pointer or already a value.
+Stencil asValue(llvm::StringRef ExprId);
+
+// Constructs an expression that idiomatically represents a pointer, taking into
+// account whether `ExprId` is a value or already a pointer.
+Stencil asPointer(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
 /// `e->m`, when e is a pointer, `e2->m` when e = `*e2` and `e.m` otherwise.
 /// Additionally, `e` is wrapped in parentheses, if needed.
 Stencil access(llvm::StringRef BaseId, Stencil Member);
 inline Stencil access(llvm::StringRef BaseId, llvm::StringRef Member) {
   return access(BaseId, text(Member));

clang/lib/Tooling/Transformer/Stencil.cpp

struct DebugPrintNodeData {
   std::string Id;
 };
 
 // Operators that take a single node Id as an argument.
 enum class UnaryNodeOperator {
   Parens,
   Deref,
   Address,
+  AsValue,
+  AsPointer,
 };
 
 // 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;
case UnaryNodeOperator::Parens:
     OpName = "expression";
     break;
   case UnaryNodeOperator::Deref:
     OpName = "deref";
     break;
   case UnaryNodeOperator::Address:
     OpName = "addressOf";
     break;
+  case UnaryNodeOperator::AsValue:
+    OpName = "asValue";
+    break;
+  case UnaryNodeOperator::AsPointer:
+    OpName = "asPointer";
+    break;
   }
   return (OpName + "(\"" + Data.Id + "\")").str();
 }
 
 std::string toStringData(const SelectorData &) { return "selection(...)"; }
 
 std::string toStringData(const AccessData &Data) {
   return (llvm::Twine("access(\"") + Data.BaseId + "\", " +
case UnaryNodeOperator::Parens:
     Source = tooling::buildParens(*E, *Match.Context);
     break;
   case UnaryNodeOperator::Deref:
     Source = tooling::buildDereference(*E, *Match.Context);
     break;
   case UnaryNodeOperator::Address:
     Source = tooling::buildAddressOf(*E, *Match.Context);
     break;
+  case UnaryNodeOperator::AsValue:
+    if (!E->getType()->isAnyPointerType()) {
+      *Result += tooling::getText(*E, *Match.Context);
+      return Error::success();
+    }
+    Source = tooling::buildDereference(*E, *Match.Context);
+    break;
+
+  case UnaryNodeOperator::AsPointer:
+    if (E->getType()->isAnyPointerType()) {
+      *Result += tooling::getText(*E, *Match.Context);
+      return Error::success();
+    }
+    Source = tooling::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();
 }
return std::make_shared<StencilImpl<UnaryOperationData>>(
       UnaryNodeOperator::Deref, ExprId);
 }
 
 Stencil transformer::addressOf(llvm::StringRef ExprId) {
   return std::make_shared<StencilImpl<UnaryOperationData>>(
       UnaryNodeOperator::Address, ExprId);
 }
 
+Stencil transformer::asValue(llvm::StringRef ExprId) {
+  return std::make_shared<StencilImpl<UnaryOperationData>>(
+      UnaryNodeOperator::AsValue, ExprId);
+}
+
+Stencil transformer::asPointer(llvm::StringRef ExprId) {
+  return std::make_shared<StencilImpl<UnaryOperationData>>(
+      UnaryNodeOperator::AsPointer, ExprId);
+}
+
 Stencil transformer::access(StringRef BaseId, Stencil Member) {
   return std::make_shared<StencilImpl<AccessData>>(BaseId, std::move(Member));
 }
 
 Stencil transformer::ifBound(StringRef Id, Stencil TrueStencil,
                              Stencil FalseStencil) {
   return std::make_shared<StencilImpl<IfBoundData>>(Id, std::move(TrueStencil),
                                                     std::move(FalseStencil));

clang/unittests/Tooling/StencilTest.cpp

TEST_F(StencilTest, AddressOfValue) {
   testExpr(Id, "int x; x;", addressOf(Id), "&x");
 }
 
 TEST_F(StencilTest, AddressOfDerefExpr) {
   StringRef Id = "id";
   testExpr(Id, "int *x; *x;", addressOf(Id), "x");
 }
 
+TEST_F(StencilTest, AsValueValue) {
+  StringRef Id = "id";
+  testExpr(Id, "int x; x;", asValue(Id), "x");
+}
+
+TEST_F(StencilTest, AsValuePointer) {
+  StringRef Id = "id";
+  testExpr(Id, "int *x; x;", asValue(Id), "*x");
+}
+
+TEST_F(StencilTest, AsValueBinOp) {
+  StringRef Id = "id";
+  testExpr(Id, "int *x; x + 1;", asValue(Id), "*(x + 1)");
+}
+
+TEST_F(StencilTest, AsValueAddressExpr) {
+  StringRef Id = "id";
+  testExpr(Id, "int x; &x;", asValue(Id), "x");
+}
+
+TEST_F(StencilTest, AsPointerPointer) {
+  StringRef Id = "id";
+  testExpr(Id, "int *x; x;", asPointer(Id), "x");
+}
+
+TEST_F(StencilTest, AsPointerValue) {
+  StringRef Id = "id";
+  testExpr(Id, "int x; x;", addressOf(Id), "&x");
+}
+
+TEST_F(StencilTest, AsPointerBinOp) {
+  StringRef Id = "id";
+  testExpr(Id, "int x; x + 1;", asPointer(Id), "&(x + 1)");
+}
+
+TEST_F(StencilTest, AsPointerDerefExpr) {
+  StringRef Id = "id";
+  testExpr(Id, "int *x; *x;", addressOf(Id), "x");
+}
+
 TEST_F(StencilTest, AccessOpValue) {
   StringRef Snippet = R"cc(
     S x;
     x;
   )cc";
   StringRef Id = "id";
   testExpr(Id, Snippet, access(Id, "field"), "x.field");
 }