[ADT] Add SFINAE guards to unique_function constructor.

[sammccall]

Summary

We can't construct a working unique_function from an object that's not callable
with the right types, so don't allow deduction to succeed.
This avoids some ambiguous conversion cases, e.g. allowing to overload
on different unique_function types, and to conversion operators to
unique_function.

std::function and the any_invocable proposal have these.
This was added to llvm::function_ref in D88901 and followups

[kadircet]

llvm/include/llvm/ADT/FunctionExtras.h

361

what about doing this in the constructor of the UniqueFunctionBase instead?

llvm/unittests/ADT/FunctionExtrasTest.cpp

267

s/function_ref/unique_function/

here and below

270

looks like having multiple tests with the same name is a failure on some buildbots :/

s/FunctionRefTest/UniqueFunctionTest/

[sammccall]

fix test

llvm/include/llvm/ADT/FunctionExtras.h

361

I don't think this works even in principle, only the signatures are taken into account when resolving overloads, and the base-initialization is not part of the signature.
https://godbolt.org/z/TWvcsE - see that the overload on the class with the sfinae constructor template works, but overloading on types derived from it is ambiguous.

Practically it also doesn't work because UniqueFunctionBase doesn't have access to R and P..., and you can't pass explicit template args to a constructor template.

(Or maybe I'm misunderstanding your suggestion)

llvm/unittests/ADT/FunctionExtrasTest.cpp

267

Oops... need to do a *little* more than copy/paste...

[kadircet]

thanks, lgtm!

llvm/include/llvm/ADT/FunctionExtras.h

361

no you understood correctly, i was missing the fact that base initialization required template instantiation. so nvm.

llvm/include/llvm/ADT/FunctionExtras.h

 template <typename FunctionT> class unique_function;
 
 namespace detail {
 
 template <typename T>
 using EnableIfTrivial =
     std::enable_if_t<llvm::is_trivially_move_constructible<T>::value &&
                      std::is_trivially_destructible<T>::value>;
+template <typename CallableT, typename ThisT>
+using EnableUnlessSameType = std::enable_if_t<!std::is_same<
+    std::remove_cv_t<std::remove_reference_t<CallableT>>, ThisT>::value>;
+template <typename CallableT, typename Ret, typename... Params>
+using EnableIfCallable =
+    std::enable_if_t<std::is_void<Ret>::value ||
+                     std::is_convertible<decltype(std::declval<CallableT>()(
+                                             std::declval<Params>()...)),
+                                         Ret>::value>;
 
 template <typename ReturnT, typename... ParamTs> class UniqueFunctionBase {
 protected:
   static constexpr size_t InlineStorageSize = sizeof(void *) * 3;
 
   template <typename T, class = void>
   struct IsSizeLessThanThresholdT : std::false_type {};
 
public:
   unique_function() = default;
   unique_function(std::nullptr_t) {}
   unique_function(unique_function &&) = default;
   unique_function(const unique_function &) = delete;
   unique_function &operator=(unique_function &&) = default;
   unique_function &operator=(const unique_function &) = delete;
 
   template <typename CallableT>
-  unique_function(CallableT Callable)
+  unique_function(
+      CallableT Callable,
+      detail::EnableUnlessSameType<CallableT, unique_function> * = nullptr,
+      detail::EnableIfCallable<CallableT, R, P...> * = nullptr)
       : Base(std::forward<CallableT>(Callable),
              typename Base::template CalledAs<CallableT>{}) {}
 
   R operator()(P... Params) {
     return this->getCallPtr()(this->getCalleePtr(), Params...);
   }
 };
 
 template <typename R, typename... P>
 class unique_function<R(P...) const>
     : public detail::UniqueFunctionBase<R, P...> {
   using Base = detail::UniqueFunctionBase<R, P...>;
 
 public:
   unique_function() = default;
   unique_function(std::nullptr_t) {}
   unique_function(unique_function &&) = default;
   unique_function(const unique_function &) = delete;
   unique_function &operator=(unique_function &&) = default;
   unique_function &operator=(const unique_function &) = delete;
 
   template <typename CallableT>
-  unique_function(CallableT Callable)
+  unique_function(
+      CallableT Callable,
+      detail::EnableUnlessSameType<CallableT, unique_function> * = nullptr,
+      detail::EnableIfCallable<const CallableT, R, P...> * = nullptr)
       : Base(std::forward<CallableT>(Callable),
              typename Base::template CalledAs<const CallableT>{}) {}
 
   R operator()(P... Params) const {
     return this->getCallPtr()(this->getCalleePtr(), Params...);
   }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_ADT_FUNCTIONEXTRAS_H

llvm/unittests/ADT/FunctionExtrasTest.cpp

TEST(UniqueFunctionTest, Const) {
   EXPECT_EQ("non-const", ChooseMutable());
   EXPECT_EQ("non-const", A());
   unique_function<StringRef() const> ChooseConst = ChooseCorrectOverload();
   const ChooseCorrectOverload &X = A;
   EXPECT_EQ("const", ChooseConst());
   EXPECT_EQ("const", X());
 }
 
+// Test that overloads on unique_functions are resolved as expected.
+std::string returns(StringRef) { return "not a function"; }
+std::string returns(unique_function<double()> F) { return "number"; }
+std::string returns(unique_function<StringRef()> F) { return "string"; }
+
+TEST(UniqueFunctionTest, SFINAE) {
+  EXPECT_EQ("not a function", returns("boo!"));
+  EXPECT_EQ("number", returns([] { return 42; }));
+  EXPECT_EQ("string", returns([] { return "hello"; }));
+}
+
 } // anonymous namespace