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

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
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
----------------
philnik wrote:
> Wouldn't that also work?
Yes, it works, but loses the symmetry with the next function template below (constrained by `__span_array_convertible<const _OtherElementType, element_type>`). I originally had it as you suggested. I'll make that change and we can let others decide which they prefer; I have a slight preference towards what you suggested because it's terser.


================
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(); }
----------------
philnik wrote:
> Is there a reason we don't just use `decltype(auto)` as the return type and drop the trailing return type completely?
No reason. Just switched to `decltype(auto)` since we have existing use of that in `variant`, and some of ranges, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109212