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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
