PR libstdc++/92124 fix incorrect container move assignment
diff mbox series

Message ID 20191017142120.GA32212@redhat.com
State New
Headers show
Series
  • PR libstdc++/92124 fix incorrect container move assignment
Related show

Commit Message

Jonathan Wakely Oct. 17, 2019, 2:21 p.m. UTC
The container requirements say that for move assignment "All existing
elements of [the target] are either move assigned or destroyed". Some of
our containers currently use __make_move_if_noexcept which makes the
move depend on whether the element type is nothrow move constructible.
This is incorrect, because the standard says we must move assign, not
move or copy depending on the move constructor.

Use make_move_iterator instead so that we move unconditionally. This
ensures existing elements won't be copy assigned.

	PR libstdc++/92124
	* include/bits/forward_list.h
	(_M_move_assign(forward_list&&, false_type)): Do not use
	__make_move_if_noexcept, instead move unconditionally.
	* include/bits/stl_deque.h (_M_move_assign2(deque&&, false_type)):
	Likewise.
	* include/bits/stl_list.h (_M_move_assign(list&&, false_type)):
	Likewise.
	* include/bits/stl_vector.h (_M_move_assign(vector&&, false_type)):
	Likewise.
	* testsuite/23_containers/vector/92124.cc: New test.

Tested x86_64-linux, committed to trunk.
commit 66f4aa35299bb8e967aa54930b27815cf8161693
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Oct 17 14:27:53 2019 +0100

    PR libstdc++/92124 fix incorrect container move assignment
    
    The container requirements say that for move assignment "All existing
    elements of [the target] are either move assigned or destroyed". Some of
    our containers currently use __make_move_if_noexcept which makes the
    move depend on whether the element type is nothrow move constructible.
    This is incorrect, because the standard says we must move assign, not
    move or copy depending on the move constructor.
    
    Use make_move_iterator instead so that we move unconditionally. This
    ensures existing elements won't be copy assigned.
    
            PR libstdc++/92124
            * include/bits/forward_list.h
            (_M_move_assign(forward_list&&, false_type)): Do not use
            __make_move_if_noexcept, instead move unconditionally.
            * include/bits/stl_deque.h (_M_move_assign2(deque&&, false_type)):
            Likewise.
            * include/bits/stl_list.h (_M_move_assign(list&&, false_type)):
            Likewise.
            * include/bits/stl_vector.h (_M_move_assign(vector&&, false_type)):
            Likewise.
            * testsuite/23_containers/vector/92124.cc: New test.

Patch
diff mbox series

diff --git a/libstdc++-v3/include/bits/forward_list.h b/libstdc++-v3/include/bits/forward_list.h
index e686283a432..cab2ae788a7 100644
--- a/libstdc++-v3/include/bits/forward_list.h
+++ b/libstdc++-v3/include/bits/forward_list.h
@@ -1336,8 +1336,8 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	else
 	  // The rvalue's allocator cannot be moved, or is not equal,
 	  // so we need to individually move each element.
-	  this->assign(std::__make_move_if_noexcept_iterator(__list.begin()),
-		       std::__make_move_if_noexcept_iterator(__list.end()));
+	  this->assign(std::make_move_iterator(__list.begin()),
+		       std::make_move_iterator(__list.end()));
       }
 
       // Called by assign(_InputIterator, _InputIterator) if _Tp is
diff --git a/libstdc++-v3/include/bits/stl_deque.h b/libstdc++-v3/include/bits/stl_deque.h
index ac76d681ff0..50491e76ff5 100644
--- a/libstdc++-v3/include/bits/stl_deque.h
+++ b/libstdc++-v3/include/bits/stl_deque.h
@@ -2256,8 +2256,8 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  {
 	    // The rvalue's allocator cannot be moved and is not equal,
 	    // so we need to individually move each element.
-	    _M_assign_aux(std::__make_move_if_noexcept_iterator(__x.begin()),
-			  std::__make_move_if_noexcept_iterator(__x.end()),
+	    _M_assign_aux(std::make_move_iterator(__x.begin()),
+			  std::make_move_iterator(__x.end()),
 			  std::random_access_iterator_tag());
 	    __x.clear();
 	  }
diff --git a/libstdc++-v3/include/bits/stl_list.h b/libstdc++-v3/include/bits/stl_list.h
index 701982538df..328a79851a8 100644
--- a/libstdc++-v3/include/bits/stl_list.h
+++ b/libstdc++-v3/include/bits/stl_list.h
@@ -1957,8 +1957,8 @@  _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	else
 	  // The rvalue's allocator cannot be moved, or is not equal,
 	  // so we need to individually move each element.
-	  _M_assign_dispatch(std::__make_move_if_noexcept_iterator(__x.begin()),
-			     std::__make_move_if_noexcept_iterator(__x.end()),
+	  _M_assign_dispatch(std::make_move_iterator(__x.begin()),
+			     std::make_move_iterator(__x.end()),
 			     __false_type{});
       }
 #endif
diff --git a/libstdc++-v3/include/bits/stl_vector.h b/libstdc++-v3/include/bits/stl_vector.h
index d33e589498a..ff08b266692 100644
--- a/libstdc++-v3/include/bits/stl_vector.h
+++ b/libstdc++-v3/include/bits/stl_vector.h
@@ -1828,8 +1828,9 @@  _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
 	  {
 	    // The rvalue's allocator cannot be moved and is not equal,
 	    // so we need to individually move each element.
-	    this->assign(std::__make_move_if_noexcept_iterator(__x.begin()),
-			 std::__make_move_if_noexcept_iterator(__x.end()));
+	    this->_M_assign_aux(std::make_move_iterator(__x.begin()),
+			        std::make_move_iterator(__x.end()),
+				std::random_access_iterator_tag());
 	    __x.clear();
 	  }
       }
diff --git a/libstdc++-v3/testsuite/23_containers/deque/92124.cc b/libstdc++-v3/testsuite/23_containers/deque/92124.cc
new file mode 100644
index 00000000000..6f8cf5560c1
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/deque/92124.cc
@@ -0,0 +1,49 @@ 
+// Copyright (C) 2019 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-do run { target c++11 } }
+
+#include <deque>
+#include <testsuite_allocator.h>
+
+struct X {
+    X() = default;
+    X(const X&) = default;
+
+    // Move constructor might throw
+    X(X&&) noexcept(false) {}
+
+    // Tracking calls to assignment functions
+    X& operator=(const X&) { throw 1; }
+
+    X& operator=(X&&) noexcept(true) { return *this; }
+};
+
+void
+test01()
+{
+  using A = __gnu_test::propagating_allocator<X, false>;
+  A a1(1), a2(2);
+  std::deque<X, A> v1(1, a1), v2(1, a2);
+  v1 = std::move(v2);
+}
+
+int
+main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/23_containers/forward_list/92124.cc b/libstdc++-v3/testsuite/23_containers/forward_list/92124.cc
new file mode 100644
index 00000000000..52a28073daf
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/forward_list/92124.cc
@@ -0,0 +1,49 @@ 
+// Copyright (C) 2019 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-do run { target c++11 } }
+
+#include <forward_list>
+#include <testsuite_allocator.h>
+
+struct X {
+    X() = default;
+    X(const X&) = default;
+
+    // Move constructor might throw
+    X(X&&) noexcept(false) {}
+
+    // Tracking calls to assignment functions
+    X& operator=(const X&) { throw 1; }
+
+    X& operator=(X&&) noexcept(true) { return *this; }
+};
+
+void
+test01()
+{
+  using A = __gnu_test::propagating_allocator<X, false>;
+  A a1(1), a2(2);
+  std::forward_list<X, A> v1(1, a1), v2(1, a2);
+  v1 = std::move(v2);
+}
+
+int
+main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/23_containers/list/92124.cc b/libstdc++-v3/testsuite/23_containers/list/92124.cc
new file mode 100644
index 00000000000..117cb71201b
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/list/92124.cc
@@ -0,0 +1,49 @@ 
+// Copyright (C) 2019 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-do run { target c++11 } }
+
+#include <list>
+#include <testsuite_allocator.h>
+
+struct X {
+    X() = default;
+    X(const X&) = default;
+
+    // Move constructor might throw
+    X(X&&) noexcept(false) {}
+
+    // Tracking calls to assignment functions
+    X& operator=(const X&) { throw 1; }
+
+    X& operator=(X&&) noexcept(true) { return *this; }
+};
+
+void
+test01()
+{
+  using A = __gnu_test::propagating_allocator<X, false>;
+  A a1(1), a2(2);
+  std::list<X, A> v1(1, a1), v2(1, a2);
+  v1 = std::move(v2);
+}
+
+int
+main()
+{
+  test01();
+}
diff --git a/libstdc++-v3/testsuite/23_containers/vector/92124.cc b/libstdc++-v3/testsuite/23_containers/vector/92124.cc
new file mode 100644
index 00000000000..3cb487d39f4
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/vector/92124.cc
@@ -0,0 +1,49 @@ 
+// Copyright (C) 2019 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-do run { target c++11 } }
+
+#include <vector>
+#include <testsuite_allocator.h>
+
+struct X {
+    X() = default;
+    X(const X&) = default;
+
+    // Move constructor might throw
+    X(X&&) noexcept(false) {}
+
+    // Tracking calls to assignment functions
+    X& operator=(const X&) { throw 1; }
+
+    X& operator=(X&&) noexcept(true) { return *this; }
+};
+
+void
+test01()
+{
+  using A = __gnu_test::propagating_allocator<X, false>;
+  A a1(1), a2(2);
+  std::vector<X, A> v1(1, a1), v2(1, a2);
+  v1 = std::move(v2);
+}
+
+int
+main()
+{
+  test01();
+}