diff mbox

Improve insert/emplace robustness to self insertion

Message ID 20160708163842.GC2957@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely July 8, 2016, 4:38 p.m. UTC 
On 06/07/16 21:46 +0200, François Dumont wrote:
>Don't you plan to add it to the testsuite ?

Done with the attached aptch.

>On my side I rebase part of my patch to reorganize a little bit code. 
>I reintroduced _M_realloc_insert which isolates the code of 
>_M_insert_aux used when we need to reallocate memory. So _M_insert_aux 
>is used only when insertion can be done in place. It is a nice 
>replacement for _M_emplace_back_aux that have been removed. In most of 
>vector modifiers we start checking if we need to reallocate or not. 
>With this reorganization we don't check it several times. Moreover, as 
>soon as we reallocate we know that we don't need to do any temporary 
>copy so insert_vs_emplace.cc test04 has been adapted and we now have 
>no situation where emplace and insert are not equivalent.
>
>    * include/bits/stl_vector.h (push_back(const value_type&)): Forward
>    to _M_realloc_insert.
>    (insert(const_iterator, value_type&&)): Forward to _M_insert_rval.
>    (_M_realloc_insert): Declare new function.
>    (_M_emplace_back_aux): Remove definition.
>    * include/bits/vector.tcc (emplace_back(_Args...)):
>    Use _M_realloc_insert.
>    (insert(const_iterator, const value_type&)): Likewise.
>    (_M_insert_rval, _M_emplace_aux): Likewise.
>    (_M_emplace_back_aux): Remove declaration.
>    (_M_realloc_insert): Define.
>    * testsuite/23_containers/vector/modifiers/insert_vs_emplace.cc:
>    Adjust expected results for emplacing an lvalue with reallocation.
>
>Tested under Linux x86_64.
>
>Ok to commit ?

This is excellent work, thanks for doing it.

OK for trunk.
diff mbox

Patch

commit dd91c89f43bb79bc4e206824341536a234542c64
Author: redi <redi@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Fri Jul 8 16:35:10 2016 +0000

    	* testsuite/23_containers/vector/modifiers/insert/aliasing.cc: New.
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@238169 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/aliasing.cc b/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/aliasing.cc
new file mode 100644
index 0000000..2ef13b4
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/modifiers/insert/aliasing.cc
@@ -0,0 +1,79 @@ 
+// Copyright (C) 2016 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
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++14" }
+
+#include <vector>
+#include <memory>
+#include <testsuite_hooks.h>
+
+// See https://gcc.gnu.org/ml/libstdc++/2016-07/msg00008.html for background.
+
+struct T
+{
+ T(int v = 0) : value(v) { }
+ T(const T& t);
+ T& operator=(const T& t);
+ void make_child() { child = std::make_unique<T>(value + 10); }
+ std::unique_ptr<T> child;
+ int value;
+};
+
+T::T(const T& t) : value(t.value)
+{
+ if (t.child)
+   child.reset(new T(*t.child));
+}
+
+T& T::operator=(const T& t)
+{
+ value = t.value;
+ if (t.child)
+ {
+   if (child)
+     *child = *t.child;
+   else
+     child.reset(new T(*t.child));
+ }
+ else
+   child.reset();
+ return *this;
+}
+
+void
+test01()
+{
+ std::vector<T> v;
+ v.reserve(3);
+ v.push_back(T(1));
+ v.back().make_child();
+ v.push_back(T(2));
+ v.back().make_child();
+
+ VERIFY(v[1].child->value == 12);
+ VERIFY(v[1].child->child == nullptr);
+
+ v.insert(v.begin(), *v[1].child);
+
+ VERIFY(v[0].value == 12);
+ VERIFY(v[0].child == nullptr);
+}
+
+int main()
+{
+  test01();
+}