On Tue, 20 Apr 2021, Jonathan Wakely wrote:
> 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.
Sounds good.
>
> > using __base_ref = typename __traits_type::reference;
> > #endif
> >
>
> OK for trunk and gcc-11 branch, thanks.
Thanks a lot, committed!
>
> We should also look into whether backporting to gcc-10 is feasible. I
> think it should be, but I didn't try.
I bet it would be modulo some small LWG issue fixes that I may have not
yet backported to the 10 branch. I'll take a look :)
