[libcxx-commits] [PATCH] D102468: [libcxx][ranges] makes `basic_string_view` and `span` satisfy concept view

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/span:25-27
+template<class ElementType, size_t Extent>
+  inline constexpr bool ranges::enable_view<span<ElementType, Extent>> = Extent == 0 ||
+    Extent == dynamic_extent;
----------------
Please linebreak for readability:
```
template<class ElementType, size_t Extent>
  inline constexpr bool ranges::enable_view<span<ElementType, Extent>> =
    (Extent == 0 || Extent == dynamic_extent);
```


================
Comment at: libcxx/include/span:536-537
 #if !defined(_LIBCPP_HAS_NO_RANGES)
+template <class _ElementType, size_t _Extent>
+inline constexpr bool ranges::enable_view<span<_ElementType, _Extent>> = _Extent == 0 || _Extent == dynamic_extent;
+
----------------
Ditto here:
```
template<class _Tp, size_t _Extent>
inline constexpr bool ranges::enable_view<span<_Tp, _Extent>> =
  (_Extent == 0 || _Extent == dynamic_extent);
```


================
Comment at: libcxx/include/string_view:188-189
 #include <__config>
 #include <__ranges/enable_borrowed_range.h>
+#include <__ranges/view.h>
 #include <__string>
----------------
I would like to see libc++ do the same thing consistently with `enable_borrowed_range.h` and `enable_view.h`. I admit, the places that want to specialize `enable_view` might not save very much (if anything) by avoiding `<concepts>`, but I think it's worth it to take a consistent approach to these "enable/disable" traits.

This might not be a blocker, since I can fiddle with organization after the fact.

...However, do I read correctly that you are moving `ranges::view` //outside of// `<__ranges/view.h>`? If that was intentional, then probably you should just rename `<__ranges/view.h>` to `<__ranges/enable_view.h>`, and that'll fix both complaints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102468