[libcxx-commits] [PATCH] D124695: [libc++] Avoid a Microsoft SAL macro.

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision.
Mordante added a comment.
This revision is now accepted and ready to land.

LGTM modulo some nits, please fix them before landing this patch.



================
Comment at: libcxx/include/__functional/perfect_forward.h:37
 public:
-    template <class ..._BoundArgs, class = enable_if_t<
-        is_constructible_v<tuple<_Bound...>, _BoundArgs&&...>
-    >>
-    explicit constexpr __perfect_forward_impl(_BoundArgs&& ...__bound)
-        : __bound_(_VSTD::forward<_BoundArgs>(__bound)...)
-    { }
-
-    __perfect_forward_impl(__perfect_forward_impl const&) = default;
-    __perfect_forward_impl(__perfect_forward_impl&&) = default;
-
-    __perfect_forward_impl& operator=(__perfect_forward_impl const&) = default;
-    __perfect_forward_impl& operator=(__perfect_forward_impl&&) = default;
-
-    template <class ..._Args, class = enable_if_t<is_invocable_v<_Op, _Bound&..., _Args...>>>
-    _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Args&&... __args) &
-        noexcept(noexcept(_Op()(_VSTD::get<_Idx>(__bound_)..., _VSTD::forward<_Args>(__args)...)))
-        -> decltype(      _Op()(_VSTD::get<_Idx>(__bound_)..., _VSTD::forward<_Args>(__args)...))
-        { return          _Op()(_VSTD::get<_Idx>(__bound_)..., _VSTD::forward<_Args>(__args)...); }
-
-    template <class ..._Args, class = enable_if_t<!is_invocable_v<_Op, _Bound&..., _Args...>>>
-    auto operator()(_Args&&...) & = delete;
-
-    template <class ..._Args, class = enable_if_t<is_invocable_v<_Op, _Bound const&..., _Args...>>>
-    _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Args&&... __args) const&
-        noexcept(noexcept(_Op()(_VSTD::get<_Idx>(__bound_)..., _VSTD::forward<_Args>(__args)...)))
-        -> decltype(      _Op()(_VSTD::get<_Idx>(__bound_)..., _VSTD::forward<_Args>(__args)...))
-        { return          _Op()(_VSTD::get<_Idx>(__bound_)..., _VSTD::forward<_Args>(__args)...); }
-
-    template <class ..._Args, class = enable_if_t<!is_invocable_v<_Op, _Bound const&..., _Args...>>>
-    auto operator()(_Args&&...) const& = delete;
-
-    template <class ..._Args, class = enable_if_t<is_invocable_v<_Op, _Bound..., _Args...>>>
-    _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Args&&... __args) &&
-        noexcept(noexcept(_Op()(_VSTD::get<_Idx>(_VSTD::move(__bound_))..., _VSTD::forward<_Args>(__args)...)))
-        -> decltype(      _Op()(_VSTD::get<_Idx>(_VSTD::move(__bound_))..., _VSTD::forward<_Args>(__args)...))
-        { return          _Op()(_VSTD::get<_Idx>(_VSTD::move(__bound_))..., _VSTD::forward<_Args>(__args)...); }
-
-    template <class ..._Args, class = enable_if_t<!is_invocable_v<_Op, _Bound..., _Args...>>>
-    auto operator()(_Args&&...) && = delete;
-
-    template <class ..._Args, class = enable_if_t<is_invocable_v<_Op, _Bound const..., _Args...>>>
-    _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Args&&... __args) const&&
-        noexcept(noexcept(_Op()(_VSTD::get<_Idx>(_VSTD::move(__bound_))..., _VSTD::forward<_Args>(__args)...)))
-        -> decltype(      _Op()(_VSTD::get<_Idx>(_VSTD::move(__bound_))..., _VSTD::forward<_Args>(__args)...))
-        { return          _Op()(_VSTD::get<_Idx>(_VSTD::move(__bound_))..., _VSTD::forward<_Args>(__args)...); }
-
-    template <class ..._Args, class = enable_if_t<!is_invocable_v<_Op, _Bound const..., _Args...>>>
-    auto operator()(_Args&&...) const&& = delete;
+  template <class... _Args, class = enable_if_t< is_constructible_v<tuple<_BoundArgs...>, _Args&&...> >>
+  explicit constexpr __perfect_forward_impl(_Args&&... __bound_args)
----------------
Minor nit.


================
Comment at: libcxx/include/__functional/perfect_forward.h:34
 private:
-    tuple<_Bound...> __bound_;
+  tuple<_Bound...> __bound__;
 
----------------
Mordante wrote:
> I'm not thrilled by the trailing double underscore, this deviates from our coding style.
> Maybe change `__bound_` to `__bound_args_` in this file, please also update the template name to `_BoundArgs`.
> 
> Please make similar changes at other places, having function arguments with a trailing underscore will lead to confusion. If you're having trouble finding good names, leave a comment to the names you need assistance with.
Next time when you address review comments can you mark the comment as done? That makes reviewing easier.


================
Comment at: libcxx/include/__iterator/advance.h:120
 
-  // Preconditions: Either `assignable_from<I&, S> || sized_sentinel_for<S, I>` is modeled, or [i, bound) denotes a range.
+  // Preconditions: Either `assignable_from<I&, S> || sized_sentinel_for<S, I>` is modeled, or [i, bound_) denotes a range.
   template <input_or_output_iterator _Ip, sentinel_for<_Ip> _Sp>
----------------
Same for the other comments.


================
Comment at: libcxx/include/__ranges/iota_view.h:381
+  namespace views {
+  namespace __iota {
   struct __fn {
----------------
We don't indent nested namespaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124695