[libcxx-commits] [PATCH] D58639: First part of P1227R2 - Signed ssize() functions, unsigned size() functions

[Louis Dionne via Phabricator via libcxx-commits]

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

LGTM, please consider my comments and commit without requiring another round of review. Thanks for doing this!



================
Comment at: libcxx/include/span:22
 // [views.span], class template span
-template <class ElementType, ptrdiff_t Extent = dynamic_extent>
+template <class ElementType, size_t Extent = dynamic_extent>
     class span;
----------------
Did nobody discuss the fact that this is an ABI (and API) break? I know it's not an ABI break as far as the Standard is concerned (because the Standard hasn't shipped yet), but it is an ABI break for our implementation, no?

Granted, folks that this will break are building with `-std=c++2a`, but I want to confirm we're doing this on purpose and we acknowledge what this change means.


================
Comment at: libcxx/include/span:201
     using pointer                = _Tp *;
-    using const_pointer          = const _Tp *;
+    using const_pointer          = const _Tp *; // not in standard
     using reference              = _Tp &;
----------------
Would it be worth adding a reference to [LWG3144](http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-active.html#3144)? This way someone can remember to remove this once the DR is applied.


================
Comment at: libcxx/test/std/containers/views/types.pass.cpp:66
 
-template <typename S, typename ElementType, std::ptrdiff_t Size>
+template <typename S, typename ElementType, size_t Size>
 void testSpan()
----------------
`std::size_t` (only for consistency, obviously)


================
Comment at: libcxx/test/std/containers/views/types.pass.cpp:71
     ASSERT_SAME_TYPE(typename S::value_type,      std::remove_cv_t<ElementType>);
-    ASSERT_SAME_TYPE(typename S::index_type,      std::ptrdiff_t);
+    ASSERT_SAME_TYPE(typename S::index_type,      size_t);
     ASSERT_SAME_TYPE(typename S::difference_type, std::ptrdiff_t);
----------------
`std::size_t`


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

https://reviews.llvm.org/D58639