This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Fix stable_sort to work on iterators returning rvalue
- From: François Dumont <frs dot dumont at gmail dot com>
- To: "libstdc++ at gcc dot gnu dot org" <libstdc++ at gcc dot gnu dot org>
- Date: Fri, 25 May 2012 22:42:03 +0200
- Subject: 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();
+}