From ece12779652dcfefee1328a3af5598ce114cb7f7 Mon Sep 17 00:00:00 2001 From: Antony Polukhin Date: Tue, 3 Dec 2013 13:40:58 +0400 Subject: [PATCH 1/3] Fixed issues with allocators that do not return pointers, added tes case for such situations (refs #9334) --- include/boost/circular_buffer/base.hpp | 40 +++++----- include/boost/circular_buffer/details.hpp | 89 +++++++++++++++-------- test/common.ipp | 89 ++++++++++++++++++++++- 3 files changed, 166 insertions(+), 52 deletions(-) diff --git a/include/boost/circular_buffer/base.hpp b/include/boost/circular_buffer/base.hpp index b3cdf884..a80a1036 100644 --- a/include/boost/circular_buffer/base.hpp +++ b/include/boost/circular_buffer/base.hpp @@ -690,7 +690,7 @@ class circular_buffer break; } if (is_uninitialized(dest)) { - ::new (dest) value_type(this_type::move_if_noexcept(*src)); + cb_details::do_construct(dest, this_type::move_if_noexcept(*src), m_alloc); ++constructed; } else { value_type tmp = this_type::move_if_noexcept(*src); @@ -902,7 +902,7 @@ class circular_buffer iterator b = begin(); BOOST_TRY { reset(buff, - cb_details::uninitialized_move_if_noexcept(b, b + (std::min)(new_capacity, size()), buff), + cb_details::uninitialized_move_if_noexcept(b, b + (std::min)(new_capacity, size()), buff, m_alloc), new_capacity); } BOOST_CATCH(...) { deallocate(buff, new_capacity); @@ -977,8 +977,8 @@ class circular_buffer pointer buff = allocate(new_capacity); iterator e = end(); BOOST_TRY { - reset(buff, cb_details::uninitialized_move_if_noexcept(e - (std::min)(new_capacity, size()), - e, buff), new_capacity); + reset(buff, cb_details::uninitialized_move_if_noexcept(e - (std::min)(new_capacity, size()), + e, buff, m_alloc), new_capacity); } BOOST_CATCH(...) { deallocate(buff, new_capacity); BOOST_RETHROW @@ -1125,7 +1125,7 @@ class circular_buffer initialize_buffer(cb.capacity()); m_first = m_buff; BOOST_TRY { - m_last = cb_details::uninitialized_copy(cb.begin(), cb.end(), m_buff); + m_last = cb_details::uninitialized_copy(cb.begin(), cb.end(), m_buff, m_alloc); } BOOST_CATCH(...) { deallocate(m_buff, cb.capacity()); BOOST_RETHROW @@ -1249,7 +1249,7 @@ class circular_buffer return *this; pointer buff = allocate(cb.capacity()); BOOST_TRY { - reset(buff, cb_details::uninitialized_copy(cb.begin(), cb.end(), buff), cb.capacity()); + reset(buff, cb_details::uninitialized_copy(cb.begin(), cb.end(), buff, m_alloc), cb.capacity()); } BOOST_CATCH(...) { deallocate(buff, cb.capacity()); BOOST_RETHROW @@ -1446,7 +1446,7 @@ class circular_buffer increment(m_last); m_first = m_last; } else { - ::new (m_last) value_type(static_cast(item)); + cb_details::do_construct(m_last, static_cast(item), m_alloc); increment(m_last); ++m_size; } @@ -1463,7 +1463,7 @@ class circular_buffer m_last = m_first; } else { decrement(m_first); - ::new (m_first) value_type(static_cast(item)); + cb_details::do_construct(m_first, static_cast(item), m_alloc); ++m_size; } } BOOST_CATCH(...) { @@ -2397,7 +2397,7 @@ class circular_buffer throw_exception(std::length_error("circular_buffer")); #if BOOST_CB_ENABLE_DEBUG pointer p = (n == 0) ? 0 : m_alloc.allocate(n, 0); - std::memset(p, cb_details::UNINITIALIZED, sizeof(value_type) * n); + std::memset(boost::addressof(*p), cb_details::UNINITIALIZED, sizeof(value_type) * n); return p; #else return (n == 0) ? 0 : m_alloc.allocate(n, 0); @@ -2438,7 +2438,7 @@ class circular_buffer */ void construct_or_replace(bool construct, pointer pos, param_value_type item) { if (construct) - ::new (pos) value_type(item); + cb_details::do_construct(pos, item, m_alloc); else replace(pos, item); } @@ -2450,7 +2450,7 @@ class circular_buffer */ void construct_or_replace(bool construct, pointer pos, rvalue_type item) { if (construct) - ::new (pos) value_type(boost::move(item)); + cb_details::do_construct(pos, boost::move(item), m_alloc); else replace(pos, boost::move(item)); } @@ -2460,7 +2460,7 @@ class circular_buffer m_alloc.destroy(p); #if BOOST_CB_ENABLE_DEBUG invalidate_iterators(iterator(this, p)); - std::memset(p, cb_details::UNINITIALIZED, sizeof(value_type)); + std::memset(boost::addressof(*p), cb_details::UNINITIALIZED, sizeof(value_type)); #endif } @@ -2590,7 +2590,7 @@ class circular_buffer if (buffer_capacity == 0) return; while (first != last && !full()) { - ::new (m_last) value_type(*first++); + cb_details::do_construct(m_last, *first++, m_alloc); increment(m_last); ++m_size; } @@ -2626,7 +2626,7 @@ class circular_buffer m_size = distance; } BOOST_TRY { - m_last = cb_details::uninitialized_copy(first, last, m_buff); + m_last = cb_details::uninitialized_copy(first, last, m_buff, m_alloc); } BOOST_CATCH(...) { deallocate(m_buff, buffer_capacity); BOOST_RETHROW @@ -2680,8 +2680,8 @@ class circular_buffer std::deque tmp(first, last, m_alloc); size_type distance = tmp.size(); assign_n(distance, distance, - cb_details::make_assign_range - (boost::make_move_iterator(tmp.begin()), boost::make_move_iterator(tmp.end()))); + cb_details::make_assign_range + (boost::make_move_iterator(tmp.begin()), boost::make_move_iterator(tmp.end()), m_alloc)); } //! Specialized assign method. @@ -2689,7 +2689,7 @@ class circular_buffer void assign(ForwardIterator first, ForwardIterator last, const std::forward_iterator_tag&) { BOOST_CB_ASSERT(std::distance(first, last) >= 0); // check for wrong range size_type distance = std::distance(first, last); - assign_n(distance, distance, cb_details::make_assign_range(first, last)); + assign_n(distance, distance, cb_details::make_assign_range(first, last, m_alloc)); } //! Specialized assign method. @@ -2732,7 +2732,7 @@ class circular_buffer distance = new_capacity; } assign_n(new_capacity, distance, - cb_details::make_assign_range(first, last)); + cb_details::make_assign_range(first, last, m_alloc)); } //! Helper assign method. @@ -2855,7 +2855,7 @@ class circular_buffer pointer p = m_last; BOOST_TRY { for (; ii < construct; ++ii, increment(p)) - ::new (p) value_type(*wrapper()); + cb_details::do_construct(p, *wrapper(), m_alloc); for (;ii < n; ++ii, increment(p)) replace(p, *wrapper()); } BOOST_CATCH(...) { @@ -2949,7 +2949,7 @@ class circular_buffer for (;ii > construct; --ii, increment(p)) replace(p, *wrapper()); for (; ii > 0; --ii, increment(p)) - ::new (p) value_type(*wrapper()); + cb_details::do_construct(p, *wrapper(), m_alloc); } BOOST_CATCH(...) { size_type constructed = ii < construct ? construct - ii : 0; m_last = add(m_last, constructed); diff --git a/include/boost/circular_buffer/details.hpp b/include/boost/circular_buffer/details.hpp index 2ab1789d..5d9c9b47 100644 --- a/include/boost/circular_buffer/details.hpp +++ b/include/boost/circular_buffer/details.hpp @@ -38,11 +38,42 @@ template void uninitialized_fill_n_with_alloc( ForwardIterator first, Diff n, const T& item, Alloc& alloc); -template -ForwardIterator uninitialized_copy(InputIterator first, InputIterator last, ForwardIterator dest); +template +ForwardIterator uninitialized_copy(InputIterator first, InputIterator last, ForwardIterator dest, Alloc& a); + +template +ForwardIterator uninitialized_move_if_noexcept(InputIterator first, InputIterator last, ForwardIterator dest, Alloc& a); + + +//! Those `do_construct` methods are required because in C++03 default allocators +//! have `construct` method that accepts second parameter in as a const reference; +//! while move-only types emulated by Boost.Move require constructor that accepts +//! a non-const reference. +//! +//! So when we need to call `construct` and pointer to value_type is provided, we +//! assume that it is safe to call placement new instead of Alloc::construct. +//! Otherwise we are asume that user has made his own allocator or uses allocator +//! from other libraries. In that case it's users ability to provide Alloc::construct +//! with non-const reference parameter or just do not use move-only types. +template +inline void do_construct(ValueType* p, BOOST_RV_REF(ValueType) item, Alloc&) { + ::new (p) ValueType(boost::move(item)); +} + +template +inline void do_construct(ValueType* p, const ValueType& item, Alloc&) { + ::new (p) ValueType(item); +} -template -ForwardIterator uninitialized_move_if_noexcept(InputIterator first, InputIterator last, ForwardIterator dest); +template +inline void do_construct(PointerT& p, BOOST_RV_REF(ValueType) item, Alloc& a) { + a.construct(p, boost::move(item)); +} + +template +inline void do_construct(PointerT& p, const ValueType& item, Alloc& a) { + a.construct(p, item); +} /*! \struct const_traits @@ -127,23 +158,24 @@ struct assign_n { \struct assign_range \brief Helper functor for assigning range of items. */ -template +template struct assign_range { Iterator m_first; Iterator m_last; + Alloc& m_alloc; - assign_range(const Iterator& first, const Iterator& last) BOOST_NOEXCEPT - : m_first(first), m_last(last) {} + assign_range(const Iterator& first, const Iterator& last, Alloc& alloc) + : m_first(first), m_last(last), m_alloc(alloc) {} template void operator () (Pointer p) const { - boost::cb_details::uninitialized_copy(m_first, m_last, p); + boost::cb_details::uninitialized_copy(m_first, m_last, p, m_alloc); } }; -template -inline assign_range make_assign_range(const Iterator& first, const Iterator& last) { - return assign_range(first, last); +template +inline assign_range make_assign_range(const Iterator& first, const Iterator& last, Alloc& a) { + return assign_range(first, last, a); } /*! @@ -427,48 +459,43 @@ operator + (typename Traits::difference_type n, const iterator& it \fn ForwardIterator uninitialized_copy(InputIterator first, InputIterator last, ForwardIterator dest) \brief Equivalent of std::uninitialized_copy but with explicit specification of value type. */ -template -inline ForwardIterator uninitialized_copy(InputIterator first, InputIterator last, ForwardIterator dest) { - typedef ValueType value_type; - - // We do not use allocator.construct and allocator.destroy - // because C++03 requires to take parameter by const reference but - // Boost.move requires nonconst reference +template +inline ForwardIterator uninitialized_copy(InputIterator first, InputIterator last, ForwardIterator dest, Alloc& a) { ForwardIterator next = dest; BOOST_TRY { for (; first != last; ++first, ++dest) - ::new (dest) value_type(*first); + do_construct(dest, *first, a); } BOOST_CATCH(...) { for (; next != dest; ++next) - next->~value_type(); + a.destroy(next); BOOST_RETHROW } BOOST_CATCH_END return dest; } -template -ForwardIterator uninitialized_move_if_noexcept_impl(InputIterator first, InputIterator last, ForwardIterator dest, +template +ForwardIterator uninitialized_move_if_noexcept_impl(InputIterator first, InputIterator last, ForwardIterator dest, Alloc& a, true_type) { for (; first != last; ++first, ++dest) - ::new (dest) ValueType(boost::move(*first)); + do_construct(dest, boost::move(*first), a); return dest; } -template -ForwardIterator uninitialized_move_if_noexcept_impl(InputIterator first, InputIterator last, ForwardIterator dest, +template +ForwardIterator uninitialized_move_if_noexcept_impl(InputIterator first, InputIterator last, ForwardIterator dest, Alloc& a, false_type) { - return uninitialized_copy(first, last, dest); + return uninitialized_copy(first, last, dest, a); } /*! \fn ForwardIterator uninitialized_move_if_noexcept(InputIterator first, InputIterator last, ForwardIterator dest) \brief Equivalent of std::uninitialized_copy but with explicit specification of value type and moves elements if they have noexcept move constructors. */ -template -ForwardIterator uninitialized_move_if_noexcept(InputIterator first, InputIterator last, ForwardIterator dest) { - typedef typename boost::is_nothrow_move_constructible::type tag_t; - return uninitialized_move_if_noexcept_impl(first, last, dest, tag_t()); +template +ForwardIterator uninitialized_move_if_noexcept(InputIterator first, InputIterator last, ForwardIterator dest, Alloc& a) { + typedef typename boost::is_nothrow_move_constructible::type tag_t; + return uninitialized_move_if_noexcept_impl(first, last, dest, a, tag_t()); } /*! @@ -480,7 +507,7 @@ inline void uninitialized_fill_n_with_alloc(ForwardIterator first, Diff n, const ForwardIterator next = first; BOOST_TRY { for (; n > 0; ++first, --n) - alloc.construct(first, item); + do_construct(first, item, alloc); } BOOST_CATCH(...) { for (; next != first; ++next) alloc.destroy(next); diff --git a/test/common.ipp b/test/common.ipp index 772b74fa..67dd4641 100644 --- a/test/common.ipp +++ b/test/common.ipp @@ -11,7 +11,8 @@ #include #include -void generic_test(CB_CONTAINER& cb) { +template +void generic_test(CB_CONTAINER& cb) { vector v; v.push_back(11); @@ -149,6 +150,88 @@ void size_test() { generic_test(cb2); } +template +class my_allocator { + typedef std::allocator base_t; + base_t base_; +public: + typedef T value_type; + + + typedef value_type& reference; + typedef const value_type& const_reference; + typedef typename base_t::size_type size_type; + typedef typename base_t::difference_type difference_type; + + struct const_pointer; + struct pointer { + pointer(){} + pointer(void* p) : hidden_ptr_((T*)p) {} + difference_type operator-(const const_pointer& rhs) const { return hidden_ptr_ - rhs.hidden_ptr_; } + difference_type operator-(pointer rhs) const { return hidden_ptr_ - rhs.hidden_ptr_; } + pointer operator-(size_type rhs) const { return hidden_ptr_ - rhs; } + bool operator == (pointer rhs) const { return hidden_ptr_ == rhs.hidden_ptr_; } + bool operator != (pointer rhs) const { return hidden_ptr_ != rhs.hidden_ptr_; } + bool operator < (pointer rhs) const { return hidden_ptr_ < rhs.hidden_ptr_; } + bool operator >= (pointer rhs) const { return hidden_ptr_ >= rhs.hidden_ptr_; } + pointer& operator++() { ++hidden_ptr_; return *this; } + pointer& operator--() { --hidden_ptr_; return *this; } + pointer& operator+=(size_type s) { hidden_ptr_ += s; return *this; } + pointer operator+(size_type s) const { return hidden_ptr_ + s; } + pointer operator++(int) { pointer p = *this; ++hidden_ptr_; return p; } + pointer operator--(int) { pointer p = *this; --hidden_ptr_; return p; } + T& operator*() const { return *hidden_ptr_; } + + T* hidden_ptr_; + }; + + struct const_pointer { + const_pointer(){} + const_pointer(pointer p) : hidden_ptr_(p.hidden_ptr_) {} + const_pointer(const void* p) : hidden_ptr_((const T*)p) {} + difference_type operator-(pointer rhs) const { return hidden_ptr_ - rhs.hidden_ptr_; } + difference_type operator-(const_pointer rhs) const { return hidden_ptr_ - rhs.hidden_ptr_; } + const_pointer operator-(size_type rhs) const { return hidden_ptr_ - rhs; } + bool operator == (const_pointer rhs) const { return hidden_ptr_ == rhs.hidden_ptr_; } + bool operator != (const_pointer rhs) const { return hidden_ptr_ != rhs.hidden_ptr_; } + bool operator < (const_pointer rhs) const { return hidden_ptr_ < rhs.hidden_ptr_; } + bool operator >= (const_pointer rhs) const { return hidden_ptr_ >= rhs.hidden_ptr_; } + const_pointer& operator++() { ++hidden_ptr_; return *this; } + const_pointer& operator--() { --hidden_ptr_; return *this; } + const_pointer& operator+=(size_type s) { hidden_ptr_ += s; return hidden_ptr_; } + const_pointer operator+(size_type s) const { return hidden_ptr_ + s; } + const_pointer operator++(int) { const_pointer p = *this; ++hidden_ptr_; return p; } + const_pointer operator--(int) { const_pointer p = *this; --hidden_ptr_; return p; } + const T& operator*() const { return *hidden_ptr_; } + + const T* hidden_ptr_; + }; + + template + struct rebind + { + typedef my_allocator other; + }; + + size_type max_size() const + { return base_.max_size(); } + + pointer allocate(size_type count, const void* hint = 0) { + return pointer(base_.allocate(count, hint)); + } + + void deallocate(const pointer &ptr, size_type s) + { base_.deallocate(ptr.hidden_ptr_, s); } + + template + void construct(const pointer &ptr, BOOST_FWD_REF(P) p) + { ::new(ptr.hidden_ptr_) value_type(::boost::forward

(p)); } + + void destroy(const pointer &ptr) + { (*ptr.hidden_ptr_).~value_type(); } + +}; + void allocator_test() { CB_CONTAINER cb1(10, 0); @@ -159,6 +242,10 @@ void allocator_test() { alloc.max_size(); generic_test(cb1); + + + CB_CONTAINER > cb_a(10, 0); + generic_test(cb_a); } void begin_and_end_test() { From fc1d341a26550dfafa505261841725157eb1e937 Mon Sep 17 00:00:00 2001 From: Antony Polukhin Date: Tue, 3 Dec 2013 15:44:43 +0400 Subject: [PATCH 2/3] Fixed setting memory to '0xcc' in debug mode for non-pointer allocator::pointer types --- include/boost/circular_buffer/base.hpp | 15 ++------------- include/boost/circular_buffer/debug.hpp | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/include/boost/circular_buffer/base.hpp b/include/boost/circular_buffer/base.hpp index a80a1036..b3913d67 100644 --- a/include/boost/circular_buffer/base.hpp +++ b/include/boost/circular_buffer/base.hpp @@ -34,21 +34,10 @@ #include #include #include -#if BOOST_CB_ENABLE_DEBUG - #include -#endif #if BOOST_WORKAROUND(__MWERKS__, BOOST_TESTED_AT(0x3205)) #include #endif -#if defined(BOOST_NO_STDC_NAMESPACE) -namespace std { - using ::memset; -} -#endif - - - namespace boost { /*! @@ -2397,7 +2386,7 @@ class circular_buffer throw_exception(std::length_error("circular_buffer")); #if BOOST_CB_ENABLE_DEBUG pointer p = (n == 0) ? 0 : m_alloc.allocate(n, 0); - std::memset(boost::addressof(*p), cb_details::UNINITIALIZED, sizeof(value_type) * n); + cb_details::do_fill_uninitialized_memory(p, sizeof(value_type) * n); return p; #else return (n == 0) ? 0 : m_alloc.allocate(n, 0); @@ -2460,7 +2449,7 @@ class circular_buffer m_alloc.destroy(p); #if BOOST_CB_ENABLE_DEBUG invalidate_iterators(iterator(this, p)); - std::memset(boost::addressof(*p), cb_details::UNINITIALIZED, sizeof(value_type)); + cb_details::do_fill_uninitialized_memory(p, sizeof(value_type)); #endif } diff --git a/include/boost/circular_buffer/debug.hpp b/include/boost/circular_buffer/debug.hpp index d3b6ef52..b6ab0fef 100644 --- a/include/boost/circular_buffer/debug.hpp +++ b/include/boost/circular_buffer/debug.hpp @@ -13,6 +13,16 @@ #pragma once #endif +#if BOOST_CB_ENABLE_DEBUG +#include + +#if defined(BOOST_NO_STDC_NAMESPACE) +namespace std { + using ::memset; +} +#endif + +#endif // BOOST_CB_ENABLE_DEBUG namespace boost { namespace cb_details { @@ -22,6 +32,17 @@ namespace cb_details { // The value the uninitialized memory is filled with. const int UNINITIALIZED = 0xcc; +template +inline void do_fill_uninitialized_memory(T* data, std::size_t size_in_bytes) BOOST_NOEXCEPT { + std::memset(static_cast(data), UNINITIALIZED, size_in_bytes); +} + +template +inline void do_fill_uninitialized_memory(T& /*data*/, std::size_t /*size_in_bytes*/) BOOST_NOEXCEPT { + // Do nothing +} + + class debug_iterator_registry; /*! From 62233c53bb21a0023522789abb54f309cb386ebc Mon Sep 17 00:00:00 2001 From: Antony Polukhin Date: Wed, 4 Dec 2013 19:16:45 +0400 Subject: [PATCH 3/3] Removed some whitespaces --- include/boost/circular_buffer/details.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/boost/circular_buffer/details.hpp b/include/boost/circular_buffer/details.hpp index 5d9c9b47..42e11cec 100644 --- a/include/boost/circular_buffer/details.hpp +++ b/include/boost/circular_buffer/details.hpp @@ -46,13 +46,13 @@ ForwardIterator uninitialized_move_if_noexcept(InputIterator first, InputIterato //! Those `do_construct` methods are required because in C++03 default allocators -//! have `construct` method that accepts second parameter in as a const reference; -//! while move-only types emulated by Boost.Move require constructor that accepts +//! have `construct` method that accepts second parameter in as a const reference; +//! while move-only types emulated by Boost.Move require constructor that accepts //! a non-const reference. //! -//! So when we need to call `construct` and pointer to value_type is provided, we +//! So when we need to call `construct` and pointer to value_type is provided, we //! assume that it is safe to call placement new instead of Alloc::construct. -//! Otherwise we are asume that user has made his own allocator or uses allocator +//! Otherwise we are asume that user has made his own allocator or uses allocator //! from other libraries. In that case it's users ability to provide Alloc::construct //! with non-const reference parameter or just do not use move-only types. template