From a07a9b225012eb6792405a698d0fbe87945773bc Mon Sep 17 00:00:00 2001 From: Gerald Squelart Date: Tue, 20 Aug 2019 12:51:31 +0000 Subject: [PATCH] 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 std::string::string(InputIt, InputIt)`). Differential Revision: https://phabricator.services.mozilla.com/D42452 --HG-- extra : moz-landing-system : lando --- mozglue/baseprofiler/public/ModuloBuffer.h | 30 +++++++++++-- mozglue/tests/TestBaseProfiler.cpp | 52 ++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/mozglue/baseprofiler/public/ModuloBuffer.h b/mozglue/baseprofiler/public/ModuloBuffer.h index dcdc62969a33..30ad9eae8661 100644 --- a/mozglue/baseprofiler/public/ModuloBuffer.h +++ b/mozglue/baseprofiler/public/ModuloBuffer.h @@ -13,6 +13,7 @@ #include "mozilla/PowerOfTwo.h" #include "mozilla/UniquePtr.h" +#include #include #include @@ -139,8 +140,7 @@ class ModuloBuffer { class Iterator { // Alias to const- or mutable-`ModuloBuffer` depending on `IsBufferConst`. using ConstOrMutableBuffer = - typename std::conditional::type; + std::conditional_t; // Implementation note about the strange enable-if's below: // `template enable_if_t` @@ -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; + using reference = std::conditional_t; + 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 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 std::enable_if_t Poke(const void* aSrc, diff --git a/mozglue/tests/TestBaseProfiler.cpp b/mozglue/tests/TestBaseProfiler.cpp index eb2513625bd0..c192deebb8ec 100644 --- a/mozglue/tests/TestBaseProfiler.cpp +++ b/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::difference_type, + MB::Index>::value, + "ModuloBuffer::Reader::difference_type should be Index"); + static_assert(std::is_same::value_type, + MB::Byte>::value, + "ModuloBuffer::Reader::value_type should be Byte"); + static_assert(std::is_same::pointer, + const MB::Byte*>::value, + "ModuloBuffer::Reader::pointer should be const Byte*"); + static_assert(std::is_same::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::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::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::iterator_category>::value, + "ModuloBuffer::Reader::iterator_category should be derived " + "from bidirectional_iterator_tag"); + static_assert( + std::is_same::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);