[PATCH] D69184: [libTooling] Introduce general combinator for fixed-value `MatchConsumer`s.

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 22 19:57:03 PDT 2019


ymandel added a comment.

In D69184#1715256 <https://reviews.llvm.org/D69184#1715256>, @gribozavr wrote:

> What are the use cases for non-text values?
>
> I like the name `text` much better... If the name conflict is the issue, I think you could rename it to `toText` -- it would even read more fluently `change(toText("abc"))`, also making it obvious that we are not changing *the text abc*, but we are changing the code to say "abc".


Another example use is: https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Tooling/Transformer/RangeSelector.h#L29-L32, although I must admit that I hadn't thought of that before this patch.  It's just the natural generalization (it's actually the K combinator form SKI fame).  So, I haven't thought about other examples -- the primary motivation was just the naming.

That said, I'm not sure about `toText` because I consider the "to" as implicit in `change`. In fact, it might be a good idea to rename `change` to `changeTo` (WDYT?).  What about any of `asText`, `textMsg`, `textMessage`? (I realize the unfortunate pun in the last two, but not sure that's worth caring about).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69184





More information about the cfe-commits mailing list