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

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 22 02:37:32 PST 2019


gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:95
 
+// Constructs an expression that idiomatically represents a value, taking into
+// account whether `ExprId` is a pointer or already a value.
----------------
Three slashes, please.


================
Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:96
+// 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);
----------------
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()`?


================
Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:99
+
+// Constructs an expression that idiomatically represents a pointer, taking into
+// account whether `ExprId` is a value or already a pointer.
----------------
Three slashes, please.


================
Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:101
+// account whether `ExprId` is a value or already a pointer.
+Stencil asPointer(llvm::StringRef ExprId);
+
----------------
Similarly, `maybeAddressOf()`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70554/new/

https://reviews.llvm.org/D70554





More information about the cfe-commits mailing list