[libcxx-commits] [PATCH] D107199: [libc++] Refactor __perfect_forward, bind_front and not_fn

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver accepted this revision as: zoecarver.
zoecarver added a comment.

Nothing major. This looks good to me!



================
Comment at: libcxx/include/__functional/bind_front.h:42
+template<class _Fn, class... _Args, class = _EnableIf<
+    conjunction_v<
+        is_constructible<decay_t<_Fn>, _Fn>,
----------------
Nit: `_And`


================
Comment at: libcxx/include/__functional/perfect_forward.h:42
 
-    template<class... _Args>
-    _LIBCPP_INLINE_VISIBILITY constexpr auto operator()(_Args&&... __args) &
-    noexcept(noexcept(_Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...)))
-    -> decltype(      _Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...))
-    {return           _Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...);}
-
-    template<class... _Args>
-    _LIBCPP_INLINE_VISIBILITY constexpr auto operator()(_Args&&... __args) const&
-    noexcept(noexcept(_Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...)))
-    -> decltype(      _Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...))
-    {return           _Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...);}
-
-    template<class... _Args>
-    _LIBCPP_INLINE_VISIBILITY constexpr auto operator()(_Args&&... __args) &&
-    noexcept(noexcept(_Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...)))
-    -> decltype(      _Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...))
-    {return           _Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...);}
-
-    template<class... _Args>
-    _LIBCPP_INLINE_VISIBILITY constexpr auto operator()(_Args&&... __args) const&&
-    noexcept(noexcept(_Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...)))
-    -> decltype(      _Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...))
-    {return           _Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...);}
-
-    template<class _Fn = typename tuple_element<0, tuple<_Bound...>>::type,
-             class = _EnableIf<is_copy_constructible_v<_Fn>>>
-    constexpr __perfect_forward_impl(__perfect_forward_impl const& __other)
-        : __bound_(__other.__bound_) {}
-
-    template<class _Fn = typename tuple_element<0, tuple<_Bound...>>::type,
-             class = _EnableIf<is_move_constructible_v<_Fn>>>
-    constexpr __perfect_forward_impl(__perfect_forward_impl && __other)
-        : __bound_(_VSTD::move(__other.__bound_)) {}
-
-    template<class... _BoundArgs>
-    explicit constexpr __perfect_forward_impl(_BoundArgs&&... __bound) :
-        __bound_(_VSTD::forward<_BoundArgs>(__bound)...) { }
+    template<class _Tuple, class ..._Args, class = decltype(_Op::__call(_VSTD::get<_Idx>(declval<_Tuple>())..., declval<_Args>()...))>
+    static auto __can_call_impl(int) -> true_type;
----------------
This makes me wonder if we should replace `__call` with `operator()` and then we can just use `is_invocable` here. I guess we could really do that anyway, though. 


================
Comment at: libcxx/include/__functional/perfect_forward.h:42
 
-    template<class... _Args>
-    _LIBCPP_INLINE_VISIBILITY constexpr auto operator()(_Args&&... __args) &
-    noexcept(noexcept(_Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...)))
-    -> decltype(      _Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...))
-    {return           _Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...);}
-
-    template<class... _Args>
-    _LIBCPP_INLINE_VISIBILITY constexpr auto operator()(_Args&&... __args) const&
-    noexcept(noexcept(_Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...)))
-    -> decltype(      _Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...))
-    {return           _Op::__call(_VSTD::get<_Idxs>(__bound_)..., _VSTD::forward<_Args>(__args)...);}
-
-    template<class... _Args>
-    _LIBCPP_INLINE_VISIBILITY constexpr auto operator()(_Args&&... __args) &&
-    noexcept(noexcept(_Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...)))
-    -> decltype(      _Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...))
-    {return           _Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...);}
-
-    template<class... _Args>
-    _LIBCPP_INLINE_VISIBILITY constexpr auto operator()(_Args&&... __args) const&&
-    noexcept(noexcept(_Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...)))
-    -> decltype(      _Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...))
-    {return           _Op::__call(_VSTD::get<_Idxs>(_VSTD::move(__bound_))...,
-                                  _VSTD::forward<_Args>(__args)...);}
-
-    template<class _Fn = typename tuple_element<0, tuple<_Bound...>>::type,
-             class = _EnableIf<is_copy_constructible_v<_Fn>>>
-    constexpr __perfect_forward_impl(__perfect_forward_impl const& __other)
-        : __bound_(__other.__bound_) {}
-
-    template<class _Fn = typename tuple_element<0, tuple<_Bound...>>::type,
-             class = _EnableIf<is_move_constructible_v<_Fn>>>
-    constexpr __perfect_forward_impl(__perfect_forward_impl && __other)
-        : __bound_(_VSTD::move(__other.__bound_)) {}
-
-    template<class... _BoundArgs>
-    explicit constexpr __perfect_forward_impl(_BoundArgs&&... __bound) :
-        __bound_(_VSTD::forward<_BoundArgs>(__bound)...) { }
+    template<class _Tuple, class ..._Args, class = decltype(_Op::__call(_VSTD::get<_Idx>(declval<_Tuple>())..., declval<_Args>()...))>
+    static auto __can_call_impl(int) -> true_type;
----------------
zoecarver wrote:
> This makes me wonder if we should replace `__call` with `operator()` and then we can just use `is_invocable` here. I guess we could really do that anyway, though. 
Concepts would make a lot of this a lot easier :(


================
Comment at: libcxx/include/__functional/perfect_forward.h:82
+
+    template<class ..._Args, class = _EnableIf<__can_call<tuple<_Bound...>, _Args...>>>
+    _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Args&&... __args) &&
----------------
Shouldn't tuple be an xvalue here, not a prvalue? Or am I just confused?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107199