Message ID | alpine.DEB.2.02.1403051948100.28958@stedding.saclay.inria.fr |
---|---|
State | New |
Headers | show |
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.
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.
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.
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.
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!
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)
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,"");