[PATCH 1/3] libstdc++: Apply the move_iterator changes described in P1207R4

[Patrick Palka]

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