[PATCH] D67969: [libTooling] Add `run` combinator to Stencils.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked an inline comment as done.
ymandel added inline comments.


================
Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:181
+/// This supports user-defined extensions to the Stencil language.
+StencilPart run(MatchConsumer<std::string> C);
+
----------------
gribozavr wrote:
> We could reimplement all other stencils through `run()` and eliminate `StencilPartInterface`. 
> 
> The idea is to make `StencilPart` contain a `std::shared_ptr<MatchConsumer<std::string>>`. Then `run()` can be implemented as creating a `StencilPart` directly, and everything else can be implemented in terms of `run()`.
> 
> WDYT?
Answering both questions together: the equality function is designed to make testing of the format-string parser feasible (or, at least, reasonable).  As is, the only reason not to do what you suggest above is exactly the equality function -- otherwise, StencilPart is just `std::shared_ptr<MatchConsumer<std::string>>`.

I can also plausibly imagine that we'll extend the interface to include a "print" function as well at some point.  

However, currently the parser hasn't been upstreamed and the print function is only an idea, so it would be reasonable to drop `StencilPartInterface` and all the associated baggage and we could reinstate it later as needed.  Or, if you can think of a good way to test the parser that doesn't require an equality function, that too would be convincing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67969