This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ 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]

Fix stable_sort to work on iterators returning rvalue


Hi

The issues I had when trying to use stable_sort with move_iterator was in fact not all coming from a missing swap overload. In fact stable_sort do not accept iterators with dereference operator returning pure rvalue. I use std::vector<bool>::iterator to illustrate this problem. The problem was in fact coming from __uninitialized_construct_buf that was taking a lvalue reference. I simply modify the function and the underlying helper struct to dereference the iterator as late as possible that is to say only when we need to pass it to the std::move function.

2012-05-25 François Dumont <fdumont@gcc.gnu.org>

    * include/bits/stl_tempbuf.h (__uninitialized_construct_buf)
    (__uninitialized_construct_buf_dispatch<>::__ucr): Fix to work
    with iterator returning rvalue.
    * testsuite/25_algorithms/stable_sort/3.cc: New.

Tested under x86_64 linux with make check and make CXXFLAGS=-std=c++11 check.

Ok to commit ?

François
Index: include/bits/stl_tempbuf.h
===================================================================
--- include/bits/stl_tempbuf.h	(revision 187550)
+++ include/bits/stl_tempbuf.h	(working copy)
@@ -1,7 +1,7 @@
 // Temporary buffer implementation -*- C++ -*-
 
 // Copyright (C) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009,
-// 2010, 2011
+// 2010, 2011, 2012
 // Free Software Foundation, Inc.
 //
 // This file is part of the GNU ISO C++ Library.  This library is free
@@ -182,25 +182,25 @@
   template<bool>
     struct __uninitialized_construct_buf_dispatch
     {
-      template<typename _ForwardIterator, typename _Tp>
+      template<typename _Pointer, typename _ForwardIterator>
         static void
-        __ucr(_ForwardIterator __first, _ForwardIterator __last,
-	      _Tp& __value)
+        __ucr(_Pointer __first, _Pointer __last,
+	      _ForwardIterator __seed)
         {
 	  if(__first == __last)
 	    return;
 
-	  _ForwardIterator __cur = __first;
+	  _Pointer __cur = __first;
 	  __try
 	    {
 	      std::_Construct(std::__addressof(*__first),
-			      _GLIBCXX_MOVE(__value));
-	      _ForwardIterator __prev = __cur;
+			      _GLIBCXX_MOVE(*__seed));
+	      _Pointer __prev = __cur;
 	      ++__cur;
 	      for(; __cur != __last; ++__cur, ++__prev)
 		std::_Construct(std::__addressof(*__cur),
 				_GLIBCXX_MOVE(*__prev));
-	      __value = _GLIBCXX_MOVE(*__prev);
+	      *__seed = _GLIBCXX_MOVE(*__prev);
 	    }
 	  __catch(...)
 	    {
@@ -213,9 +213,9 @@
   template<>
     struct __uninitialized_construct_buf_dispatch<true>
     {
-      template<typename _ForwardIterator, typename _Tp>
+      template<typename _Pointer, typename _ForwardIterator>
         static void
-        __ucr(_ForwardIterator, _ForwardIterator, _Tp&) { }
+        __ucr(_Pointer, _Pointer, _ForwardIterator) { }
     };
 
   // Constructs objects in the range [first, last).
@@ -223,23 +223,22 @@
   // their exact value is not defined. In particular they may
   // be 'moved from'.
   //
-  // While __value may altered during this algorithm, it will have
+  // While *__seed may be altered during this algorithm, it will have
   // the same value when the algorithm finishes, unless one of the
   // constructions throws.
   //
-  // Requirements: _ForwardIterator::value_type(_Tp&&) is valid.
-  template<typename _ForwardIterator, typename _Tp>
+  // Requirements: _Pointer::value_type(_Tp&&) is valid.
+  template<typename _Pointer, typename _ForwardIterator>
     inline void
-    __uninitialized_construct_buf(_ForwardIterator __first,
-				  _ForwardIterator __last,
-				  _Tp& __value)
+    __uninitialized_construct_buf(_Pointer __first, _Pointer __last,
+				  _ForwardIterator __seed)
     {
-      typedef typename std::iterator_traits<_ForwardIterator>::value_type
+      typedef typename std::iterator_traits<_Pointer>::value_type
 	_ValueType;
 
       std::__uninitialized_construct_buf_dispatch<
         __has_trivial_constructor(_ValueType)>::
-	  __ucr(__first, __last, __value);
+	  __ucr(__first, __last, __seed);
     }
 
   template<typename _ForwardIterator, typename _Tp>
@@ -254,9 +253,9 @@
 					    value_type>(_M_original_len));
 	  _M_buffer = __p.first;
 	  _M_len = __p.second;
-	  if(_M_buffer)
+	  if (_M_buffer)
 	    std::__uninitialized_construct_buf(_M_buffer, _M_buffer + _M_len,
-					       *__first);
+					       __first);
 	}
       __catch(...)
 	{
Index: testsuite/25_algorithms/stable_sort/3.cc
===================================================================
--- testsuite/25_algorithms/stable_sort/3.cc	(revision 0)
+++ testsuite/25_algorithms/stable_sort/3.cc	(revision 0)
@@ -0,0 +1,40 @@
+// Copyright (C) 2012 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/>.
+
+// 25.3.1.2 [lib.stable.sort]
+
+#include <vector>
+#include <algorithm>
+#include <testsuite_hooks.h>
+
+void 
+test1()
+{
+  std::vector<bool> bools;
+  bools.push_back(true);
+  bools.push_back(false);
+  bools.push_back(true);
+  bools.push_back(false);
+  std::stable_sort(bools.begin(), bools.end());
+  VERIFY( !bools[0] && !bools[1] && bools[2] && bools[3] );
+}
+
+int 
+main()
+{
+  test1();
+}

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