[PATCH] D88901: [ADT] function_ref's constructor is unavailable if the argument is not callable.

[Sam McCall via Phabricator via llvm-commits]

sammccall added inline comments.


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:193-206
+  template <typename Callable,
+            // Only allow this constructor if the object is actually callable
+            // and returns the correct type.
+            typename = typename std::enable_if<
+                std::is_void<Ret>::value ||
+                std::is_convertible<
+                    typename std::result_of<Callable(Params...)>::type,
----------------
dblaikie wrote:
> kadircet wrote:
> > dblaikie wrote:
> > > This now has two different SFINAE approaches (one in a defaulted template type parameter, one in a defaulted function/ctor parameter) - do they need to be different? Could they be merged in some way to at least use the same/common infrastructure?
> > i believe this is to prevent creation of function_ref from function_ref, so it is different than what the template type parameter is doing. but I suppose they can be merged.
> Yep, I understand that there are two different things that need to be tested - but if they don't need to be tested in different ways, it'd be good if they were merged to reduce confusion when reading the code (like seeing two variables next to each other with different naming conventions - raises questions about whether the differences are important/intentional/significant in some way the reader doesn't understand, etc).
Good point, I merged them to use the existing default-param technique.

Because the compatible-types condition is complex, I extracted it to a variable template.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88901