On 30/08/16 11:17 +0100, Jonathan Wakely wrote:
On 27/08/16 21:29 +0200, François Dumont wrote:
On 23/08/2016 15:17, Jonathan Wakely wrote:
This fixes move assignment of RB trees to tag dispatch so the branch
that copies nodes doesn't need to be well-formed.

  PR libstdc++/77334
  * include/bits/stl_tree.h (_Rb_tree::_M_move_assign): New functions.
  (_Rb_tree::operator=(_Rb_tree&&)): Dispatch to _M_move_assign.
  * testsuite/23_containers/map/77334.cc: New test.

Tested powerpc64le-linux, committed to trunk.

Backports to 5 and 6 to follow.

+    operator=(_Rb_tree&& __x)
+    noexcept(_Alloc_traits::_S_nothrow_move()
+            && is_nothrow_move_assignable<_Compare>::value)
+    {
+      _M_impl._M_key_compare = __x._M_impl._M_key_compare;


Shouldn't it be:

+      _M_impl._M_key_compare = std::move(__x._M_impl._M_key_compare);

I can't find anything in Standard saying to do so but wouldn't it be more 
natural ?

Yes, I can't think of any reason that would cause a problem. The
moved-from container is empty after the move assignment so even if
moving the comparison object changes the order it defines, no elements
will be left in an invalid order.

We don't need to use the comparison object after it's moved, but I'll
add a test to guarantee that remains true.

Done with this patch.

Tested powerpc64le-linux, committed to trunk.

commit 308f80881e5368cc16aa32e3c5c65f341f981329
Author: Jonathan Wakely <jwak...@redhat.com>
Date:   Tue Aug 30 12:14:29 2016 +0100

    Move comparison object in map/set move assignment
    
    	* include/bits/stl_tree.h (_Rb_tree::operator=(_Rb_tree&&)): Move
    	comparison object.
    	* testsuite/23_containers/set/move_comparison.cc: New test.

diff --git a/libstdc++-v3/include/bits/stl_tree.h b/libstdc++-v3/include/bits/stl_tree.h
index 25580e4..02b00b0 100644
--- a/libstdc++-v3/include/bits/stl_tree.h
+++ b/libstdc++-v3/include/bits/stl_tree.h
@@ -1436,7 +1436,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     noexcept(_Alloc_traits::_S_nothrow_move()
 	     && is_nothrow_move_assignable<_Compare>::value)
     {
-      _M_impl._M_key_compare = __x._M_impl._M_key_compare;
+      _M_impl._M_key_compare = std::move(__x._M_impl._M_key_compare);
       constexpr bool __move_storage =
 	  _Alloc_traits::_S_propagate_on_move_assign()
 	  || _Alloc_traits::_S_always_equal();
diff --git a/libstdc++-v3/testsuite/23_containers/set/move_comparison.cc b/libstdc++-v3/testsuite/23_containers/set/move_comparison.cc
new file mode 100644
index 0000000..317821f
--- /dev/null
+++ b/libstdc++-v3/testsuite/23_containers/set/move_comparison.cc
@@ -0,0 +1,58 @@
+// 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-do run { target c++11 } }
+
+#include <set>
+#include <testsuite_allocator.h>
+#include <testsuite_hooks.h>
+
+struct Cmp
+{
+  Cmp() = default;
+  Cmp(const Cmp&) = default;
+  Cmp(Cmp&&) = default;
+  Cmp& operator=(const Cmp&) = default;
+  Cmp& operator=(Cmp&& c) { c.moved_from = true; return *this; }
+
+  bool moved_from = false;
+
+  bool operator()(int l, int r) const
+  {
+    VERIFY(!moved_from);
+    return l < r;
+  }
+};
+
+void
+test01()
+{
+  using allocator = __gnu_test::uneq_allocator<int>;
+  using test_type = std::set<int, Cmp, allocator>;
+  test_type s1(allocator(1)), s2(allocator(2));
+  s1.insert(1);
+  s1.insert(2);
+  s1.insert(3);
+  s2 = std::move(s1);
+  VERIFY(s1.key_comp().moved_from);
+}
+
+int
+main()
+{
+  test01();
+}

Reply via email to