From fb45fb60efba787afdd8b859df738f4ec91cd44c Mon Sep 17 00:00:00 2001
From: Vinnie Falco <vinnie.falco@gmail.com>
Date: Tue, 12 Nov 2019 08:25:26 -0800
Subject: [PATCH] Refactor array impl

---
 include/boost/json/array.hpp      |  48 ++++++--
 include/boost/json/impl/array.hpp |  84 ++++++-------
 include/boost/json/impl/array.ipp | 198 ++++++++++++++----------------
 3 files changed, 176 insertions(+), 154 deletions(-)

diff --git a/include/boost/json/array.hpp b/include/boost/json/array.hpp
index 948e8f06..530b01f8 100644
--- a/include/boost/json/array.hpp
+++ b/include/boost/json/array.hpp
@@ -176,7 +176,7 @@ public:
     */
     ~array()
     {
-        if( impl_.vec &&
+        if( impl_.begin() &&
             sp_->need_free())
             destroy();
     }
@@ -961,7 +961,7 @@ public:
     bool
     empty() const noexcept
     {
-        return impl_.size == 0;
+        return impl_.size() == 0;
     }
 
     /** Return the number of elements in the container
@@ -977,7 +977,7 @@ public:
     std::size_t
     size() const noexcept
     {
-        return impl_.size;
+        return impl_.size();
     }
 
     /** Return the maximum number of elements the container can hold
@@ -1011,7 +1011,7 @@ public:
     std::size_t
     capacity() const noexcept
     {
-        return impl_.capacity;
+        return impl_.capacity();
     }
 
     /** Increase the capacity to at least a certain amount.
@@ -1049,7 +1049,7 @@ public:
     reserve(std::size_t new_capacity)
     {
         // never shrink
-        if(new_capacity <= impl_.capacity)
+        if(new_capacity <= impl_.capacity())
             return;
         reserve_impl(new_capacity);
     }
@@ -1590,12 +1590,13 @@ private:
     constexpr
     unsigned long min_capacity_ = 16;
 
-    struct impl_type
+    class impl_type
     {
-        value* vec = nullptr;
-        std::uint32_t size = 0;
-        std::uint32_t capacity = 0;
+        value* vec_ = nullptr;
+        std::uint32_t size_ = 0;
+        std::uint32_t capacity_ = 0;
 
+    public:
         impl_type() = default;
         impl_type(impl_type const&) = default;
         impl_type& operator=(
@@ -1611,6 +1612,35 @@ private:
         operator=(
             impl_type&& other) noexcept;
 
+        inline
+        value*
+        begin() const noexcept
+        {
+            return vec_;
+        }
+
+        inline
+        std::size_t
+        size() const noexcept
+        {
+            return size_;
+        }
+
+        inline
+        void
+        size(std::size_t n) noexcept
+        {
+            size_ = static_cast<
+                std::uint32_t>(n);
+        }
+
+        inline
+        std::size_t
+        capacity() const noexcept
+        {
+            return capacity_;
+        }
+
         inline
         std::size_t
         index_of(value const*) const noexcept;
diff --git a/include/boost/json/impl/array.hpp b/include/boost/json/impl/array.hpp
index 5d847057..6d147689 100644
--- a/include/boost/json/impl/array.hpp
+++ b/include/boost/json/impl/array.hpp
@@ -50,7 +50,7 @@ index_of(value const* pos) const noexcept ->
     std::size_t
 {
     return static_cast<
-        std::size_t>(pos - vec);
+        std::size_t>(pos - vec_);
 }
 
 //----------------------------------------------------------
@@ -116,11 +116,11 @@ array::
 at(std::size_t pos) ->
     reference
 {
-    if(pos >= impl_.size)
+    if(pos >= impl_.size())
         BOOST_JSON_THROW(
             std::out_of_range(
                 "bad array index"));
-    return impl_.vec[pos];
+    return impl_.begin()[pos];
 }
 
 auto
@@ -128,11 +128,11 @@ array::
 at(std::size_t pos) const ->
     const_reference
 {
-    if(pos >= impl_.size)
+    if(pos >= impl_.size())
         BOOST_JSON_THROW(
             std::out_of_range(
                 "bad array index"));
-    return impl_.vec[pos];
+    return impl_.begin()[pos];
 }
 
 auto
@@ -140,7 +140,7 @@ array::
 operator[](std::size_t pos) noexcept ->
     reference
 {
-    return impl_.vec[pos];
+    return impl_.begin()[pos];
 }
 
 auto
@@ -148,7 +148,7 @@ array::
 operator[](std::size_t pos) const noexcept ->
 const_reference
 {
-    return impl_.vec[pos];
+    return impl_.begin()[pos];
 }
 
 auto
@@ -156,7 +156,7 @@ array::
 front() noexcept ->
     reference
 {
-    return *impl_.vec;
+    return *impl_.begin();
 }
 
 auto
@@ -164,7 +164,7 @@ array::
 front() const noexcept ->
     const_reference
 {
-    return *impl_.vec;
+    return *impl_.begin();
 }
 
 auto
@@ -172,7 +172,7 @@ array::
 back() noexcept ->
     reference
 {
-    return impl_.vec[impl_.size - 1];
+    return impl_.begin()[impl_.size() - 1];
 }
 
 auto
@@ -180,7 +180,7 @@ array::
 back() const noexcept ->
     const_reference
 {
-    return impl_.vec[impl_.size - 1];
+    return impl_.begin()[impl_.size() - 1];
 }
 
 auto
@@ -188,7 +188,7 @@ array::
 data() noexcept ->
     value*
 {
-    return impl_.vec;
+    return impl_.begin();
 }
 
 auto
@@ -196,7 +196,7 @@ array::
 data() const noexcept ->
     value const*
 {
-    return impl_.vec;
+    return impl_.begin();
 }
 
 //----------------------------------------------------------
@@ -210,7 +210,7 @@ array::
 begin() noexcept ->
     iterator
 {
-    return impl_.vec;
+    return impl_.begin();
 }
 
 auto
@@ -218,7 +218,7 @@ array::
 begin() const noexcept ->
     const_iterator
 {
-    return impl_.vec;
+    return impl_.begin();
 }
 
 auto
@@ -226,7 +226,7 @@ array::
 cbegin() const noexcept ->
     const_iterator
 {
-    return impl_.vec;
+    return impl_.begin();
 }
 
 auto
@@ -234,7 +234,7 @@ array::
 end() noexcept ->
     iterator
 {
-    return impl_.vec + impl_.size;
+    return impl_.begin() + impl_.size();
 }
 
 auto
@@ -242,7 +242,7 @@ array::
 end() const noexcept ->
     const_iterator
 {
-    return impl_.vec + impl_.size;
+    return impl_.begin() + impl_.size();
 }
 
 auto
@@ -250,7 +250,7 @@ array::
 cend() const noexcept ->
     const_iterator
 {
-    return impl_.vec + impl_.size;
+    return impl_.begin() + impl_.size();
 }
 
 auto
@@ -259,7 +259,7 @@ rbegin() noexcept ->
     reverse_iterator
 {
     return reverse_iterator(
-        impl_.vec + impl_.size);
+        impl_.begin() + impl_.size());
 }
 
 auto
@@ -268,7 +268,7 @@ rbegin() const noexcept ->
     const_reverse_iterator
 {
     return const_reverse_iterator(
-        impl_.vec + impl_.size);
+        impl_.begin() + impl_.size());
 }
 
 auto
@@ -277,7 +277,7 @@ crbegin() const noexcept ->
     const_reverse_iterator
 {
     return const_reverse_iterator(
-        impl_.vec + impl_.size);
+        impl_.begin() + impl_.size());
 }
 
 auto
@@ -286,7 +286,7 @@ rend() noexcept ->
     reverse_iterator
 {
     return reverse_iterator(
-        impl_.vec);
+        impl_.begin());
 }
 
 auto
@@ -295,7 +295,7 @@ rend() const noexcept ->
     const_reverse_iterator
 {
     return const_reverse_iterator(
-        impl_.vec);
+        impl_.begin());
 }
 
 auto
@@ -304,7 +304,7 @@ crend() const noexcept ->
     const_reverse_iterator
 {
     return const_reverse_iterator(
-        impl_.vec);
+        impl_.begin());
 }
 
 //----------------------------------------------------------
@@ -356,7 +356,7 @@ emplace(
     undo_insert u(pos, 1, *this);
     u.emplace(std::forward<Arg>(arg));
     u.commit = true;
-    return impl_.vec + u.pos;
+    return impl_.begin() + u.pos;
 }
 
 template<class Arg>
@@ -365,11 +365,11 @@ array::
 emplace_back(Arg&& arg) ->
     reference
 {
-    reserve(impl_.size + 1);
+    reserve(impl_.size() + 1);
     auto& v = *::new(
-        impl_.vec + impl_.size) value(
+        impl_.begin() + impl_.size()) value(
             std::forward<Arg>(arg), sp_);
-    ++impl_.size;
+    impl_.size(impl_.size() + 1);
     return v;
 }
 
@@ -390,11 +390,11 @@ array(
     undo_construct u(*this);
     while(first != last)
     {
-        if(impl_.size >= impl_.capacity)
-            reserve(impl_.size + 1);
-        ::new(impl_.vec + impl_.size) value(
+        if(impl_.size() >= impl_.capacity())
+            reserve(impl_.size() + 1);
+        ::new(impl_.begin() + impl_.size()) value(
             *first++, sp_);
-        ++impl_.size;
+        impl_.size(impl_.size() + 1);
     }
     u.commit = true;
 }
@@ -415,13 +415,13 @@ array(
         BOOST_JSON_THROW(
             detail::array_too_large_exception());
     reserve(static_cast<std::size_t>(n));
-    while(impl_.size < n)
+    while(impl_.size() < n)
     {
         ::new(
-            impl_.vec +
-            impl_.size) value(
+            impl_.begin() +
+            impl_.size()) value(
                 *first++, sp_);
-        ++impl_.size;
+        impl_.size(impl_.size() + 1);
     }
     u.commit = true;
 }
@@ -436,17 +436,17 @@ insert(
         iterator
 {
     if(first == last)
-        return impl_.vec +
+        return impl_.begin() +
             impl_.index_of(pos);
     array tmp(first, last, sp_);
     undo_insert u(
-        pos, tmp.impl_.size, *this);
+        pos, tmp.impl_.size(), *this);
     relocate(u.it,
-        tmp.impl_.vec, tmp.impl_.size);
+        tmp.impl_.begin(), tmp.impl_.size());
     // don't destroy values in tmp
-    tmp.impl_.size = 0;
+    tmp.impl_.size(0);
     u.commit = true;
-    return impl_.vec + u.pos;
+    return impl_.begin() + u.pos;
 }
 
 template<class InputIt>
diff --git a/include/boost/json/impl/array.ipp b/include/boost/json/impl/array.ipp
index c5d86d91..e7eebc7b 100644
--- a/include/boost/json/impl/array.ipp
+++ b/include/boost/json/impl/array.ipp
@@ -28,34 +28,34 @@ namespace json {
 array::
 impl_type::
 impl_type(
-    std::size_t capacity_,
+    std::size_t capacity,
     storage_ptr const& sp)
 {
-    if(capacity_ > max_size())
+    if(capacity > max_size())
         BOOST_JSON_THROW(
             detail::array_too_large_exception());
-    if(capacity_ > 0)
-        vec = reinterpret_cast<value*>(
+    if(capacity > 0)
+        vec_ = reinterpret_cast<value*>(
             sp->allocate(
-                capacity_ * sizeof(value),
+                capacity * sizeof(value),
                 alignof(value)));
     else
-        vec = nullptr;
-    capacity = static_cast<
-        std::uint32_t>(capacity_);
-    size = 0;
+        vec_ = nullptr;
+    capacity_ = static_cast<
+        std::uint32_t>(capacity);
+    size_ = 0;
 }
 
 
 array::
 impl_type::
 impl_type(impl_type&& other) noexcept
-    : vec(detail::exchange(
-        other.vec, nullptr))
-    , size(detail::exchange(
-        other.size, 0))
-    , capacity(detail::exchange(
-        other.capacity, 0))
+    : vec_(detail::exchange(
+        other.vec_, nullptr))
+    , size_(detail::exchange(
+        other.size_, 0))
+    , capacity_(detail::exchange(
+        other.capacity_, 0))
 {
 }
 
@@ -77,9 +77,9 @@ array::
 impl_type::
 swap(impl_type& rhs) noexcept
 {
-    std::swap(vec, rhs.vec);
-    std::swap(size, rhs.size);
-    std::swap(capacity, rhs.capacity);
+    std::swap(vec_, rhs.vec_);
+    std::swap(size_, rhs.size_);
+    std::swap(capacity_, rhs.capacity_);
 }
 
 void
@@ -88,13 +88,13 @@ impl_type::
 destroy(
     storage_ptr const& sp) noexcept
 {
-    if(vec && sp->need_free())
+    if(vec_ && sp->need_free())
     {
-        auto it = vec + size;
-        while(it != vec)
+        auto it = vec_ + size_;
+        while(it != vec_)
             (*--it).~value();
-        sp->deallocate(vec,
-            capacity * sizeof(value),
+        sp->deallocate(vec_,
+            capacity_ * sizeof(value),
             alignof(value));
     }
 }
@@ -125,14 +125,13 @@ undo_insert(
         BOOST_JSON_THROW(
             detail::array_too_large_exception());
     self_.reserve(
-        self_.impl_.size + n_);
+        self_.impl_.size() + n_);
     // (iterators invalidated now)
-    it = self_.impl_.vec + pos;
+    it = self_.impl_.begin() + pos;
     relocate(it + n_, it,
-        self_.impl_.size - pos);
-    self_.impl_.size = static_cast<
-        std::uint32_t>(
-            self_.impl_.size + n_);
+        self_.impl_.size() - pos);
+    self_.impl_.size(
+        self_.impl_.size() + n_);
 }
 
 array::
@@ -142,14 +141,13 @@ undo_insert::
     if(! commit)
     {
         auto const first =
-            self_.impl_.vec + pos;
+            self_.impl_.begin() + pos;
         self_.destroy(first, it);
-        self_.impl_.size = static_cast<
-            std::uint32_t>(
-                self_.impl_.size - n_);
+        self_.impl_.size(
+            self_.impl_.size() - n_);
         relocate(
             first, first + n_,
-            self_.impl_.size - pos);
+            self_.impl_.size() - pos);
     }
 }
 
@@ -174,12 +172,12 @@ array(
 {
     undo_construct u(*this);
     reserve(count);
-    while(impl_.size < count)
+    while(impl_.size() < count)
     {
         ::new(
-            impl_.vec +
-            impl_.size) value(v, sp_);
-        ++impl_.size;
+            impl_.begin() +
+            impl_.size()) value(v, sp_);
+        impl_.size(impl_.size() + 1);
     }
     u.commit = true;
 }
@@ -192,12 +190,12 @@ array(
 {
     undo_construct u(*this);
     reserve(count);
-    while(impl_.size < count)
+    while(impl_.size() < count)
     {
         ::new(
-            impl_.vec +
-            impl_.size) value(sp_);
-        ++impl_.size;
+            impl_.begin() +
+            impl_.size()) value(sp_);
+        impl_.size(impl_.size() + 1);
     }
     u.commit = true;
 }
@@ -271,9 +269,8 @@ array(unchecked_array&& ua)
     : sp_(ua.get_storage())
     , impl_(ua.size(), sp_) // exact
 {
-    impl_.size = static_cast<
-        std::uint32_t>(ua.size());
-    ua.relocate(impl_.vec);
+    impl_.size(ua.size());
+    ua.relocate(impl_.begin());
 }
 
 //----------------------------------------------------------
@@ -321,28 +318,28 @@ void
 array::
 shrink_to_fit() noexcept
 {
-    if(impl_.capacity <= impl_.size)
+    if(impl_.capacity() <= impl_.size())
         return;
-    if(impl_.size == 0)
+    if(impl_.size() == 0)
     {
         impl_.destroy(sp_);
         impl_ = {};
         return;
     }
-    if( impl_.size < min_capacity_ &&
-        impl_.capacity <= min_capacity_)
+    if( impl_.size() < min_capacity_ &&
+        impl_.capacity() <= min_capacity_)
         return;
 
 #ifndef BOOST_NO_EXCEPTIONS
     try
     {
 #endif
-        impl_type impl(impl_.size, sp_);
+        impl_type impl(impl_.size(), sp_);
         relocate(
-            impl.vec,
-            impl_.vec, impl_.size);
-        impl.size = impl_.size;
-        impl_.size = 0;
+            impl.begin(),
+            impl_.begin(), impl_.size());
+        impl.size(impl_.size());
+        impl_.size(0);
         impl_.swap(impl);
         impl.destroy(sp_);
 #ifndef BOOST_NO_EXCEPTIONS
@@ -365,11 +362,11 @@ void
 array::
 clear() noexcept
 {
-    if(! impl_.vec)
+    if(! impl_.begin())
         return;
-    destroy(impl_.vec,
-        impl_.vec + impl_.size);
-    impl_.size = 0;
+    destroy(impl_.begin(),
+        impl_.begin() + impl_.size());
+    impl_.size(0);
 }
 
 auto
@@ -384,7 +381,7 @@ insert(
     while(count--)
         u.emplace(v);
     u.commit = true;
-    return impl_.vec + u.pos;
+    return impl_.begin() + u.pos;
 }
 
 auto
@@ -400,7 +397,7 @@ insert(
     for(auto const& v : init)
         u.emplace(v);
     u.commit = true;
-    return impl_.vec + u.pos;
+    return impl_.begin() + u.pos;
 }
 
 auto
@@ -409,11 +406,11 @@ erase(
     const_iterator pos) noexcept ->
     iterator
 {
-    auto p = impl_.vec +
-        (pos - impl_.vec);
+    auto p = impl_.begin() +
+        (pos - impl_.begin());
     destroy(p, p + 1);
     relocate(p, p + 1, 1);
-    --impl_.size;
+    impl_.size(impl_.size() - 1);
     return p;
 }
 
@@ -427,13 +424,12 @@ erase(
     auto const n = static_cast<
         std::size_t>(last - first);
     auto const p =
-        impl_.vec + impl_.index_of(first);
+        impl_.begin() + impl_.index_of(first);
     destroy(p, p + n);
     relocate(p, p + n,
-        impl_.size -
+        impl_.size() -
             impl_.index_of(last));
-    impl_.size = static_cast<
-        std::uint32_t>(impl_.size - n);
+    impl_.size(impl_.size() - n);
     return p;
 }
 
@@ -443,32 +439,30 @@ pop_back() noexcept
 {
     auto const p = &back();
     destroy(p, p + 1);
-    --impl_.size;
+    impl_.size(impl_.size() - 1);
 }
 
 void
 array::
 resize(std::size_t count)
 {
-    if(count <= impl_.size)
+    if(count <= impl_.size())
     {
         destroy(
-            impl_.vec + count,
-            impl_.vec + impl_.size);
-        impl_.size = static_cast<
-            std::uint32_t>(count);
+            impl_.begin() + count,
+            impl_.begin() + impl_.size());
+        impl_.size(count);
         return;
     }
 
     reserve(count);
     auto it =
-        impl_.vec + impl_.size;
+        impl_.begin() + impl_.size();
     auto const end =
-        impl_.vec + count;
+        impl_.begin() + count;
     while(it != end)
         ::new(it++) value(sp_);
-    impl_.size = static_cast<
-        std::uint32_t>(count);
+    impl_.size(count);
 }
 
 void
@@ -480,10 +474,9 @@ resize(
     if(count <= size())
     {
         destroy(
-            impl_.vec + count,
-            impl_.vec + impl_.size);
-        impl_.size = static_cast<
-            std::uint32_t>(count);
+            impl_.begin() + count,
+            impl_.begin() + impl_.size());
+        impl_.size(count);
         return;
     }
 
@@ -503,18 +496,17 @@ resize(
     };
 
     auto const end =
-        impl_.vec + count;
+        impl_.begin() + count;
     undo u{*this,
-        impl_.vec + impl_.size,
-        impl_.vec + impl_.size};
+        impl_.begin() + impl_.size(),
+        impl_.begin() + impl_.size()};
     do
     {
         ::new(u.it) value(v, sp_);
         ++u.it;
     }
     while(u.it != end);
-    impl_.size = static_cast<
-        std::uint32_t>(count);
+    impl_.size(count);
     u.it = nullptr;
 }
 
@@ -556,11 +548,11 @@ void
 array::
 destroy() noexcept
 {
-    auto it = impl_.vec + impl_.size;
-    while(it != impl_.vec)
+    auto it = impl_.begin() + impl_.size();
+    while(it != impl_.begin())
         (*--it).~value();
-    sp_->deallocate(impl_.vec,
-        impl_.capacity * sizeof(value),
+    sp_->deallocate(impl_.begin(),
+        impl_.capacity() * sizeof(value),
         alignof(value));
 }
 
@@ -572,9 +564,9 @@ copy(array const& other)
     for(auto const& v : other)
     {
         ::new(
-            impl_.vec +
-            impl_.size) value(v, sp_);
-        ++impl_.size;
+            impl_.begin() +
+            impl_.size()) value(v, sp_);
+        impl_.size(impl_.size() + 1);
     }
 }
 
@@ -592,9 +584,9 @@ copy(
     for(auto const& v : init)
     {
         ::new(
-            impl_.vec +
-            impl_.size) value(v, sp_);
-        ++impl_.size;
+            impl_.begin() +
+            impl_.size()) value(v, sp_);
+        impl_.size(impl_.size() + 1);
     }
 }
 
@@ -602,12 +594,12 @@ void
 array::
 reserve_impl(std::size_t capacity)
 {
-    if(impl_.vec)
+    if(impl_.begin())
     {
         // 2x growth
         auto const hint =
-            impl_.capacity * 2;
-        if(hint < impl_.capacity) // overflow
+            impl_.capacity() * 2;
+        if(hint < impl_.capacity()) // overflow
             capacity = max_size();
         else if(capacity < hint)
             capacity = hint;
@@ -616,10 +608,10 @@ reserve_impl(std::size_t capacity)
         capacity = min_capacity_;
     impl_type impl(capacity, sp_);
     relocate(
-        impl.vec,
-        impl_.vec, impl_.size);
-    impl.size = impl_.size;
-    impl_.size = 0;
+        impl.begin(),
+        impl_.begin(), impl_.size());
+    impl.size(impl_.size());
+    impl_.size(0);
     impl_.destroy(sp_);
     impl_ = impl;
 }