Make Vec and Slice iterators be ExactSizeIterator

Author: danakj

subspace/containers/__private/array_iter.h

@@ -27,7 +27,7 @@
 namespace sus::containers {
 
 template <class T, size_t N>
-  requires(N <= PTRDIFF_MAX)
+  requires(N <= size_t{PTRDIFF_MAX})
 class Array;
 
 template <::sus::mem::Move ItemT, size_t N>

subspace/containers/__private/slice_iter.h

@@ -24,6 +24,7 @@
 #include "subspace/mem/nonnull.h"
 #include "subspace/mem/relocate.h"
 #include "subspace/mem/replace.h"
+#include "subspace/num/signed_integer.h"
 #include "subspace/num/unsigned_integer.h"
 
 namespace sus::containers {
@@ -52,34 +53,46 @@ struct [[sus_trivial_abi]] SliceIter final
   Option<Item> next() noexcept final {
     if (ptr_ == end_) [[unlikely]]
       return Option<Item>::none();
-    // SAFETY: Since end_ > ptr_, which is checked in the constructor, ptr_ + 1
-    // will never be null.
+    // SAFETY: end_ is always > ptr_ when we get here (this was checked by the
+    // constructor) so ptr_ will be inside the allocation, not pointing just
+    // after it (like end_ may be).
     return Option<Item>::some(*::sus::mem::replace_ptr(mref(ptr_), ptr_ + 1u));
   }
 
   // sus::iter::DoubleEndedIterator trait.
   Option<Item> next_back() noexcept {
     if (ptr_ == end_) [[unlikely]]
       return Option<Item>::none();
-    // SAFETY: Since end_ > ptr_, which is checked in the constructor, end_ - 1
-    // will never be null.
+    // SAFETY: end_ is always > ptr_ when we get here (this was checked by the
+    // constructor) so subtracting one and dereffing will be inside the
+    // allocation.
     end_ -= 1u;
     return Option<Item>::some(*end_);
   }
 
-  ::sus::iter::SizeHint size_hint() noexcept final {
-    // SAFETY: end_ is always larger than ptr_ which is only incremented until
-    // end_, so this static cast does not drop a negative sign bit. That ptr_
-    // starts at or before end_ is checked in the constructor.
-    const usize remaining = static_cast<size_t>(end_ - ptr_);
+  ::sus::iter::SizeHint size_hint() const noexcept final {
+    // SAFETY: The constructor checks that end_ - ptr_ is positive and Slice can
+    // not exceed isize::MAX.
+    // TODO: Use from_unchecked()
+    const auto remaining = ::sus::num::usize(static_cast<size_t>(end_ - ptr_));
     return ::sus::iter::SizeHint(
         remaining, ::sus::Option<::sus::num::usize>::some(remaining));
   }
 
+  /// sus::iter::ExactSizeIterator trait.
+  ::sus::num::usize exact_size_hint() const noexcept {
+    // SAFETY: The constructor checks that end_ - ptr_ is positive and Slice can
+    // not exceed isize::MAX.
+    // TODO: Use from_unchecked()
+    return ::sus::num::usize::from(static_cast<size_t>(end_ - ptr_));
+  }
+
  private:
   constexpr SliceIter(const RawItem* start, usize len) noexcept
-      : ptr_(start), end_(start + len.primitive_value) {
-    check(end_ >= ptr_ || !end_);  // end_ may wrap around to 0, but not past 0.
+      : ptr_(start), end_(start + size_t{len}) {
+    // Wrap-around would be an invalid allocation and would break our distance
+    // functions.
+    check(end_ >= ptr_);
   }
 
   const RawItem* ptr_;
@@ -111,8 +124,9 @@ struct [[sus_trivial_abi]] SliceIterMut final
   Option<Item> next() noexcept final {
     if (ptr_ == end_) [[unlikely]]
       return Option<Item>::none();
-    // SAFETY: Since end_ > ptr_, which is checked in the constructor, ptr_ + 1
-    // will never be null.
+    // SAFETY: end_ is always > ptr_ when we get here (this was checked by the
+    // constructor) so ptr_ will be inside the allocation, not pointing just
+    // after it (like end_ may be).
     return Option<Item>::some(
         mref(*::sus::mem::replace_ptr(mref(ptr_), ptr_ + 1u)));
   }
@@ -121,25 +135,36 @@ struct [[sus_trivial_abi]] SliceIterMut final
   Option<Item> next_back() noexcept {
     if (ptr_ == end_) [[unlikely]]
       return Option<Item>::none();
-    // SAFETY: Since end_ > ptr_, which is checked in the constructor, end_ - 1
-    // will never be null.
+    // SAFETY: end_ is always > ptr_ when we get here (this was checked by the
+    // constructor) so subtracting one and dereffing will be inside the
+    // allocation.
     end_ -= 1u;
     return Option<Item>::some(mref(*end_));
   }
 
-  ::sus::iter::SizeHint size_hint() noexcept final {
-    // SAFETY: end_ is always larger than ptr_ which is only incremented until
-    // end_, so this static cast does not drop a negative sign bit. That ptr_
-    // starts at or before end_ is checked in the constructor.
-    const usize remaining = static_cast<size_t>(end_ - ptr_);
+  ::sus::iter::SizeHint size_hint() const noexcept final {
+    // SAFETY: The constructor checks that end_ - ptr_ is positive and Slice can
+    // not exceed isize::MAX.
+    // TODO: Use from_unchecked()
+    const auto remaining = ::sus::num::usize(static_cast<size_t>(end_ - ptr_));
     return ::sus::iter::SizeHint(
         remaining, ::sus::Option<::sus::num::usize>::some(remaining));
   }
 
+  /// sus::iter::ExactSizeIterator trait.
+  ::sus::num::usize exact_size_hint() const noexcept {
+    // SAFETY: The constructor checks that end_ - ptr_ is positive and Slice can
+    // not exceed isize::MAX.
+    // TODO: Use from_unchecked()
+    return ::sus::num::usize::from(static_cast<size_t>(end_ - ptr_));
+  }
+
  private:
   constexpr SliceIterMut(RawItem* start, usize len) noexcept
-      : ptr_(start), end_(start + len.primitive_value) {
-    check(end_ >= ptr_ || !end_);  // end_ may wrap around to 0, but not past 0.
+      : ptr_(start), end_(start + size_t{len}) {
+    // Wrap-around would be an invalid allocation and would break our distance
+    // functions.
+    check(end_ >= ptr_);
   }
 
   RawItem* ptr_;

subspace/containers/__private/vec_iter.h

@@ -35,11 +35,12 @@ struct VecIntoIter final
  public:
   using Item = ItemT;
 
+  /// Constructs `VecIntoIter` from a `Vec`.
   static constexpr auto with(Vec<Item>&& vec) noexcept {
     return VecIntoIter(::sus::move(vec));
   }
 
-  // sus::iter::Iterator trait.
+  /// sus::iter::Iterator trait.
   Option<Item> next() noexcept final {
     if (front_index_ == back_index_) [[unlikely]]
       return Option<Item>::none();
@@ -52,7 +53,7 @@ struct VecIntoIter final
     return Option<Item>::some(move(item));
   }
 
-  // sus::iter::DoubleEndedIterator trait.
+  /// sus::iter::DoubleEndedIterator trait.
   Option<Item> next_back() noexcept {
     if (front_index_ == back_index_) [[unlikely]]
       return Option<Item>::none();
@@ -64,18 +65,21 @@ struct VecIntoIter final
     return Option<Item>::some(move(item));
   }
 
-  ::sus::iter::SizeHint size_hint() noexcept final {
+  /// sus::iter::Iterator method.
+  ::sus::iter::SizeHint size_hint() const noexcept final {
     const usize remaining = back_index_ - front_index_;
     return ::sus::iter::SizeHint(
         remaining, ::sus::Option<::sus::num::usize>::some(remaining));
   }
 
+  /// sus::iter::ExactSizeIterator trait.
+  ::sus::num::usize exact_size_hint() const noexcept {
+    return back_index_ - front_index_;
+  }
+
  private:
   VecIntoIter(Vec<Item>&& vec) noexcept : vec_(::sus::move(vec)) {}
 
-  // TODO: Decompose the Vec (with a Vec::into_raw_parts) into a (pointer, len,
-  // cap) and just store the pointer here, as the front_index_ and back_index_
-  // are all we need to retain wrt the Vec's size.
   Vec<Item> vec_;
   usize front_index_ = 0_usize;
   usize back_index_ = vec_.len();

subspace/containers/array.h

@@ -57,10 +57,10 @@ struct Storage<T, 0> final {};
 
 /// A container of objects of type T, with a fixed size N.
 ///
-/// An Array can not be larger than PTRDIFF_MAX, as subtracting a pointer at a
+/// An Array can not be larger than `isize::MAX`, as subtracting a pointer at a
 /// greater distance results in Undefined Behaviour.
 template <class T, size_t N>
-  requires(N <= PTRDIFF_MAX)
+  requires(N <= size_t{PTRDIFF_MAX})
 class Array final {
   static_assert(!std::is_const_v<T>,
                 "`Array<const T, N>` should be written `const Array<T, N>`, as "

subspace/containers/slice.h

@@ -51,13 +51,13 @@ class Slice {
   static constexpr inline Slice from_raw_parts(::sus::marker::UnsafeFnMarker,
                                                T* data,
                                                ::sus::usize len) noexcept {
-    ::sus::check(len.primitive_value <= PTRDIFF_MAX);
+    ::sus::check(len.primitive_value <= static_cast<size_t>(isize::MAX_PRIMITIVE));
     return Slice(data, len);
   }
 
   // sus::construct::From<Slice<T>, T[]> trait.
   template <size_t N>
-    requires(N <= PTRDIFF_MAX)
+    requires(N <= size_t{PTRDIFF_MAX})
   static constexpr inline Slice from(T (&data)[N]) {
     return Slice(data, N);
   }

subspace/containers/slice_unittest.cc

@@ -300,6 +300,7 @@ TEST(Slice, DoubleEndedIterator) {
     auto slice = Slice<const usize>::from(ar);
 
     auto it = slice.iter();
+    static_assert(sus::iter::DoubleEndedIterator<decltype(it), const usize&>);
     EXPECT_EQ(it.next_back(), sus::some(3_usize).construct());
     EXPECT_EQ(it.next_back(), sus::some(2_usize).construct());
     EXPECT_EQ(it.next_back(), sus::some(1_usize).construct());
@@ -310,6 +311,47 @@ TEST(Slice, DoubleEndedIterator) {
     auto slice = Slice<usize>::from(ar);
 
     auto it = slice.iter_mut();
+    static_assert(sus::iter::DoubleEndedIterator<decltype(it), usize&>);
+    EXPECT_EQ(it.next_back(), sus::some(3_usize).construct());
+    EXPECT_EQ(it.next_back(), sus::some(2_usize).construct());
+    EXPECT_EQ(it.next_back(), sus::some(1_usize).construct());
+    EXPECT_EQ(it.next_back(), sus::None);
+  }
+}
+
+TEST(Slice, ExactSizeIterator) {
+  {
+    const usize ar[] = {1u, 2u, 3u};
+    auto slice = Slice<const usize>::from(ar);
+
+    auto it = slice.iter();
+    static_assert(sus::iter::ExactSizeIterator<decltype(it), const usize&>);
+    EXPECT_EQ(it.size_hint().lower, 3u);
+    EXPECT_EQ(it.size_hint().upper, sus::Option<usize>::some(3u));
+    EXPECT_EQ(it.exact_size_hint(), 3u);
+    EXPECT_EQ(it.next_back(), sus::some(3_usize).construct());
+    EXPECT_EQ(it.size_hint().lower, 2u);
+    EXPECT_EQ(it.size_hint().upper, sus::Option<usize>::some(2u));
+    EXPECT_EQ(it.exact_size_hint(), 2u);
+    EXPECT_EQ(it.next_back(), sus::some(2_usize).construct());
+    EXPECT_EQ(it.size_hint().lower, 1u);
+    EXPECT_EQ(it.size_hint().upper, sus::Option<usize>::some(1u));
+    EXPECT_EQ(it.exact_size_hint(), 1u);
+    EXPECT_EQ(it.next_back(), sus::some(1_usize).construct());
+    EXPECT_EQ(it.size_hint().lower, 0u);
+    EXPECT_EQ(it.size_hint().upper, sus::Option<usize>::some(0u));
+    EXPECT_EQ(it.exact_size_hint(), 0u);
+    EXPECT_EQ(it.next_back(), sus::None);
+    EXPECT_EQ(it.size_hint().lower, 0u);
+    EXPECT_EQ(it.size_hint().upper, sus::Option<usize>::some(0u));
+    EXPECT_EQ(it.exact_size_hint(), 0u);
+  }
+  {
+    usize ar[] = {1u, 2u, 3u};
+    auto slice = Slice<usize>::from(ar);
+
+    auto it = slice.iter_mut();
+    static_assert(sus::iter::ExactSizeIterator<decltype(it), usize&>);
     EXPECT_EQ(it.next_back(), sus::some(3_usize).construct());
     EXPECT_EQ(it.next_back(), sus::some(2_usize).construct());
     EXPECT_EQ(it.next_back(), sus::some(1_usize).construct());

subspace/containers/vec_unittest.cc

@@ -323,6 +323,7 @@ TEST(Vec, IntoIterDoubleEnded) {
   v.push(3_i32);
 
   auto it = sus::move(v).into_iter();
+  static_assert(sus::iter::DoubleEndedIterator<decltype(it), i32>);
   EXPECT_EQ(it.next_back(), sus::some(3_i32).construct());
   EXPECT_EQ(it.next_back(), sus::some(2_i32).construct());
   EXPECT_EQ(it.next_back(), sus::some(1_i32).construct());
@@ -515,9 +516,54 @@ TEST(Vec, SizeHint) {
   v.push(1_i32);
   v.push(2_i32);
   v.push(3_i32);
-  auto [lower, upper] = sus::move(v).into_iter().size_hint();
-  EXPECT_EQ(lower, 3_usize);
-  EXPECT_EQ(upper, sus::Option<usize>::some(3_usize));
+  auto it = sus::move(v).into_iter();
+  {
+    auto [lower, upper] = it.size_hint();
+    EXPECT_EQ(lower, 3_usize);
+    EXPECT_EQ(upper, sus::Option<usize>::some(3_usize));
+  }
+  EXPECT_EQ(it.next(), sus::Some);
+  {
+    auto [lower, upper] = it.size_hint();
+    EXPECT_EQ(lower, 2_usize);
+    EXPECT_EQ(upper, sus::Option<usize>::some(2_usize));
+  }
+  EXPECT_EQ(it.next(), sus::Some);
+  {
+    auto [lower, upper] = it.size_hint();
+    EXPECT_EQ(lower, 1_usize);
+    EXPECT_EQ(upper, sus::Option<usize>::some(1_usize));
+  }
+  EXPECT_EQ(it.next(), sus::Some);
+  {
+    auto [lower, upper] = it.size_hint();
+    EXPECT_EQ(lower, 0_usize);
+    EXPECT_EQ(upper, sus::Option<usize>::some(0_usize));
+  }
+  EXPECT_EQ(it.next(), sus::None);
+  {
+    auto [lower, upper] = it.size_hint();
+    EXPECT_EQ(lower, 0_usize);
+    EXPECT_EQ(upper, sus::Option<usize>::some(0_usize));
+  }
+}
+
+TEST(Vec, ExactSizeIterator) {
+  auto v = Vec<i32>();
+  v.push(1_i32);
+  v.push(2_i32);
+  v.push(3_i32);
+  auto it = sus::move(v).into_iter();
+  static_assert(sus::iter::ExactSizeIterator<decltype(it), i32>);
+  EXPECT_EQ(it.exact_size_hint(), 3_usize);
+  EXPECT_EQ(it.next(), sus::Some);
+  EXPECT_EQ(it.exact_size_hint(), 2_usize);
+  EXPECT_EQ(it.next(), sus::Some);
+  EXPECT_EQ(it.exact_size_hint(), 1_usize);
+  EXPECT_EQ(it.next(), sus::Some);
+  EXPECT_EQ(it.exact_size_hint(), 0_usize);
+  EXPECT_EQ(it.next(), sus::None);
+  EXPECT_EQ(it.exact_size_hint(), 0_usize);
 }
 
 TEST(Vec, Destroy) {

subspace/iter/iterator_concept.h

@@ -20,6 +20,10 @@
 #include "subspace/convert/subclass.h"
 #include "subspace/mem/forward.h"
 
+namespace sus::num {
+struct usize;
+}
+
 namespace sus::option {
 template <class T>
 class Option;
@@ -76,7 +80,12 @@ concept Iterator = requires(T& t) {
 /// interchangeable for this purpose.
 template <class T, class Item>
 concept DoubleEndedIterator = Iterator<T, Item> && requires(T& t) {
-  { t.next_back() } -> std::same_as<::sus::option::Option<Item>>;
+  { t.next_back() } noexcept -> std::same_as<::sus::option::Option<Item>>;
+};
+
+template <class T, class Item>
+concept ExactSizeIterator = Iterator<T, Item> && requires(const T& t) {
+  { t.exact_size_hint() } noexcept -> std::same_as<::sus::num::usize>;
 };
 
 /// Conversion into an `Iterator`.

subspace/iter/iterator_defn.h

@@ -98,7 +98,7 @@ class IteratorBase {
   ///
   /// The default implementation returns `lower = 0` and `upper = None` which is
   /// correct for any iterator.
-  virtual SizeHint size_hint() noexcept {
+  virtual SizeHint size_hint() const noexcept {
     return SizeHint(0_usize, ::sus::Option<::sus::num::usize>::none());
   }
 

subspace/num/__private/float_macros.h

@@ -29,7 +29,7 @@
 
 namespace sus::containers {
 template <class T, size_t N>
-  requires(N <= PTRDIFF_MAX)
+  requires(N <= size_t{PTRDIFF_MAX})
 class Array;
 }