Re: [Boost-bugs] [Boost C++ Libraries] #11830: small_vector move is broken

Subject: Re: [Boost-bugs] [Boost C++ Libraries] #11830: small_vector move is broken
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2015-12-24 10:52:33

#11830: small_vector move is broken
------------------------------------------+-------------------------------
  Reporter:  mrmiller <michael_miller@…>  |      Owner:  igaztanaga
      Type:  Bugs                         |     Status:  closed
 Milestone:  To Be Determined             |  Component:  container
   Version:  Boost 1.59.0                 |   Severity:  Showstopper
Resolution:  invalid                      |   Keywords:  small_vector move
------------------------------------------+-------------------------------
Changes (by igaztanaga):

 * status:  new => closed
 * resolution:   => invalid

Comment:

 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.

-- 
Ticket URL: <https://svn.boost.org/trac/boost/ticket/11830#comment:2>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.