Re: [PATCH] libstdc++: Implement P2259R1 changes [PR95983]

[Jonathan Wakely <jwakely@redhat.com>]

On 19/04/21 22:25 -0400, Patrick Palka via Libstdc++ wrote:
>This implements the wording changes of P2259R1 "Repairing input range
>adaptors and counted_iterator", which resolves LWG 3283, 3289 and 3408.
>
>The wording changes are relatively straightforward, but they require
>some boilerplate to implement: the changes to make a type alias
>"conditionally present" in some iterator class are realized by defining
>a base class template and an appropriately constrained partial
>specialization thereof that contains the type alias, and having the
>iterator class derive from this base class.  Sometimes the relevant
>condition depend on members from the iterator class, but because a
>class is incomplete when instantiating its bases, the constraints on
>the partial specialization can't use anything from the iterator class.
>This patch works around this by hoisting these members out to the
>enclosing scope (e.g. transform_view::_Iterator::_Base is hoisted out
>to transform_view::_Base so that transform_view::__iter_cat can use it).
>
>This patch also implements the proposed resolution of LWG 3291 to rename
>iota_view::iterator_category to iota_view::iterator_concept, which was
>previously problematic due to LWG 3408.
>
>Tested on x86_64-pc-linux-gnu.
>
>libstdc++-v3/ChangeLog:
>
>	PR libstdc++/95983
>	* include/bits/stl_iterator.h (__detail::__move_iter_cat):
>	Define.
>	(move_iterator): Derive from the above in C++20 in order to
>	conditionally define iterator_category as per P2259.
>	(move_iterator::__base_cat): No longer used, so remove.
>	(move_iterator::iterator_category): Remove in C++20.
>	(__detail::__common_iter_use_postfix_proxy): Define.
>	(common_iterator::_Proxy): Rename to ...
>	(common_iterator:__arrow_proxy): ... this.
>	(common_iterator::__postfix_proxy): Define as per P2259.
>	(common_iterator::operator->): Adjust.
>	(common_iterator::operator++): Adjust as per P2259.
>	(iterator_traits<common_iterator>::_S_iter_cat): Define.
>	(iterator_traits<common_iterator>::iterator_category): Change as
>	per P2259.
>	(__detail::__counted_iter_value_type): Define.
>	(__detail::__counted_iter_concept): Define.
>	(__detail::__counted_iter_cat): Define.
>	(counted_iterator): Derive from the above three classes in order
>	to conditionally define value_type, iterator_concept and
>	iterator category respectively as per P2259.
>	(counted_iterator::operator->): Define as per P2259.
>	(incrementable_traits<counted_iterator>): Remove as per P2259.
>	(iterator_traits<counted_iterator>): Adjust as per P2259.
>	* include/std/ranges (__detail::__iota_view_iter_cat): Define.
>	(iota_view::_Iterator): Derive from the above in order to
>	conditionally define iterator_category as per P2259.
>	(iota_view::_S_iter_cat): Rename to ...
>	(iota_view::_S_iter_concept): ... this.
>	(iota_view::iterator_concept): Use it to apply LWG 3291 changes.
>	(iota_view::iterator_category): Remove.
>	(__detail::__filter_view_iter_cat): Define.
>	(filter_view::_Iterator): Derive from the above in order to
>	conditionally define iterator_category as per P2259.
>	(filter_view::_Iterator): Move to struct __filter_view_iter_cat.
>	(filter_view::_Iterator::iterator_category): Remove.
>	(transform_view::_Base): Define.
>	(transform_view::__iter_cat): Define.
>	(transform_view::_Iterator): Derive from the above in order to
>	conditionally define iterator_category as per P2259.
>	(transform_view::_Iterator::_Base): Just alias
>	transform_view::_Base.
>	(transform_view::_Iterator::_S_iter_cat): Move to struct
>	transform_view::__iter_cat.
>	(transform_view::_Iterator::iterator_category): Remove.
>	(transform_view::_Sentinel::_Base): Just alias
>	transform_view::_Base.
>	(join_view::_Base): Define.
>	(join_view::_Outer_iter): Define.
>	(join_view::_Inner_iter): Define.
>	(join_view::_S_ref_is_glvalue): Define.
>	(join_view::__iter_cat): Define.
>	(join_view::_Iterator): Derive from it in order to conditionally
>	define iterator_category as per P2259.
>	(join_view::_Iterator::_Base): Just alias join_view::_Base.
>	(join_view::_Iterator::_S_ref_is_glvalue): Just alias
>	join_view::_S_ref_is_glvalue.
>	(join_view::_Iterator::_S_iter_cat): Move to struct
>	transform_view::__iter_cat.
>	(join_view::_Iterator::_Outer_iter): Just alias
>	join_view::_Outer_iter.
>	(join_view::_Iterator::_Inner_iter): Just alias
>	join_view::_Inner_iter.
>	(join_view::_Iterator::iterator_category): Remove.
>	(join_view::_Sentinel::_Base): Just alias join_view::_Base.
>	(__detail::__split_view_outer_iter_cat): Define.
>	(__detail::__split_view_inner_iter_cat): Define.
>	(split_view::_Base): Define.
>	(split_view::_Outer_iter): Derive from __split_view_outer_iter_cat
>	in order to conditionally define iterator_category as per P2259.
>	(split_view::_Outer_iter::iterator_category): Remove.
>	(split_view::_Inner_iter): Derive from __split_view_inner_iter_cat
>	in order to conditionally define iterator_category as per P2259.
>	(split_view::_Inner_iter::_S_iter_cat): Move to
>	__split_view_inner_iter_cat.
>	(split_view::_Inner_iter::iterator_category): Remove.
>	(elements_view::_Base): Define.
>	(elements_view::__iter_cat): Define.
>	(elements_view::_Iterator): Derive from the above in order to
>	conditionall define iterator_category as per P2259.
>	(elements_view::_Iterator::_Base): Just alias
>	elements_view::_Base.
>	(elements_view::_Iterator::_S_iter_concept)
>	(elements_view::_Iterator::iterator_concept): Define as per
>	P2259.
>	(elements_view::_Iterator::iterator_category): Remove.
>	(elements_view::_Sentinel::_Base): Just alias
>	elements_view::_Base.
>	* testsuite/24_iterators/headers/iterator/synopsis_c++20: Adjust
>	constraints on iterator_traits<counted_iterator>.
>	* testsuite/std/ranges/p2259.cc: New test.
>---
> libstdc++-v3/include/bits/stl_iterator.h      | 128 ++++++--
> libstdc++-v3/include/std/ranges               | 299 ++++++++++++------
> .../headers/iterator/synopsis_c++20.cc        |   1 +
> libstdc++-v3/testsuite/std/ranges/p2259.cc    |  91 ++++++
> 4 files changed, 412 insertions(+), 107 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/std/ranges/p2259.cc
>
>diff --git a/libstdc++-v3/include/bits/stl_iterator.h b/libstdc++-v3/include/bits/stl_iterator.h
>index dc8b101e8f8..049f83cff90 100644
>--- a/libstdc++-v3/include/bits/stl_iterator.h
>+++ b/libstdc++-v3/include/bits/stl_iterator.h
>@@ -1302,6 +1302,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     };
> #endif // C++20
>
>+  namespace __detail
>+  {
>+#if __cplusplus > 201703L && __cpp_lib_concepts
>+    template<typename _Iterator>
>+      struct __move_iter_cat
>+      { };
>+
>+    template<typename _Iterator>
>+      requires requires { typename iterator_traits<_Iterator>::iterator_category; }
>+      struct __move_iter_cat<_Iterator>
>+      {
>+	using iterator_category
>+	  = __clamp_iter_cat<typename iterator_traits<_Iterator>::iterator_category,
>+			     random_access_iterator_tag>;
>+      };
>+#endif
>+  }
>+
>   // 24.4.3  Move iterators
>   /**
>    *  Class template move_iterator is an iterator adapter with the same
>@@ -1313,13 +1331,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>    */
>   template<typename _Iterator>
>     class move_iterator
>+#if __cplusplus > 201703L && __cpp_lib_concepts
>+      : public __detail::__move_iter_cat<_Iterator>
>+#endif
>     {
>       _Iterator _M_current;
>
>       using __traits_type = iterator_traits<_Iterator>;
>-#if __cplusplus > 201703L && __cpp_lib_concepts
>-      using __base_cat = typename __traits_type::iterator_category;
>-#else
>+#if ! (__cplusplus > 201703L && __cpp_lib_concepts)

These preprocessor conditionals would be simpler if we just used
__cpp_lib_concepts instead of (C++20 && __cpp_lib_concepts). That
would be fine, because __cpp_lib_concepts is only defined for C++20
anyway. But we can change that later, not as part of this patch.

>       using __base_ref = typename __traits_type::reference;
> #endif
>

OK for trunk and gcc-11 branch, thanks.

We should also look into whether backporting to gcc-10 is feasible. I
think it should be, but I didn't try.