[PATCH] D68315: [libTooling] Add various Stencil combinators for expressions.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.


================
Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:149
 /// \returns the source corresponding to the selected range.
 StencilPart selection(RangeSelector Selector);
 
----------------
ymandel wrote:
> gribozavr wrote:
> > Should the comment cross-reference expression() and say that the user probably wants that instead?
> That depends on what selector they're using. For `selection(node(ExprId))`, yes I think that `expression(ExprId)` is going to be better in most cases. But, for other selectors, no.  So, I'm not sure that the cross-reference will be generally useful.  WDYT?
> 
> Also, it occurs to me that we have an asymmetry for statements and expressions. Getting the source of a statement is
> `selection(statement(Id))` versus `expression(Id)` for expressions. However, in the context of `cat`, which takes `RangeSelector` directly, they look the same, because `selection` isn't needed.
Yeah, you're right -- there are different cases, and `selection()` is not just for expressions. Maybe just point out that digging out raw source code should be considered a fallback option, and other AST-aware and context-aware stencils should be preferred where they exist.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68315