[PATCH] D120360: [libTooling] Generalize string explanation as templated metadata

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel accepted this revision.
ymandel added inline comments.


================
Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:116
+  explicit Transformer(transformer::RewriteRuleWith<T> Rule,
+                       ConsumerFn Consumer);
 
----------------
asoffer wrote:
> li.zhe.hua wrote:
> > ymandel wrote:
> > > why won't we have the same SFINAE here to ensure ConsumerFn is invocable?
> > I can't figure out how to implement the SFINAE without instantiating `Result<void>` (which is invalid because `T Metadata` is illegal then). My current attempt is
> > 
> > ```
> >   template <
> >       typename T, typename ConsumerFn,
> >       std::enable_if_t<
> >           std::conjunction<
> >               std::negation<std::is_same<T, void>>,
> >               llvm::is_invocable<ConsumerFn, llvm::Expected<Result<T>>>>::value,
> >           int> = 0>
> >   explicit Transformer(transformer::RewriteRuleWith<T> Rule,
> >                        ConsumerFn Consumer);
> > ```
> > 
> > I suppose I could provide a specialization of `Result<void>` that is valid? I guess I'll go with that, and just document why it exists?
> Accepting the std:function directly should simplify this and avoid the need for the Result<void> specialization.
Seems fine, but I'll admit to being out of my depth. Seemed nice to have, but not necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360