[libcxx-commits] [PATCH] D94452: [libc++] Support immovable return types in std::function.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

The theme of the answers to many of your questions is "because the standard library sucks on purpose." Both `is_convertible` and `declval` are unfit for purpose in //most// places where a metaprogrammer might think about using them.



================
Comment at: libcxx/include/__functional_base:343
+template <class _Ret>
+struct __invoke_void_return_wrapper<_Ret, true>
 {
----------------
ldionne wrote:
> I don't understand why this change is necessary. Can you explain?
This change is severable (if I sever some of the test cases with it); it's not directly related to NonCopyable.

It allows us to support `std::function<const void()>`, because `std::is_void_v<const void> == true` but `std::is_same_v<const void, void> == false`.


================
Comment at: libcxx/include/functional:2352
+            static const bool value = is_void<_Rp>::value ||
+                __is_core_convertible<typename __invoke_of<_Fp, _ArgTypes...>::type,
+                                      _Rp>::value;
----------------
ldionne wrote:
> Can you explain why we can't use `std::is_convertible`? Is it possible that it's simply a bug in how we implement the `__is_convertible` intrinsic?
`std::is_convertible_v<mutex, mutex>` is (intentionally) `false`. So we need a different type-trait or intrinsic to express the core-language idea of "//can be implicitly converted//" as defined in http://eel.is/c++draft/conv.general#3 . libc++ might already have such a type-trait; I admit I didn't look super hard before deciding to write this myself. My intent is that if libc++ ever needs this trait anywhere else, they can reuse `__is_core_convertible` — it's not intended to be `std::function`-specific.


================
Comment at: libcxx/include/type_traits:1673
+// to a function. Notice that __is_core_convertible<void,void> is false,
+// and __is_core_convertible<mutex,mutex> is true in C++17 and later.
+
----------------
ldionne wrote:
> `mutex`? Do you mean `any-non-moveable-type`? If so, please use that in the example as it's clearer.
Yes. Will do, but I wish there were a shorter way to spell it.


================
Comment at: libcxx/include/type_traits:1680
+struct __is_core_convertible<_Tp, _Up, decltype(
+    static_cast<void(*)(_Up)>(0) ( static_cast<_Tp(*)()>(0)() )
+)> : public true_type {};
----------------
ldionne wrote:
> Can't you use `declval` here?
No, because `declval<T>()` gives you a `T&&` instead of a `T`, and we need specifically a `T`.
I did initially consider adding a helper function `_Tp __core_declval<_Tp>()` that could be reused elsewhere, but then decided that that was pretty silly because:

    __core_declval<_Tp>()
    ((_Tp(*)())0)()  // same effect, shorter, no template-instantiation overhead



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94452