Separate overridable from non-overridable methods in IteratorImpl

Author: danakj

subdoc/lib/database.h

@@ -647,7 +647,7 @@ struct Database {
 
       // Namespace path goes from the outside in to the global, we want the
       // inverse, and then to skip the global namespace.
-      auto it = iter_namespace_path(decl).collect_vec().into_iter().reverse();
+      auto it = iter_namespace_path(decl).collect_vec().into_iter().rev();
       it.next();  // TODO: Use Iterator::skip().
       for (const Namespace& n : it) {
         switch (n) {
@@ -686,7 +686,7 @@ struct Database {
         for (std::string_view name : iter_record_path(containing_record_decl)
                                          .collect_vec()
                                          .into_iter()
-                                         .reverse()) {
+                                         .rev()) {
           if (first) {
             auto r_it = ns_cursor->records.find(RecordId(name));
             if (r_it == ns_cursor->records.end()) {

subdoc/lib/gen/files.h

@@ -45,7 +45,7 @@ inline std::filesystem::path construct_html_file_path(
   std::filesystem::path p = sus::move(root);
 
   std::ostringstream fname;
-  for (const Namespace& n : namespace_path.iter().reverse()) {
+  for (const Namespace& n : namespace_path.iter().rev()) {
     switch (n) {
       case Namespace::Tag::Global: break;
       case Namespace::Tag::Anonymous:
@@ -58,7 +58,7 @@ inline std::filesystem::path construct_html_file_path(
         break;
     }
   }
-  for (std::string_view n : record_path.iter().reverse()) {
+  for (std::string_view n : record_path.iter().rev()) {
     fname << n << "-";
   }
   fname << name;

subdoc/lib/gen/generate_record.cc

@@ -50,7 +50,7 @@ void generate_record_overview(HtmlWriter::OpenDiv& record_div,
       {
         // TODO: This code gets duplicated a lot, share it.
 
-        for (const Namespace &n : element.namespace_path.iter().reverse()) {
+        for (const Namespace &n : element.namespace_path.iter().rev()) {
           switch (n) {
             case Namespace::Tag::Global: break;
             case Namespace::Tag::Anonymous: {
@@ -68,7 +68,7 @@ void generate_record_overview(HtmlWriter::OpenDiv& record_div,
               break;
           }
         }
-        for (std::string_view record_name: element.record_path.iter().reverse()) {
+        for (std::string_view record_name: element.record_path.iter().rev()) {
           {
             auto record_anchor = full_type_span.open_a();
             record_anchor.write_text(record_name);
@@ -228,7 +228,7 @@ void generate_record(const RecordElement& element,
 
   {
     std::ostringstream title;
-    for (const Namespace& n: element.namespace_path.iter().reverse()) {
+    for (const Namespace& n: element.namespace_path.iter().rev()) {
       switch (n) {
         case Namespace::Tag::Global: break;
         case Namespace::Tag::Anonymous:
@@ -241,7 +241,7 @@ void generate_record(const RecordElement& element,
           break;
       }
     }
-    for (std::string_view record_name: element.record_path.iter().reverse()) {
+    for (std::string_view record_name: element.record_path.iter().rev()) {
       title << record_name;
       title << "::";
     }

subdoc/lib/path.h

@@ -43,7 +43,7 @@ inline std::string namespace_with_path_to_string(
   std::ostringstream s;
   bool add_colons = false;
 
-  for (const Namespace& n : path.iter().reverse()) {
+  for (const Namespace& n : path.iter().rev()) {
     if (add_colons) s << "::";
     switch (n) {
       case Namespace::Tag::Global: break;

subspace/iter/iterator_defn.h

@@ -63,12 +63,17 @@ class IteratorImpl {
                   "methods must know the complete type.");
   }
 
+  inline const Iter& as_subclass() const {
+    return static_cast<const Iter&>(*this);
+  }
+  inline Iter& as_subclass_mut() { return static_cast<Iter&>(*this); }
+
  public:
   using Item = ItemT;
 
   /// Adaptor for use in ranged for loops.
   auto begin() & noexcept {
-    return __private::IteratorLoop<Iter&>(static_cast<Iter&>(*this));
+    return __private::IteratorLoop<Iter&>(as_subclass_mut());
   }
   /// Adaptor for use in ranged for loops.
   auto end() & noexcept { return __private::IteratorEnd(); }
@@ -78,7 +83,7 @@ class IteratorImpl {
   /// sus::iter::IntoIterator trait implementation.
   Iter&& into_iter() && noexcept { return static_cast<Iter&&>(*this); }
 
-  // Provided methods.
+  // Provided overridable methods.
 
   /// Returns the bounds on the remaining length of the iterator.
   ///
@@ -104,9 +109,7 @@ class IteratorImpl {
   ///
   /// The default implementation returns `lower = 0` and `upper = None` which is
   /// correct for any iterator.
-  virtual SizeHint size_hint() const noexcept {
-    return SizeHint(0_usize, ::sus::Option<::sus::num::usize>::none());
-  }
+  virtual SizeHint size_hint() const noexcept;
 
   /// Tests whether all elements of the iterator match a predicate.
   ///
@@ -116,7 +119,7 @@ class IteratorImpl {
   /// from the predicate.
   ///
   /// Returns `true` if the iterator is empty.
-  bool all(::sus::fn::FnMut<bool(Item)> f) noexcept;
+  virtual bool all(::sus::fn::FnMut<bool(Item)> f) noexcept;
 
   /// Tests whether any elements of the iterator match a predicate.
   ///
@@ -126,7 +129,21 @@ class IteratorImpl {
   /// the predicate.
   ///
   /// Returns `false` if the iterator is empty.
-  bool any(::sus::fn::FnMut<bool(Item)> f) noexcept;
+  virtual bool any(::sus::fn::FnMut<bool(Item)> f) noexcept;
+
+  /// Consumes the iterator, and returns the number of elements that were in
+  /// it.
+  ///
+  /// The function walks the iterator until it sees an Option holding #None.
+  ///
+  /// # Safety
+  ///
+  /// If the `usize` type does not have trapping arithmetic enabled, and the
+  /// iterator has more than `usize::MAX` elements in it, the value will wrap
+  /// and be incorrect. Otherwise, `usize` will catch overflow and panic.
+  virtual ::sus::num::usize count() noexcept;
+
+  // Provided final methods.
 
   /// Wraps the iterator in a new iterator that is trivially relocatable.
   ///
@@ -145,18 +162,6 @@ class IteratorImpl {
   auto box() && noexcept
     requires(!::sus::mem::relocate_by_memcpy<Iter>);
 
-  /// Consumes the iterator, and returns the number of elements that were in
-  /// it.
-  ///
-  /// The function walks the iterator until it sees an Option holding #None.
-  ///
-  /// # Safety
-  ///
-  /// If the `usize` type does not have trapping arithmetic enabled, and the
-  /// iterator has more than `usize::MAX` elements in it, the value will wrap
-  /// and be incorrect. Otherwise, `usize` will catch overflow and panic.
-  ::sus::num::usize count() noexcept;
-
   /// Creates an iterator which uses a closure to map each element to another
   /// type.
   ///
@@ -176,8 +181,16 @@ class IteratorImpl {
                   pred) && noexcept
     requires(::sus::mem::relocate_by_memcpy<Iter>);
 
-  auto reverse() && noexcept
-    requires(::sus::mem::relocate_by_memcpy<Iter>);
+  /// Reverses an iterator's direction.
+  ///
+  /// Usually, iterators iterate from front to back. After using `rev()`, an
+  /// iterator will instead iterate from back to front.
+  ///
+  /// This is only possible if the iterator has an end, so `rev()` only works on
+  /// `DoubleEndedIterator`s.
+  auto rev() && noexcept
+    requires(::sus::mem::relocate_by_memcpy<Iter> &&
+             ::sus::iter::DoubleEndedIterator<Iter, Item>);
 
   /// Transforms an iterator into a collection.
   ///
@@ -219,11 +232,15 @@ class IteratorImpl {
   // TODO: cloned().
 };
 
+template <class Iter, class Item>
+SizeHint IteratorImpl<Iter, Item>::size_hint() const noexcept {
+  return SizeHint(0_usize, ::sus::Option<::sus::num::usize>::none());
+}
+
 template <class Iter, class Item>
 bool IteratorImpl<Iter, Item>::all(::sus::fn::FnMut<bool(Item)> f) noexcept {
-  // TODO: If constexpr(I::all() exists) then call that instead.
   while (true) {
-    Option<Item> item = static_cast<Iter&>(*this).next();
+    Option<Item> item = as_subclass_mut().next();
     if (item.is_none()) return true;
     // SAFETY: `item` was checked to hold Some already.
     if (!f(item.take().unwrap_unchecked(::sus::marker::unsafe_fn)))
@@ -233,23 +250,14 @@ bool IteratorImpl<Iter, Item>::all(::sus::fn::FnMut<bool(Item)> f) noexcept {
 
 template <class Iter, class Item>
 bool IteratorImpl<Iter, Item>::any(::sus::fn::FnMut<bool(Item)> f) noexcept {
-  // TODO: If constexpr(I::any() exists) then call that instead.
   while (true) {
-    Option<Item> item = static_cast<Iter&>(*this).next();
+    Option<Item> item = as_subclass_mut().next();
     if (item.is_none()) return false;
     // SAFETY: `item` was checked to hold Some already.
     if (f(item.take().unwrap_unchecked(::sus::marker::unsafe_fn))) return true;
   }
 }
 
-template <class Iter, class Item>
-::sus::num::usize IteratorImpl<Iter, Item>::count() noexcept {
-  // TODO: If constexpr(I::count() exists) then call that instead.
-  auto c = 0_usize;
-  while (static_cast<Iter&>(*this).next().is_some()) c += 1_usize;
-  return c;
-}
-
 template <class Iter, class Item>
 auto IteratorImpl<Iter, Item>::box() && noexcept
   requires(!::sus::mem::relocate_by_memcpy<Iter>)
@@ -260,6 +268,13 @@ auto IteratorImpl<Iter, Item>::box() && noexcept
   return BoxedIterator::with(static_cast<Iter&&>(*this));
 }
 
+template <class Iter, class Item>
+::sus::num::usize IteratorImpl<Iter, Item>::count() noexcept {
+  auto c = 0_usize;
+  while (as_subclass_mut().next().is_some()) c += 1_usize;
+  return c;
+}
+
 template <class Iter, class Item>
 template <class MapFn, int&..., class R, class MapFnMut>
   requires(::sus::construct::Into<MapFn, MapFnMut> && !std::is_void_v<R>)
@@ -285,8 +300,9 @@ auto IteratorImpl<Iter, Item>::filter(
 }
 
 template <class Iter, class Item>
-auto IteratorImpl<Iter, Item>::reverse() && noexcept
-  requires(::sus::mem::relocate_by_memcpy<Iter>)
+auto IteratorImpl<Iter, Item>::rev() && noexcept
+  requires(::sus::mem::relocate_by_memcpy<Iter> &&
+           ::sus::iter::DoubleEndedIterator<Iter, Item>)
 {
   using Sized = SizedIteratorType<Iter>::type;
   using Reverse = Reverse<Sized>;

subspace/iter/iterator_unittest.cc

@@ -374,10 +374,10 @@ TEST(Iterator, CollectVec) {
   EXPECT_EQ(collected[4u], 5);
 }
 
-TEST(Iterator, Reverse) {
+TEST(Iterator, Rev) {
   i32 nums[5] = {1, 2, 3, 4, 5};
 
-  auto it = ArrayIterator<i32, 5>::with_array(nums).reverse();
+  auto it = ArrayIterator<i32, 5>::with_array(nums).rev();
   static_assert(sus::iter::Iterator<decltype(it), i32>);
   static_assert(sus::iter::DoubleEndedIterator<decltype(it), i32>);
   EXPECT_EQ(it.next(), sus::some(5).construct());