This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
[Patch] Two changes to the C++0x vector
- From: Paolo Carlini <paolo dot carlini at oracle dot com>
- To: libstdc++ <libstdc++ at gcc dot gnu dot org>
- Cc: "HERVE BRONNIMANN, BLOOMBERG/ 731 LEXIN" <hbronnimann at bloomberg dot net>
- Date: Sun, 01 Jun 2008 13:00:59 +0200
- Subject: [Patch] Two changes to the C++0x vector
Hi,
the below solves two issues in the C++0x vector, a small optimization
issue pointed out by Herve (thanks again!) and a correctness issue (a
risk of memory leak) noticed while dealing with the former.
Tested x86_64-linux, will go ahead tomorrow for mainline and, because of
the correctness bit, probably will go in 4_3-branch too.
Paolo.
/////////////////////
2008-06-02 Paolo Carlini <paolo.carlini@oracle.com>
* include/bits/vector.tcc (vector<>::_M_insert_aux): In C++0x mode,
avoid a memory leak if the first __uninitialized_move_a throws.
(vector<>::_M_fill_insert): Do not always copy to __x_copy, similarly
to _M_insert_aux.
* testsuite/23_containers/vector/modifiers/moveable.cc: Adjust.
* testsuite/23_containers/vector/resize/moveable.cc: Likewise.
Index: include/bits/vector.tcc
===================================================================
--- include/bits/vector.tcc (revision 136250)
+++ include/bits/vector.tcc (working copy)
@@ -1,6 +1,6 @@
// Vector implementation (out of line) -*- C++ -*-
-// Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007
+// Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008
// Free Software Foundation, Inc.
//
// This file is part of the GNU ISO C++ Library. This library is free
@@ -305,22 +305,29 @@
{
const size_type __len =
_M_check_len(size_type(1), "vector::_M_insert_aux");
+ const size_type __elems_before = __position - begin();
pointer __new_start(this->_M_allocate(__len));
pointer __new_finish(__new_start);
try
{
+ // The order of the three operations is dictated by the C++0x
+ // case, where the moves could alter a new element belonging
+ // to the existing vector. This is an issue only for callers
+ // taking the element by const lvalue ref (see 23.1/13).
+ this->_M_impl.construct(__new_start + __elems_before,
#ifdef __GXX_EXPERIMENTAL_CXX0X__
- this->_M_impl.construct(__new_start + (__position - begin()),
std::forward<_Args>(__args)...);
+#else
+ __x);
#endif
+ __new_finish = 0;
+
__new_finish =
std::__uninitialized_move_a(this->_M_impl._M_start,
__position.base(), __new_start,
_M_get_Tp_allocator());
-#ifndef __GXX_EXPERIMENTAL_CXX0X__
- this->_M_impl.construct(__new_finish, __x);
-#endif
++__new_finish;
+
__new_finish =
std::__uninitialized_move_a(__position.base(),
this->_M_impl._M_finish,
@@ -329,7 +336,10 @@
}
catch(...)
{
- std::_Destroy(__new_start, __new_finish, _M_get_Tp_allocator());
+ if (!__new_finish)
+ this->_M_impl.destroy(__new_start + __elems_before);
+ else
+ std::_Destroy(__new_start, __new_finish, _M_get_Tp_allocator());
_M_deallocate(__new_start, __len);
__throw_exception_again;
}
@@ -351,15 +361,10 @@
{
if (__n != 0)
{
-#ifdef __GXX_EXPERIMENTAL_CXX0X__
- value_type __x_copy = __x;
-#endif
if (size_type(this->_M_impl._M_end_of_storage
- this->_M_impl._M_finish) >= __n)
{
-#ifndef __GXX_EXPERIMENTAL_CXX0X__
value_type __x_copy = __x;
-#endif
const size_type __elems_after = end() - __position;
pointer __old_finish(this->_M_impl._M_finish);
if (__elems_after > __n)
@@ -392,22 +397,24 @@
{
const size_type __len =
_M_check_len(__n, "vector::_M_fill_insert");
+ const size_type __elems_before = __position - begin();
pointer __new_start(this->_M_allocate(__len));
pointer __new_finish(__new_start);
try
{
+ // See _M_insert_aux above.
+ std::__uninitialized_fill_n_a(__new_start + __elems_before,
+ __n, __x,
+ _M_get_Tp_allocator());
+ __new_finish = 0;
+
__new_finish =
std::__uninitialized_move_a(this->_M_impl._M_start,
__position.base(),
__new_start,
_M_get_Tp_allocator());
-#ifdef __GXX_EXPERIMENTAL_CXX0X__
- std::__uninitialized_fill_n_a(__new_finish, __n, __x_copy,
-#else
- std::__uninitialized_fill_n_a(__new_finish, __n, __x,
-#endif
- _M_get_Tp_allocator());
__new_finish += __n;
+
__new_finish =
std::__uninitialized_move_a(__position.base(),
this->_M_impl._M_finish,
@@ -416,8 +423,13 @@
}
catch(...)
{
- std::_Destroy(__new_start, __new_finish,
- _M_get_Tp_allocator());
+ if (!__new_finish)
+ std::_Destroy(__new_start + __elems_before,
+ __new_start + __elems_before + __n,
+ _M_get_Tp_allocator());
+ else
+ std::_Destroy(__new_start, __new_finish,
+ _M_get_Tp_allocator());
_M_deallocate(__new_start, __len);
__throw_exception_again;
}
Index: testsuite/23_containers/vector/modifiers/moveable.cc
===================================================================
--- testsuite/23_containers/vector/modifiers/moveable.cc (revision 136250)
+++ testsuite/23_containers/vector/modifiers/moveable.cc (working copy)
@@ -1,6 +1,6 @@
// { dg-options "-std=gnu++0x" }
-// Copyright (C) 2005, 2007 Free Software Foundation, Inc.
+// Copyright (C) 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
//
// This file is part of the GNU ISO C++ Library. This library is free
// software; you can redistribute it and/or modify it under the
@@ -104,11 +104,11 @@
std::vector<copycounter> a(10, c);
copycounter::copycount = 0;
a.insert(a.begin(), 20, c);
- VERIFY(copycounter::copycount == 20 + 1);
+ VERIFY(copycounter::copycount == 20);
a.insert(a.end(), 50, c);
- VERIFY(copycounter::copycount == 70 + 2);
+ VERIFY(copycounter::copycount == 70);
a.insert(a.begin() + 50, 100, c);
- VERIFY(copycounter::copycount == 170 + 3);
+ VERIFY(copycounter::copycount == 170);
}
// Test vector::insert(iterator, count, value) makes no unneeded copies
Index: testsuite/23_containers/vector/resize/moveable.cc
===================================================================
--- testsuite/23_containers/vector/resize/moveable.cc (revision 136250)
+++ testsuite/23_containers/vector/resize/moveable.cc (working copy)
@@ -1,6 +1,6 @@
// { dg-options "-std=gnu++0x" }
-// Copyright (C) 2005, 2006, 2007 Free Software Foundation, Inc.
+// Copyright (C) 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
//
// This file is part of the GNU ISO C++ Library. This library is free
// software; you can redistribute it and/or modify it under the
@@ -51,28 +51,28 @@
a.resize(99);
a.resize(100);
#ifndef _GLIBCXX_DEBUG
- VERIFY( copycounter::copycount == 100 + 4 );
+ VERIFY( copycounter::copycount == 100 + 1 );
#else
- VERIFY( copycounter::copycount == 100 + 4 + 4 );
+ VERIFY( copycounter::copycount == 100 + 1 + 4 );
#endif
a.resize(99);
a.resize(0);
#ifndef _GLIBCXX_DEBUG
- VERIFY( copycounter::copycount == 100 + 4 );
+ VERIFY( copycounter::copycount == 100 + 1 );
#else
- VERIFY( copycounter::copycount == 100 + 4 + 6 );
+ VERIFY( copycounter::copycount == 100 + 1 + 6 );
#endif
a.resize(100);
#ifndef _GLIBCXX_DEBUG
- VERIFY( copycounter::copycount == 200 + 5 );
+ VERIFY( copycounter::copycount == 200 + 2 );
#else
- VERIFY( copycounter::copycount == 200 + 5 + 7 );
+ VERIFY( copycounter::copycount == 200 + 2 + 7 );
#endif
a.clear();
#ifndef _GLIBCXX_DEBUG
- VERIFY( copycounter::copycount == 200 + 5 );
+ VERIFY( copycounter::copycount == 200 + 2 );
#else
- VERIFY( copycounter::copycount == 200 + 5 + 7 );
+ VERIFY( copycounter::copycount == 200 + 2 + 7 );
#endif
}