Relax std::move_if_noexcept for std::pair

["François Dumont" <frs.dumont@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]

Hi

     I eventually find out what was the problem with the 
std::move_if_noexcept within associative containers.

     The std::pair move default constructor might not move both first 
and second member. If any is not moveable it will just copy it. And then 
the noexcept qualification of the copy constructor will participate in 
the noexcept qualification of the std::pair move constructor. So 
std::move_if_noexcept can eventually decide to not use move because a 
_copy_ constructor not noexcept qualified.

     This is why I am partially specializing __move_if_noexcept_cond. As 
there doesn't seem to exist any Standard meta function to find out if 
move will take place I resort using std::is_const as in this case for 
sure the compiler won't call the move constructor.

     Note that I find __move_if_noexcept_cond very counter-intuitive 
cause it says if the move semantic should _not_ be used.

     I am submitting this now cause it might be consider as a bug even 
if the end result is just that it pessimizes the occasion to use move 
semantic rather than copy.

     * include/bits/stl_pair.h (__move_if_noexcept_cond<pair<>>): New
     partial specialization.
     * testsuite/20_util/move_if_noexcept/1.cc (test02): New.
     * testsuite/23_containers/unordered_map/allocator/move_assign.cc
     (test03): New.

     Tested under Linux x86_64 normal mode.

François


[-- Attachment #2: pair_move_if_noexcept.patch --]
[-- Type: text/x-patch, Size: 5278 bytes --]

diff --git a/libstdc++-v3/include/bits/stl_pair.h b/libstdc++-v3/include/bits/stl_pair.h
index 48af2b02ef9..85aad838860 100644
--- a/libstdc++-v3/include/bits/stl_pair.h
+++ b/libstdc++-v3/include/bits/stl_pair.h
@@ -528,6 +528,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       typedef pair<__ds_type1, __ds_type2> 	      __pair_type;
       return __pair_type(std::forward<_T1>(__x), std::forward<_T2>(__y));
     }
+
+  template<typename _T1, typename _T2>
+    struct __move_if_noexcept_cond<pair<_T1, _T2>>
+    : public __and_<is_copy_constructible<pair<_T1, _T2>>,
+		    __not_<__and_<
+	       __or_<is_const<_T1>, is_nothrow_move_constructible<_T1>>,
+	       __or_<is_const<_T2>, is_nothrow_move_constructible<_T2>>>>>::type
+  { };
 #else
   template<typename _T1, typename _T2>
     inline pair<_T1, _T2>
diff --git a/libstdc++-v3/testsuite/20_util/move_if_noexcept/1.cc b/libstdc++-v3/testsuite/20_util/move_if_noexcept/1.cc
index 078ccb83d36..b6f01097e40 100644
--- a/libstdc++-v3/testsuite/20_util/move_if_noexcept/1.cc
+++ b/libstdc++-v3/testsuite/20_util/move_if_noexcept/1.cc
@@ -33,7 +33,7 @@ struct noexcept_move_copy
 
   noexcept_move_copy(const noexcept_move_copy&) = default;
 
-  operator bool() { return status; }
+  operator bool() const { return status; }
 
 private:
   bool status;
@@ -50,7 +50,7 @@ struct noexcept_move_no_copy
 
   noexcept_move_no_copy(const noexcept_move_no_copy&) = delete;
 
-  operator bool() { return status; }
+  operator bool() const { return status; }
 
 private:
   bool status;
@@ -67,7 +67,7 @@ struct except_move_copy
 
   except_move_copy(const except_move_copy&) = default;
 
-  operator bool() { return status; }
+  operator bool() const { return status; }
 
 private:
   bool status;
@@ -84,7 +84,7 @@ struct except_move_no_copy
 
   except_move_no_copy(const except_move_no_copy&) = delete;
 
-  operator bool() { return status; }
+  operator bool() const { return status; }
 
 private:
   bool status;
@@ -110,8 +110,38 @@ test01()
   VERIFY( emnc1 == false );
 }
 
+void
+test02()
+{
+  std::pair<noexcept_move_copy, noexcept_move_copy> nemc1;
+  auto nemc2 __attribute__((unused)) = std::move_if_noexcept(nemc1);
+  VERIFY( nemc1.first == false );
+  VERIFY( nemc1.second == false );
+
+  std::pair<except_move_copy, noexcept_move_copy> emc1;
+  auto emc2 __attribute__((unused)) = std::move_if_noexcept(emc1);
+  VERIFY( emc1.first == true );
+  VERIFY( emc1.second == true );
+
+  std::pair<except_move_no_copy, noexcept_move_copy> emnc1;
+  auto emnc2 __attribute__((unused)) = std::move_if_noexcept(emnc1);
+  VERIFY( emnc1.first == false );
+  VERIFY( emnc1.second == false );
+
+  std::pair<const except_move_copy, noexcept_move_copy> cemc1;
+  auto cemc2 __attribute__((unused)) = std::move_if_noexcept(cemc1);
+  VERIFY( cemc1.first == true );
+  VERIFY( cemc1.second == false );
+
+  std::pair<noexcept_move_no_copy, noexcept_move_copy> nemnc1;
+  auto nemnc2 __attribute__((unused)) = std::move_if_noexcept(nemnc1);
+  VERIFY( nemnc1.first == false );
+  VERIFY( nemnc1.second == false );
+}
+
 int main()
 {
   test01();
+  test02();
   return 0;
 }
diff --git a/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/move_assign.cc b/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/move_assign.cc
index b27269e607a..d1be3adaae5 100644
--- a/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/move_assign.cc
+++ b/libstdc++-v3/testsuite/23_containers/unordered_map/allocator/move_assign.cc
@@ -21,6 +21,7 @@
 #include <testsuite_hooks.h>
 #include <testsuite_allocator.h>
 #include <testsuite_counter_type.h>
+#include <testsuite_rvalref.h>
 
 using __gnu_test::propagating_allocator;
 using __gnu_test::counter_type;
@@ -49,8 +50,10 @@ void test01()
   VERIFY( 1 == v1.get_allocator().get_personality() );
   VERIFY( 2 == v2.get_allocator().get_personality() );
 
-  // No move because key is const.
-  VERIFY( counter_type::move_assign_count == 0  );
+  // Key copied, value moved.
+  VERIFY( counter_type::copy_count == 1  );
+  VERIFY( counter_type::move_count == 1  );
+  VERIFY( counter_type::destructor_count == 4 );
 }
 
 void test02()
@@ -79,15 +82,41 @@ void test02()
   VERIFY(0 == v1.get_allocator().get_personality());
   VERIFY(1 == v2.get_allocator().get_personality());
 
-  VERIFY( counter_type::move_assign_count == 0 );
+  VERIFY( counter_type::copy_count == 0  );
+  VERIFY( counter_type::move_count == 0  );
   VERIFY( counter_type::destructor_count == 2 );
 
   VERIFY( it == v2.begin() );
 }
 
+void test03()
+{
+  using __gnu_test::rvalstruct;
+
+  typedef std::pair<const int, rvalstruct> value_type;
+  typedef propagating_allocator<value_type, false> alloc_type;
+  typedef std::unordered_map<int, rvalstruct, std::hash<int>,
+			     std::equal_to<int>,
+			     alloc_type> test_type;
+
+  test_type v1(alloc_type(1));
+  v1.emplace(std::piecewise_construct,
+	     std::make_tuple(1), std::make_tuple(1));
+
+  test_type v2(alloc_type(2));
+  v2.emplace(std::piecewise_construct,
+	     std::make_tuple(2), std::make_tuple(2));
+
+  v2 = std::move(v1);
+
+  VERIFY( 1 == v1.get_allocator().get_personality() );
+  VERIFY( 2 == v2.get_allocator().get_personality() );
+}
+
 int main()
 {
   test01();
   test02();
+  test03();
   return 0;
 }