On Tue, 4 Feb 2020, Jonathan Wakely wrote: > On 03/02/20 21:07 -0500, Patrick Palka wrote: > > These changes are needed for some of the tests in the constrained algorithm > > patch, because they use move_iterator with an uncopyable output_iterator. > > The > > other changes described in the paper are already applied, it seems. > > > > libstdc++-v3/ChangeLog: > > > > * include/bits/stl_iterator.h (move_iterator::move_iterator): Move the > > iterator when initializing _M_current. > > (move_iterator::base): Split into two overloads differing in > > ref-qualifiers as in P1207R4. > > --- > > libstdc++-v3/include/bits/stl_iterator.h | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/libstdc++-v3/include/bits/stl_iterator.h > > b/libstdc++-v3/include/bits/stl_iterator.h > > index 784d200d22f..1a288a5c785 100644 > > --- a/libstdc++-v3/include/bits/stl_iterator.h > > +++ b/libstdc++-v3/include/bits/stl_iterator.h > > @@ -1166,7 +1166,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > > > explicit _GLIBCXX17_CONSTEXPR > > move_iterator(iterator_type __i) > > - : _M_current(__i) { } > > + : _M_current(std::move(__i)) { } > > > > template<typename _Iter> > > _GLIBCXX17_CONSTEXPR > > @@ -1174,9 +1174,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > : _M_current(__i.base()) { } > > > > _GLIBCXX17_CONSTEXPR iterator_type > > - base() const > > + base() const & > > +#if __cplusplus > 201703L && __cpp_lib_concepts > > + requires copy_constructible<iterator_type> > > +#endif > > { return _M_current; } > > > > + _GLIBCXX17_CONSTEXPR iterator_type > > + base() && > > + { return std::move(_M_current); } > > + > > I think this change has to be restricted to C++20 and later, otherwise > a move_iterator in C++17 can change state so that its _M_current > becomes moved-from, which should not be possible before C++20. > > So something like: > > #if __cplusplus <= 201703L > _GLIBCXX17_CONSTEXPR iterator_type > base() const > { return _M_current; } > #else > constexpr iterator_type > base() const & > #if __cpp_lib_concepts > requires copy_constructible<iterator_type> > #endif > { return _M_current; } > > constexpr iterator_type > base() && > { return std::move(_M_current); } > #endif > >
Thanks for the review, here is the updated patch. libstdc++-v3/ChangeLog: * include/bits/stl_iterator.h (move_iterator::move_iterator): Move __i when initializing _M_current. (move_iterator::base): Split into two overloads differing in ref-qualifiers as in P1207R4 for C++20. --- libstdc++-v3/include/bits/stl_iterator.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h index 784d200d22f..c200f7a9d14 100644 --- a/libstdc++-v3/include/bits/stl_iterator.h +++ b/libstdc++-v3/include/bits/stl_iterator.h @@ -1166,16 +1166,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION explicit _GLIBCXX17_CONSTEXPR move_iterator(iterator_type __i) - : _M_current(__i) { } + : _M_current(std::move(__i)) { } template<typename _Iter> _GLIBCXX17_CONSTEXPR move_iterator(const move_iterator<_Iter>& __i) : _M_current(__i.base()) { } +#if __cplusplus <= 201703L _GLIBCXX17_CONSTEXPR iterator_type base() const { return _M_current; } +#else + constexpr iterator_type + base() const & +#if __cpp_lib_concepts + requires copy_constructible<iterator_type> +#endif + { return _M_current; } + + constexpr iterator_type + base() && + { return std::move(_M_current); } +#endif _GLIBCXX17_CONSTEXPR reference operator*() const -- 2.25.0.114.g5b0ca878e0