This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [test] Swapping non-equal non-POCS allocators is UB.
AbandonedPublic

Authored by STL_MSFT on Nov 14 2016, 12:04 PM.

Details

Summary

[libcxx] [test] Swapping non-equal non-POCS allocators is UB.

test_allocator is a non-POCS allocator. Instead of swapping containers
with A(1) and A(2), which triggers undefined behavior,
swapping A(3) with A(3) is conformant.

test/std/containers/sequences/vector/vector.special/swap.pass.cpp
Remove "#ifndef _LIBCPP_DEBUG_LEVEL" and a comment about
the now-fixed undefined behavior.

Diff Detail

Event Timeline

STL_MSFT updated this revision to Diff 77854.Nov 14 2016, 12:04 PM
STL_MSFT retitled this revision from to [libcxx] [test] Swapping non-equal non-POCS allocators is UB..
STL_MSFT updated this object.
STL_MSFT added reviewers: EricWF, mclow.lists.
STL_MSFT added a subscriber: cfe-commits.
mclow.lists edited edge metadata.Nov 15 2016, 7:05 AM

I'm working on this, too.
Trying to decide what the right choice is here.

STL_MSFT updated this revision to Diff 80352.Dec 5 2016, 5:33 PM
STL_MSFT edited edge metadata.

Merged with the recent addition of static_cast<std::size_t> to various lines in the unordered container tests. No actual changes to the diff, only to the context.

EricWF edited edge metadata.Dec 10 2016, 7:53 PM

I hope you don't mind but I committed by own version of this change as r289358. I was concerned about testing that the container didn't actually perform a swap, so I modified test_allocator to take an "id" parameter that does not participate in equality. I then changed the tests to check that the "id" was unchanged after the swap.

Once confirming that r289358 fixes your issues could you please close this review?

STL_MSFT abandoned this revision.Dec 12 2016, 12:44 PM

r289358 fixes everything for me, thanks! Abandoning this revision.