asoffer added inline comments.
================ Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:103-108 + template <typename ConsumerFn, + std::enable_if_t< + llvm::is_invocable< + ConsumerFn, + llvm::Expected<llvm::MutableArrayRef<AtomicChange>>>::value, + int> = 0> ---------------- Given that we're simply passing this off to the NoMetadataImpl which accepts a std::function, I don't see any reason to accept a generic parameter via sfinae. We could just accept the std::function and move it. Then the implicit conversion from whatever is passed in to std::function will do the heavy sfinae lifting. ================ Comment at: clang/include/clang/Tooling/Transformer/Transformer.h:116 + explicit Transformer(transformer::RewriteRuleWith<T> Rule, + ConsumerFn Consumer); ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120360/new/ https://reviews.llvm.org/D120360 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
