This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: std::vector move assign patch


On Thu, 9 Jan 2014, Jonathan Wakely wrote:

On 9 January 2014 12:22, H.J. Lu wrote:
On Fri, Dec 27, 2013 at 10:27 AM, François Dumont <frs.dumont@gmail.com> wrote:
Hi

    Here is a patch to fix an issue in normal mode during the move
assignment. The destination vector allocator instance is moved too during
the assignment which is wrong.

    As I discover this problem while working on issues with management of
safe iterators during move operations this patch also fix those issues in
the debug mode for the vector container. Fixes for other containers in debug
mode will come later.

2013-12-27  François Dumont <fdumont@gcc.gnu.org>

    * include/bits/stl_vector.h (std::vector<>::_M_move_assign): Pass
    *this allocator instance when building temporary vector instance
    so that *this allocator do not get moved.
    * include/debug/safe_base.h
    (_Safe_sequence_base(_Safe_sequence_base&&)): New.
    * include/debug/vector (__gnu_debug::vector<>(vector&&)): Use
    latter.
    (__gnu_debug::vector<>(vector&&, const allocator_type&)): Swap
    safe iterators if the instance is moved.
    (__gnu_debug::vector<>::operator=(vector&&)): Likewise.
    * testsuite/23_containers/vector/allocator/move.cc (test01): Add
    check on a vector iterator.
    * testsuite/23_containers/vector/allocator/move_assign.cc
    (test02): Likewise.
    (test03): New, test with a non-propagating allocator.
    * testsuite/23_containers/vector/debug/move_assign_neg.cc: New.

Tested under Linux x86_64 normal and debug modes.

I will be in vacation for a week starting today so if you want to apply it
quickly do not hesitate to do it yourself.


This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59738

Fixed by the attached patch, tested x86_64-linux and committed to
trunk.  I've also rotated the libstdc++ ChangeLog.


2014-01-09  Jonathan Wakely  <jwakely@redhat.com>

       PR libstdc++/59738
       * include/bits/stl_vector.h (vector<>::_M_move_assign): Restore
       support for non-Movable types.

It seems that this patch had 2 consequences that may or may not have been planned. Consider this example (from PR64601)

#include <vector>
typedef std::vector<int> V;
void f(V&v,V&w){ V(std::move(w)).swap(v); }
void g(V&v,V&w){ v=std::move(w); }

1) We generate shorter code for f than for g, probably since the fix for PR59738. g ends up zeroing v, copying w to v, and finally zeroing w, and for weird reasons (and because we swap the members one by one) the standard prevents us from assuming that v and w do not overlap in weird ways so we cannot optimize as much as one might expect.

2) g(v,v) seems to turn v into a nice empty vector, while f(v,v) turns it into an invalid vector pointing at released memory.

Since 2) is a nice side-effect, it may not be worth rewriting operator= in a way that improves 1) but loses 2). Anyway, just mentioning this here.

--
Marc Glisse


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]