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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 6 12:26:06 PDT 2020


dblaikie 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,
----------------
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).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88901



More information about the llvm-commits mailing list