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

Eric Li via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 3 13:26:01 PST 2022


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>
----------------
li.zhe.hua wrote:
> 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?
Nevermind! I just tried and it works. I suspect this is because the non-template constructor and the template constructor are not ambiguous, as the non-template is tried first, and if it doesn't match, it goes to the template.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120360



More information about the cfe-commits mailing list