This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Relax std::move_if_noexcept for std::pair


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

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;
 }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]