[PATCH] D69613: [libTooling] Simplify type structure of `Stencil`s.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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