Sun, 06 Dec 2015 08:05:25 GMT

I located a couple different problem spots that seem to fix the issue in the cases I've tested. On my end, I refactored steal_resources and broke out the resetting into clear_resources:

   void steal_resources(vector_alloc_holder &x) BOOST_NOEXCEPT_OR_NOTHROW
   {
      this->m_start     = x.m_start;
      this->m_size      = x.m_size;
      this->m_capacity  = x.m_capacity;
      x.clear_resources();
   }
   void clear_resources() BOOST_NOEXCEPT_OR_NOTHROW
   {
      this->m_start = pointer();
      this->m_size = this->m_capacity = 0;
   }

Every time we use a move_iterator to do the copy because the allocator isn't propagable, we need to clear out the previous allocator. That means simply adding x.clear_resources() after the call to assign in vector::priv_move_assign and small_vector::move_construct_impl.

Lastly, small_vector's move constructor seems rather suspect to me:

   small_vector(BOOST_RV_REF(small_vector) other)
      : base_type(initial_capacity_t(), internal_capacity(), ::boost::move(other.get_stored_allocator()))
   {  this->move_construct_impl(other, other.get_stored_allocator());   }

That call to other.get_stored_allocator() looks really suspicious after we've moved the allocator in the initializer...

status changed; resolution set

Ion Gaztañaga — Thu, 24 Dec 2015 10:52:33 GMT

status new → closed
resolution → invalid

This is not a bug but is intended to work just like std::vector::operator= when:

allocator_type is not propagable (e.g. when propagate_on_container_move_assignment::value is false)
allocators from *this and other are not equal.

In that case, elements are moved one-by-one but there is no requirement to clear the source. Quoting ( http://en.cppreference.com/w/cpp/container/vector/operator%3D):

Move assignment operator. Replaces the contents with those of other using move semantics (...). other is in a valid but unspecified state afterwards. (...)If std::allocator_traits<allocator_type>::propagate_on_container_move_assignment() (...) is false and the source and the target allocators do not compare equal, the target cannot take ownership of the source memory and must move-assign each element individually, allocating additional memory using its own allocator as needed.

Several std implementations (I think libcpp and dinkumware) don't clear() contents of the moved container after move assignment, if allocators are not propagable and they don't compare equal. libstdc++ seems to clear the source. LLVM's SmallVector also clears the source.

std::vector does not support unequal allocators in the move constructor (moving the allocator is required to yield to equal allocators), but small_vector must (as it holds internal storage which is never propagable). So the move constructor behaves more or less like a default-constructor + move assignment.

In your example, "a" uses internal storage to hold the unique_ptr, when it's moved to "b", "b" checks if "a"'s memory can be transferred, but since it's internal, it can't be done. So it move constructs elements one by one to "b"'s internal storage (so unique_ptr's in "b" should be nulled. "b" is left in a valid but unspecified state, which is the requirement of standard containers.

It's a bit surprising, but no one should care about the state of "b", it can be safely reused (and if it has capacity, some performance benefit can be obtained), but not guaranteed to be empty. All Boost.Container containers works like this, when allocators can't be propagated so it's a design decision.

Boost C++ Libraries: Ticket #11830: small_vector move is broken

status changed; resolution set