[libcxx-commits] [PATCH] D110503: [libc++] Implement P1394r4 for span: range constructor

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/include/span:455-461
     template <class _OtherElementType, size_t _OtherExtent>
     _LIBCPP_INLINE_VISIBILITY
         constexpr span(const span<_OtherElementType, _OtherExtent>& __other,
                        enable_if_t<
                           is_convertible_v<_OtherElementType(*)[], element_type (*)[]>,
                           nullptr_t> = nullptr) noexcept
         : __data{__other.data()}, __size{__other.size()} {}
----------------
Quuxplusone wrote:
> Scope-creep: It would be nice to refactor this SFINAE, for consistency, as
> ```
> template <class _OtherElementType, size_t _OtherExtent,
>           enable_if_t<is_convertible_v<_OtherElementType(*)[], element_type(*)[]>>* = nullptr>
> _LIBCPP_INLINE_VISIBILITY
> constexpr span(const span<_OtherElementType, _OtherExtent>& __other) noexcept
>         : __data(__other.data()), __size(__other.size()) {}
> ```
> and so on throughout this file. But I could submit a separate (subsequent) PR for this myself. (Heck, I could even underscore-suffix the data members `__data` and `__size`!)
I'd vote let's just do that consistently throughout this file in a follow-up PR if that's OK with you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110503