diff mbox

[v3] LWG 2106: move_iterator::reference

Message ID alpine.DEB.2.02.1403051948100.28958@stedding.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse March 5, 2014, 7:36 p.m. UTC
Hello,

this issue got delayed in LWG, apparently because of a failed 
"improvement" to the wording along the way (happens, that's ok), but there 
seems to be a consensus on the resolution and I don't really see the point 
of waiting (it changes code that currently returns a reference to a 
temporary).

Tested on x86_64-linux-gnu. Stage 1?

2014-04-01  Marc Glisse  <marc.glisse@inria.fr>

 	PR libstdc++/59434
 	* include/bits/stl_iterator.h (move_iterator::reference,
 	move_iterator::operator*): Implement LWG 2106.
 	* testsuite/24_iterators/move_iterator/dr2106.cc: New file.

Comments

Jonathan Wakely March 5, 2014, 8:42 p.m. UTC | #1
On 5 March 2014 19:36, Marc Glisse wrote:
> Hello,
>
> this issue got delayed in LWG, apparently because of a failed "improvement"
> to the wording along the way (happens, that's ok), but there seems to be a
> consensus on the resolution and I don't really see the point of waiting (it
> changes code that currently returns a reference to a temporary).
>
> Tested on x86_64-linux-gnu. Stage 1?

Yes, OK for Stage 1.  Please put _GLIBCXX_RESOLVE_DEFECTS (or whatever
it is we use elsewhere) in the comment, rather than just "DR 2106". I
think the point of that is to be able to grep for all DR fixes
(especially ones that aren't actually accepted as defects yet :-)

Thanks.
Marc Glisse March 5, 2014, 9:35 p.m. UTC | #2
On Wed, 5 Mar 2014, Jonathan Wakely wrote:

> Please put _GLIBCXX_RESOLVE_DEFECTS (or whatever
> it is we use elsewhere) in the comment, rather than just "DR 2106". I
> think the point of that is to be able to grep for all DR fixes
> (especially ones that aren't actually accepted as defects yet :-)

Believe it or not, I did look at other occurences in the code, and didn't 
hit any that used such a keyword. Now that I know to look for it, there is 
indeed _GLIBCXX_RESOLVE_LIB_DEFECTS (I'll add it), but it is missing in a 
lot of places.
Jonathan Wakely March 5, 2014, 9:47 p.m. UTC | #3
On 5 March 2014 21:35, Marc Glisse wrote:
> On Wed, 5 Mar 2014, Jonathan Wakely wrote:
>
>> Please put _GLIBCXX_RESOLVE_DEFECTS (or whatever
>> it is we use elsewhere) in the comment, rather than just "DR 2106". I
>> think the point of that is to be able to grep for all DR fixes
>> (especially ones that aren't actually accepted as defects yet :-)
>
>
> Believe it or not, I did look at other occurences in the code, and didn't
> hit any that used such a keyword. Now that I know to look for it, there is
> indeed _GLIBCXX_RESOLVE_LIB_DEFECTS (I'll add it), but it is missing in a
> lot of places.

That's the one. I know I've not always been consistent about adding
it, but I try to when I remember.
Paolo Carlini March 6, 2014, 9:35 a.m. UTC | #4
Hi,

On 03/05/2014 10:47 PM, Jonathan Wakely wrote:
> On 5 March 2014 21:35, Marc Glisse wrote:
>> On Wed, 5 Mar 2014, Jonathan Wakely wrote:
>>
>>> Please put _GLIBCXX_RESOLVE_DEFECTS (or whatever
>>> it is we use elsewhere) in the comment, rather than just "DR 2106". I
>>> think the point of that is to be able to grep for all DR fixes
>>> (especially ones that aren't actually accepted as defects yet :-)
>>
>> Believe it or not, I did look at other occurences in the code, and didn't
>> hit any that used such a keyword. Now that I know to look for it, there is
>> indeed _GLIBCXX_RESOLVE_LIB_DEFECTS (I'll add it), but it is missing in a
>> lot of places.
> That's the one. I know I've not always been consistent about adding
> it, but I try to when I remember.
Agreed. I remember being sloppy about issues not relative to a released 
Standard. Well, in fact we also used to keep

     http://gcc.gnu.org/onlinedocs/libstdc++/manual/bugs.html

up to date...

Paolo.
Jonathan Wakely March 6, 2014, 9:38 a.m. UTC | #5
On 6 March 2014 09:35, Paolo Carlini wrote:
>
> Agreed. I remember being sloppy about issues not relative to a released
> Standard.

Yes, if it's just a change from one draft to another then it's just
"work in progress" and  not important to record that we've applied a
resolution.

> Well, in fact we also used to keep
>
>     http://gcc.gnu.org/onlinedocs/libstdc++/manual/bugs.html
>
> up to date...

Oh yes, I've definitely not been doing that!
Marc Glisse April 11, 2014, 7:28 p.m. UTC | #6
On Wed, 5 Mar 2014, Jonathan Wakely wrote:

> On 5 March 2014 19:36, Marc Glisse wrote:
>> Hello,
>>
>> this issue got delayed in LWG, apparently because of a failed "improvement"
>> to the wording along the way (happens, that's ok), but there seems to be a
>> consensus on the resolution and I don't really see the point of waiting (it
>> changes code that currently returns a reference to a temporary).
>>
>> Tested on x86_64-linux-gnu. Stage 1?
>
> Yes, OK for Stage 1.  Please put _GLIBCXX_RESOLVE_DEFECTS (or whatever
> it is we use elsewhere) in the comment, rather than just "DR 2106".

r209323
(again, only posting because this was accepted long ago)
diff mbox

Patch

Index: libstdc++-v3/include/bits/stl_iterator.h
===================================================================
--- libstdc++-v3/include/bits/stl_iterator.h	(revision 208349)
+++ libstdc++-v3/include/bits/stl_iterator.h	(working copy)
@@ -958,48 +958,52 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
    *  generic algorithms can be called with move iterators to replace
    *  copying with moving.
    */
   template<typename _Iterator>
     class move_iterator
     {
     protected:
       _Iterator _M_current;
 
       typedef iterator_traits<_Iterator>		__traits_type;
+      typedef typename __traits_type::reference		__base_ref;
 
     public:
       typedef _Iterator					iterator_type;
       typedef typename __traits_type::iterator_category iterator_category;
       typedef typename __traits_type::value_type  	value_type;
       typedef typename __traits_type::difference_type	difference_type;
       // NB: DR 680.
       typedef _Iterator					pointer;
-      typedef value_type&&				reference;
+      // DR 2106.
+      typedef typename conditional<is_reference<__base_ref>::value,
+			 typename remove_reference<__base_ref>::type&&,
+			 __base_ref>::type		reference;
 
       move_iterator()
       : _M_current() { }
 
       explicit
       move_iterator(iterator_type __i)
       : _M_current(__i) { }
 
       template<typename _Iter>
 	move_iterator(const move_iterator<_Iter>& __i)
 	: _M_current(__i.base()) { }
 
       iterator_type
       base() const
       { return _M_current; }
 
       reference
       operator*() const
-      { return std::move(*_M_current); }
+      { return static_cast<reference>(*_M_current); }
 
       pointer
       operator->() const
       { return _M_current; }
 
       move_iterator&
       operator++()
       {
 	++_M_current;
 	return *this;
Index: libstdc++-v3/testsuite/24_iterators/move_iterator/dr2106.cc
===================================================================
--- libstdc++-v3/testsuite/24_iterators/move_iterator/dr2106.cc	(revision 0)
+++ libstdc++-v3/testsuite/24_iterators/move_iterator/dr2106.cc	(working copy)
@@ -0,0 +1,33 @@ 
+// { dg-options "-std=gnu++11" }
+// { dg-do compile }
+
+// Copyright (C) 2014 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/>.
+
+#include <iterator>
+#include <type_traits>
+#include <vector>
+
+typedef std::vector<bool>	Vec;
+typedef Vec::reference		Ref;
+typedef Vec::const_reference	CRef;
+typedef Vec::iterator		It;
+typedef Vec::const_iterator	CIt;
+typedef std::move_iterator<It>	MIt;
+typedef std::move_iterator<CIt>	MCIt;
+static_assert(std::is_same<MIt::reference, Ref>::value,"");
+static_assert(std::is_same<MCIt::reference, CRef>::value,"");