Bug 1574825 - Make ModuloBuffer::Iterator a proper random-access iterator - r=gregtatum

This allows its use in std algorithms and types that require a real iterator
(like `template <typename InputIt> std::string::string(InputIt, InputIt)`).

Differential Revision: https://phabricator.services.mozilla.com/D42452

--HG--
extra : moz-landing-system : lando

mozglue/baseprofiler/public/ModuloBuffer.h

@@ -13,6 +13,7 @@
 #include "mozilla/PowerOfTwo.h"
 #include "mozilla/UniquePtr.h"
 
+#include <iterator>
 #include <limits>
 #include <type_traits>
 
@@ -139,8 +140,7 @@ class ModuloBuffer {
   class Iterator {
     // Alias to const- or mutable-`ModuloBuffer` depending on `IsBufferConst`.
     using ConstOrMutableBuffer =
-        typename std::conditional<IsBufferConst, const ModuloBuffer,
-                                  ModuloBuffer>::type;
+        std::conditional_t<IsBufferConst, const ModuloBuffer, ModuloBuffer>;
 
     // Implementation note about the strange enable-if's below:
     //   `template <bool NotIBC = !IsBufferConst> enable_if_t<NotIBC>`
@@ -188,6 +188,14 @@ class ModuloBuffer {
     // C++ is fun!
 
    public:
+    // These definitions are expected by std functions, to recognize this as an
+    // iterator. See https://en.cppreference.com/w/cpp/iterator/iterator_traits
+    using difference_type = Index;
+    using value_type = Byte;
+    using pointer = std::conditional_t<IsBufferConst, const Byte*, Byte*>;
+    using reference = std::conditional_t<IsBufferConst, const Byte&, Byte&>;
+    using iterator_category = std::random_access_iterator_tag;
+
     // Can always copy/assign from the same kind of iterator.
     Iterator(const Iterator& aRhs) = default;
     Iterator& operator=(const Iterator& aRhs) = default;
@@ -247,10 +255,20 @@ class ModuloBuffer {
       ++mIndex;
       return *this;
     }
+    Iterator operator++(int) {
+      Iterator here(*mModuloBuffer, mIndex);
+      ++mIndex;
+      return here;
+    }
     Iterator& operator--() {
       --mIndex;
       return *this;
     }
+    Iterator operator--(int) {
+      Iterator here(*mModuloBuffer, mIndex);
+      --mIndex;
+      return here;
+    }
     Iterator& operator+=(Length aLength) {
       mIndex += aLength;
       return *this;
@@ -258,6 +276,9 @@ class ModuloBuffer {
     Iterator operator+(Length aLength) const {
       return Iterator(*mModuloBuffer, mIndex + aLength);
     }
+    friend Iterator operator+(Length aLength, const Iterator& aIt) {
+      return aIt + aLength;
+    }
     Iterator& operator-=(Length aLength) {
       mIndex -= aLength;
       return *this;
@@ -274,10 +295,13 @@ class ModuloBuffer {
     }
 
     // Dereference a single byte (read-only if `IsBufferConst` is true).
-    std::conditional_t<IsBufferConst, const Byte&, Byte&> operator*() const {
+    reference operator*() const {
       return mModuloBuffer->mBuffer[OffsetInBuffer()];
     }
 
+    // Random-access dereference.
+    reference operator[](Length aLength) const { return *(*this + aLength); }
+
     // Write data (if `IsBufferConst` is false) but don't move iterator.
     template <bool NotIsBufferConst = !IsBufferConst>
     std::enable_if_t<NotIsBufferConst> Poke(const void* aSrc,

mozglue/tests/TestBaseProfiler.cpp

@@ -303,9 +303,18 @@ static void TestModuloBuffer(ModuloBuffer<>& mb, uint32_t MBSize) {
   MOZ_RELEASE_ASSERT(--arit == mb.ReaderAt(0));
   MOZ_RELEASE_ASSERT(arit == mb.ReaderAt(0));
 
+  MOZ_RELEASE_ASSERT(arit++ == mb.ReaderAt(0));
+  MOZ_RELEASE_ASSERT(arit == mb.ReaderAt(1));
+
+  MOZ_RELEASE_ASSERT(arit-- == mb.ReaderAt(1));
+  MOZ_RELEASE_ASSERT(arit == mb.ReaderAt(0));
+
   MOZ_RELEASE_ASSERT(arit + 3 == mb.ReaderAt(3));
   MOZ_RELEASE_ASSERT(arit == mb.ReaderAt(0));
 
+  MOZ_RELEASE_ASSERT(4 + arit == mb.ReaderAt(4));
+  MOZ_RELEASE_ASSERT(arit == mb.ReaderAt(0));
+
   // (Can't have assignments inside asserts, hence the split.)
   const bool checkPlusEq = ((arit += 3) == mb.ReaderAt(3));
   MOZ_RELEASE_ASSERT(checkPlusEq);
@@ -318,6 +327,10 @@ static void TestModuloBuffer(ModuloBuffer<>& mb, uint32_t MBSize) {
   MOZ_RELEASE_ASSERT(checkMinusEq);
   MOZ_RELEASE_ASSERT(arit == mb.ReaderAt(1));
 
+  // Random access.
+  MOZ_RELEASE_ASSERT(&arit[3] == &*(arit + 3));
+  MOZ_RELEASE_ASSERT(arit == mb.ReaderAt(1));
+
   // Iterator difference.
   MOZ_RELEASE_ASSERT(mb.ReaderAt(3) - mb.ReaderAt(1) == 2);
   MOZ_RELEASE_ASSERT(mb.ReaderAt(1) - mb.ReaderAt(3) == MB::Index(-2));
@@ -350,6 +363,45 @@ static void TestModuloBuffer(ModuloBuffer<>& mb, uint32_t MBSize) {
   it2 = it;
   MOZ_RELEASE_ASSERT(it2.CurrentIndex() == 2);
 
+  // Iterator traits.
+  static_assert(std::is_same<std::iterator_traits<MB::Reader>::difference_type,
+                             MB::Index>::value,
+                "ModuloBuffer::Reader::difference_type should be Index");
+  static_assert(std::is_same<std::iterator_traits<MB::Reader>::value_type,
+                             MB::Byte>::value,
+                "ModuloBuffer::Reader::value_type should be Byte");
+  static_assert(std::is_same<std::iterator_traits<MB::Reader>::pointer,
+                             const MB::Byte*>::value,
+                "ModuloBuffer::Reader::pointer should be const Byte*");
+  static_assert(std::is_same<std::iterator_traits<MB::Reader>::reference,
+                             const MB::Byte&>::value,
+                "ModuloBuffer::Reader::reference should be const Byte&");
+  static_assert(std::is_base_of<
+                    std::input_iterator_tag,
+                    std::iterator_traits<MB::Reader>::iterator_category>::value,
+                "ModuloBuffer::Reader::iterator_category should be derived "
+                "from input_iterator_tag");
+  static_assert(std::is_base_of<
+                    std::forward_iterator_tag,
+                    std::iterator_traits<MB::Reader>::iterator_category>::value,
+                "ModuloBuffer::Reader::iterator_category should be derived "
+                "from forward_iterator_tag");
+  static_assert(std::is_base_of<
+                    std::bidirectional_iterator_tag,
+                    std::iterator_traits<MB::Reader>::iterator_category>::value,
+                "ModuloBuffer::Reader::iterator_category should be derived "
+                "from bidirectional_iterator_tag");
+  static_assert(
+      std::is_same<std::iterator_traits<MB::Reader>::iterator_category,
+                   std::random_access_iterator_tag>::value,
+      "ModuloBuffer::Reader::iterator_category should be "
+      "random_access_iterator_tag");
+
+  // Use as input iterator by std::string constructor (which is only considered
+  // with proper input iterators.)
+  std::string s(mb.ReaderAt(0), mb.ReaderAt(2));
+  MOZ_RELEASE_ASSERT(s == "xy");
+
   // Write 4-byte number at index 2.
   it.WriteObject(int32_t(123));
   MOZ_RELEASE_ASSERT(it.CurrentIndex() == 6);