This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: std::vector move assign patch
- From: Jonathan Wakely <jwakely dot gcc at gmail dot com>
- To: François Dumont <frs dot dumont at gmail dot com>
- Cc: "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 8 Jan 2014 10:14:43 +0000
- Subject: Re: std::vector move assign patch
- Authentication-results: sourceware.org; auth=none
- References: <52BDC6AD dot 5030207 at gmail dot com>
On 27 December 2013 18:27, François Dumont 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.
Thanks for your patience, the normal-mode fix is definitely correct,
and I've finished reviewing the other parts and they look good too.
> 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.
OK, great.
In the new test you have:
+ VERIFY( it == v1.begin() ); // Error, it singular
Please change this to "Error, it is singular"
> 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.
Please change this to "does not get moved"
> * include/debug/safe_base.h
> (_Safe_sequence_base(_Safe_sequence_base&&)): New.
> * include/debug/vector (__gnu_debug::vector<>(vector&&)): Use
> latter.
I don't think "latter" is clear here, please say something like "Use
new move constructor for base class" or "... for _Safe_sequence_base".
This is OK for trunk, thanks very much.
We might also want to fix just the normal-mode part on the 4.8 branch,
I'll think about that.