[libcxx-commits] [PATCH] D109212: [libc++][NFCI] span: replace enable_if with concepts

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/span:209
+template <class _Sentinel, class _It>
+concept __span_compatible_sentinel = sized_sentinel_for<_Sentinel, _It> && !is_convertible_v<_Sentinel, size_t>;
+
----------------
Could you rename this to `__span_compatible_sentinel_for` to keep the naming scheme of `sized_sentinel_for` and friends?


================
Comment at: libcxx/include/span:454-455
 
-    template <class _OtherElementType, size_t _Sz,
-              enable_if_t<is_convertible_v<_OtherElementType(*)[], element_type (*)[]>, nullptr_t> = nullptr>
+    template <class _OtherElementType, size_t _Sz>
+        requires __span_array_convertible<_OtherElementType, element_type>
     _LIBCPP_INLINE_VISIBILITY
----------------
Wouldn't that also work?


================
Comment at: libcxx/include/span:479-480
 
     template <class _OtherElementType, size_t _OtherExtent>
+        requires __span_array_convertible<_OtherElementType, element_type>
     _LIBCPP_INLINE_VISIBILITY
----------------
Same here.


================
Comment at: libcxx/include/span:608
 auto as_writable_bytes(span<_Tp, _Extent> __s) noexcept
--> enable_if_t<!is_const_v<_Tp>, decltype(__s.__as_writable_bytes())>
+-> decltype(__s.__as_writable_bytes())
 { return __s.__as_writable_bytes(); }
----------------
Is there a reason we don't just use `decltype(auto)` as the return type and drop the trailing return type completely?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109212