[libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

[BillyONeal]

Summary

The standard only requires that moved-from standard library types are in a 'valid but unspecified state', not that any moved-from container is empty. When the container is using a POCMA allocator, asserting that the moved-from container is empty is reasonable because the target container needs to take ownership of the memory buffers allocated from the source container's allocator. However, when the allocator is non-POCMA, the destination container must not take over any buffers, and effectively must copy the contents of the source container.

In the MSVC++ implementation, in this non-POCMA case, we do not clear() the source container, so that subsequent operations can reuse memory if the container is not immediately destroyed.

[BillyONeal]

Also fixed vector<bool> test.

[EricWF]

I'm not sure I agree with your design decision, but this patch LGTM.

Are you not allowed to move the containers elements in this case?

[BillyONeal]

In D61366#1502073, @EricWF wrote:

I'm not sure I agree with your design decision, but this patch LGTM.

I wouldn't object to a standards change to make this the case; though it is suboptimal to destroy all the elements needlessly.

Are you not allowed to move the containers elements in this case?

Correct. The allocator is not POCMA and not equal, so it's functionally the same as doing assign(make_move_iterator(begin()), make_move_iterator(end())).

Billy3

[• Quuxplusone]

Are you not allowed to move the containers elements in this case?

Correct. The allocator is not POCMA and not equal, so it's functionally the same as doing assign(make_move_iterator(begin()), make_move_iterator(end())).

In case the clarification helps some reader: When the allocator is not-POCMA-and-not-equal, then you are forbidden to pilfer the pointer from the source container, but you are indeed allowed to move the elements, and that's what Billy is describing. You could finish by having the source container destroy all its elements (those elements being now in a moved-from state) and become empty, but that's less efficient than keeping the moved-from elements around.

Unless of course you know that the element type is trivially relocatable... ;)

[BillyONeal]

In D61366#1502170, @Quuxplusone wrote:

you are indeed allowed to move the elements

And indeed we *must* do that :).

[mclow.lists]

Is there a reason this hasn't been committed?

[BillyONeal]

In D61366#1569151, @mclow.lists wrote:

Is there a reason this hasn't been committed?

Because it needs that one change Casey requested I was going to do that next time we take a libcxx test harness update, and I was hoping we'd get some closure on what happens with https://reviews.llvm.org/D61364 before doing that. Would you like me to expedite this change specifically?

[BillyONeal]

https://github.com/llvm/llvm-project/commit/45f630d729e2cce044ed48e6eaf4b8e61e06fede Committed

test/std/containers/associative/map/map.cons/move_assign.pass.cpp

int main(int, char**)
             V(3, 3)
         };
         M m2(I(a2), I(a2+sizeof(a2)/sizeof(a2[0])), C(5), A(7));
         M m3(C(3), A(5));
         m3 = std::move(m1);
         assert(m3 == m2);
         assert(m3.get_allocator() == A(5));
         assert(m3.key_comp() == C(5));
-        assert(m1.empty());
+        LIBCPP_ASSERT(m1.empty());
     }
     {
         typedef std::pair<MoveOnly, MoveOnly> V;
         typedef std::pair<const MoveOnly, MoveOnly> VC;
         typedef test_compare<std::less<MoveOnly> > C;
         typedef other_allocator<VC> A;
         typedef std::map<MoveOnly, MoveOnly, C, A> M;
         typedef std::move_iterator<V*> I;

test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp

int main(int, char**)
             V(3, 3)
         };
         M m2(I(a2), I(a2+sizeof(a2)/sizeof(a2[0])), C(5), A(7));
         M m3(C(3), A(5));
         m3 = std::move(m1);
         assert(m3 == m2);
         assert(m3.get_allocator() == A(5));
         assert(m3.key_comp() == C(5));
-        assert(m1.empty());
+        LIBCPP_ASSERT(m1.empty());
     }
     {
         typedef std::pair<MoveOnly, MoveOnly> V;
         typedef std::pair<const MoveOnly, MoveOnly> VC;
         typedef test_compare<std::less<MoveOnly> > C;
         typedef other_allocator<VC> A;
         typedef std::multimap<MoveOnly, MoveOnly, C, A> M;
         typedef std::move_iterator<V*> I;

test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp

int main(int, char**)
             V(3)
         };
         M m2(I(a2), I(a2+sizeof(a2)/sizeof(a2[0])), C(5), A(7));
         M m3(C(3), A(5));
         m3 = std::move(m1);
         assert(m3 == m2);
         assert(m3.get_allocator() == A(5));
         assert(m3.key_comp() == C(5));
-        assert(m1.empty());
+        LIBCPP_ASSERT(m1.empty());
     }
     {
         typedef MoveOnly V;
         typedef test_compare<std::less<MoveOnly> > C;
         typedef other_allocator<V> A;
         typedef std::multiset<MoveOnly, C, A> M;
         typedef std::move_iterator<V*> I;
         V a1[] =

test/std/containers/associative/set/set.cons/move_assign.pass.cpp

int main(int, char**)
             V(3)
         };
         M m2(I(a2), I(a2+sizeof(a2)/sizeof(a2[0])), C(5), A(7));
         M m3(C(3), A(5));
         m3 = std::move(m1);
         assert(m3 == m2);
         assert(m3.get_allocator() == A(5));
         assert(m3.key_comp() == C(5));
-        assert(m1.empty());
+        LIBCPP_ASSERT(m1.empty());
     }
     {
         typedef MoveOnly V;
         typedef test_compare<std::less<MoveOnly> > C;
         typedef other_allocator<V> A;
         typedef std::set<MoveOnly, C, A> M;
         typedef std::move_iterator<V*> I;
         V a1[] =

test/std/containers/sequences/vector.bool/assign_move.pass.cpp

int main(int, char**)
         for (int i = 1; i <= 3; ++i)
         {
             l.push_back(i);
             lo.push_back(i);
         }
         std::vector<bool, test_allocator<bool> > l2(test_allocator<bool>(5));
         l2 = std::move(l);
         assert(l2 == lo);
-        assert(l.empty());
+        LIBCPP_ASSERT(l.empty());
         assert(l2.get_allocator() == lo.get_allocator());
     }
     {
         std::vector<bool, test_allocator<bool> > l(test_allocator<bool>(5));
         std::vector<bool, test_allocator<bool> > lo(test_allocator<bool>(5));
         for (int i = 1; i <= 3; ++i)
         {
             l.push_back(i);