On 7 March 2013 22:21, Jonathan Wakely wrote: > On 7 March 2013 21:28, François Dumont wrote: >> Hi >> >> While working on unordered containers C++11 allocator integration I used >> forward_list tests you have done Jon. It reported some problems that should >> have been seen on forward_list or vector allocator tests too if those tests >> were indeed manipulating memory. But there weren't because no element was >> ever injected in the containers. >> >> So I have started injecting elements and the propagating_allocator issue >> shown up like in my unordered container tests. The problem is that there is >> an assertion in the allocator to check that memory is returned to the >> correct allocator instance thanks to the personality. But when forward_list >> or vector instances with non propagating allocators are swapped personnality >> is not swapped and the issue. So I have introduced a bool template parameter >> to disable all assertions regarding personality so that the allocator can >> still be used to check that personality hasn't been exchanged. >> >> Additional, making the vector tests more functional revealed a bug in >> vector implementation. > > Good catch, thanks. > > Instead of making uneq_allocator more complicated how about just doing > std::swap(v1, v2) again after the tests comparing get_personality? > > So we don't touch uneq_allocator, keep the operator== overloads in the > test, and the diff for forward_list/allocator/swap.cc just becomes > > diff --git > a/libstdc++-v3/testsuite/23_containers/forward_list/allocator/swap.cc > b/libstdc++-v3/testsuite/23_containers/forward_list/allocator/swap.cc > index a2e70b7..b5b4480 100644 > --- a/libstdc++-v3/testsuite/23_containers/forward_list/allocator/swap.cc > +++ b/libstdc++-v3/testsuite/23_containers/forward_list/allocator/swap.cc > @@ -48,10 +48,14 @@ void test01() > typedef propagating_allocator<T, false> alloc_type; > typedef std::forward_list<T, alloc_type> test_type; > test_type v1(alloc_type(1)); > + v1.push_front(T()); > test_type v2(alloc_type(2)); > + v2.push_front(T()); > std::swap(v1, v2); > VERIFY(1 == v1.get_allocator().get_personality()); > VERIFY(2 == v2.get_allocator().get_personality()); > + // swap back so assertions in uneq_allocator::deallocate don't fail > + std::swap(v1, v2); > } > > void test02() > @@ -60,7 +64,9 @@ void test02() > typedef propagating_allocator<T, true> alloc_type; > typedef std::forward_list<T, alloc_type> test_type; > test_type v1(alloc_type(1)); > + v1.push_front(T()); > test_type v2(alloc_type(2)); > + v2.push_front(T()); > std::swap(v1, v2); > VERIFY(2 == v1.get_allocator().get_personality()); > VERIFY(1 == v2.get_allocator().get_personality()); > > > That fixes the forward_list failure. I'm just testing teh vector parts now > ...
As expected it works for vector/swap.cc too. So we definitely need the bug fix to std::vector::operator= and the testsuite changes to add elements, but I think I'd prefer to just re-swap the containers in the non-propagating case.
