ymandel marked 12 inline comments as done. ymandel added a comment. In D69613#1730238 <https://reviews.llvm.org/D69613#1730238>, @ymandel wrote:
> In D69613#1729703 <https://reviews.llvm.org/D69613#1729703>, @gribozavr2 > wrote: > > > I see. Some decoupling is desirable, I agree. Maybe move `StencilInterface` > > and `Stencil` into a separate header that `RewriteRule` can depend on, and > > then define the library of stencils in another header? > > > This sounds good. I'll put together a (stacked) revision to do this. It seems > worth doing separately given that it will involve a substantial to the > RewriteRule library & tests. In order to keep the client unaffected by the change, I reinstated a simple Stencil class to wrap the shared_ptr and provide the call operator. This means that the code is now backwards compatible and forwards compatible (to the change we've discussed in RewriteRule to be stencil-aware). This was simpler than adding temporary(?) overloads in `RewriteRule.h` for every function with a `TextGenerator` argument. I included an overload of `operator->` so that we can collapse it back to just a type alias. ================ Comment at: clang/include/clang/Tooling/Transformer/Stencil.h:155 +template <typename... Ts> MatchConsumer<std::string> stencil(Ts &&... Parts) { + Stencil S = cat(std::forward<Ts>(Parts)...); + return [S](const ast_matchers::MatchFinder::MatchResult &R) { ---------------- gribozavr2 wrote: > I'm not sure the convenience of being able to avoid to say "cat(...)" at the > callsite is worth the API complexity. In other words, my suggestion is to > make this function take a single stencil. Based on discussion, removed this entirely. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69613/new/ https://reviews.llvm.org/D69613 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
