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

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua 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>
----------------
asoffer wrote:
> 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.
So, I ran into this during [[ https://reviews.llvm.org/D119745 | D119745 ]], where one of the Windows CI builds doesn't have the fix for [[ http://cplusplus.github.io/LWG/lwg-defects.html#2132 | DR 2132 ]] backported, so overloading on different `std::function` doesn't work. (To be clear, this using `std::function` and doing the whole overloading thing works fine in clang locally.)

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360