Re: [committed v2] libstdc++: Define std::ranges::to for C++23 (P1206R7) [PR111055]

[Jonathan Wakely <jwakely@redhat.com>]

On Wed, 29 Nov 2023 at 16:28, Patrick Palka wrote:
>
> On Thu, 23 Nov 2023, Jonathan Wakely wrote:
>
> > Here's the finished version of the std::ranges::to patch, which I've
> > pushed to trunk.
> >
> > Tested x86_64-linux.
> >
> > -- >8 --
> >
> > This adds the std::ranges::to functions for C++23. The rest of P1206R7
> > is not yet implemented, i.e. the new constructors taking the
> > std::from_range tag, and the new insert_range, assign_range, etc. member
> > functions. std::ranges::to works with the standard containers even
> > without the new constructors, so this is useful immediately.
> >
> > The __cpp_lib_ranges_to_container feature test macro can be defined now,
> > because that only indicates support for the changes in <ranges>, which
> > are implemented by this patch. The __cpp_lib_containers_ranges macro
> > will be defined once all containers support the new member functions.
> >
> > libstdc++-v3/ChangeLog:
> >
> >       PR libstdc++/111055
> >       * include/bits/ranges_base.h (from_range_t): Define new tag
> >       type.
> >       (from_range): Define new tag object.
> >       * include/bits/version.def (ranges_to_container): Define.
> >       * include/bits/version.h: Regenerate.
> >       * include/std/ranges (ranges::to): Define.
> >       * testsuite/std/ranges/conv/1.cc: New test.
> >       * testsuite/std/ranges/conv/2_neg.cc: New test.
> >       * testsuite/std/ranges/conv/version.cc: New test.
> > ---
> >  libstdc++-v3/include/bits/ranges_base.h       |   8 +-
> >  libstdc++-v3/include/bits/version.def         |  34 +-
> >  libstdc++-v3/include/bits/version.h           | 111 +++---
> >  libstdc++-v3/include/std/ranges               | 361 ++++++++++++++++-
> >  libstdc++-v3/testsuite/std/ranges/conv/1.cc   | 369 ++++++++++++++++++
> >  .../testsuite/std/ranges/conv/2_neg.cc        |  24 ++
> >  .../testsuite/std/ranges/conv/version.cc      |  19 +
> >  7 files changed, 866 insertions(+), 60 deletions(-)
> >  create mode 100644 libstdc++-v3/testsuite/std/ranges/conv/1.cc
> >  create mode 100644 libstdc++-v3/testsuite/std/ranges/conv/2_neg.cc
> >  create mode 100644 libstdc++-v3/testsuite/std/ranges/conv/version.cc
> >
> > diff --git a/libstdc++-v3/include/bits/ranges_base.h b/libstdc++-v3/include/bits/ranges_base.h
> > index 7fa43d1965a..1ca2c5ce2bb 100644
> > --- a/libstdc++-v3/include/bits/ranges_base.h
> > +++ b/libstdc++-v3/include/bits/ranges_base.h
> > @@ -37,6 +37,7 @@
> >  #include <bits/stl_iterator.h>
> >  #include <ext/numeric_traits.h>
> >  #include <bits/max_size_type.h>
> > +#include <bits/version.h>
> >
> >  #ifdef __cpp_lib_concepts
> >  namespace std _GLIBCXX_VISIBILITY(default)
> > @@ -1056,8 +1057,13 @@ namespace ranges
> >      using borrowed_iterator_t = __conditional_t<borrowed_range<_Range>,
> >                                               iterator_t<_Range>,
> >                                               dangling>;
> > -
> >  } // namespace ranges
> > +
> > +#if __glibcxx_ranges_to_container // C++ >= 23
> > +  struct from_range_t { explicit from_range_t() = default; };
> > +  inline constexpr from_range_t from_range{};
> > +#endif
> > +
> >  _GLIBCXX_END_NAMESPACE_VERSION
> >  } // namespace std
> >  #endif // library concepts
> > diff --git a/libstdc++-v3/include/bits/version.def b/libstdc++-v3/include/bits/version.def
> > index 605708dfee7..140777832ed 100644
> > --- a/libstdc++-v3/include/bits/version.def
> > +++ b/libstdc++-v3/include/bits/version.def
> > @@ -1439,19 +1439,21 @@ ftms = {
> >    };
> >  };
> >
> > -ftms = {
> > -  name = to_underlying;
> > -  values = {
> > -    v = 202102;
> > -    cxxmin = 23;
> > -  };
> > -};
> > +//ftms = {
> > +//  name = container_ranges;
> > +//  values = {
> > +//    v = 202202;
> > +//    cxxmin = 23;
> > +//    hosted = yes;
> > +//  };
> > +//};
> >
> >  ftms = {
> > -  name = unreachable;
> > +  name = ranges_to_container;
> >    values = {
> >      v = 202202;
> >      cxxmin = 23;
> > +    hosted = yes;
> >    };
> >  };
> >
> > @@ -1683,6 +1685,22 @@ ftms = {
> >    };
> >  };
> >
> > +ftms = {
> > +  name = to_underlying;
> > +  values = {
> > +    v = 202102;
> > +    cxxmin = 23;
> > +  };
> > +};
> > +
> > +ftms = {
> > +  name = unreachable;
> > +  values = {
> > +    v = 202202;
> > +    cxxmin = 23;
> > +  };
> > +};
> > +
> >  ftms = {
> >    name = fstream_native_handle;
> >    values = {
> > diff --git a/libstdc++-v3/include/bits/version.h b/libstdc++-v3/include/bits/version.h
> > index cacd9375cab..1fb1d148459 100644
> > --- a/libstdc++-v3/include/bits/version.h
> > +++ b/libstdc++-v3/include/bits/version.h
> > @@ -1740,29 +1740,18 @@
> >  #endif /* !defined(__cpp_lib_reference_from_temporary) && defined(__glibcxx_want_reference_from_temporary) */
> >  #undef __glibcxx_want_reference_from_temporary
> >
> > -// from version.def line 1443
> > -#if !defined(__cpp_lib_to_underlying)
> > -# if (__cplusplus >= 202100L)
> > -#  define __glibcxx_to_underlying 202102L
> > -#  if defined(__glibcxx_want_all) || defined(__glibcxx_want_to_underlying)
> > -#   define __cpp_lib_to_underlying 202102L
> > +// from version.def line 1452
> > +#if !defined(__cpp_lib_ranges_to_container)
> > +# if (__cplusplus >= 202100L) && _GLIBCXX_HOSTED
> > +#  define __glibcxx_ranges_to_container 202202L
> > +#  if defined(__glibcxx_want_all) || defined(__glibcxx_want_ranges_to_container)
> > +#   define __cpp_lib_ranges_to_container 202202L
> >  #  endif
> >  # endif
> > -#endif /* !defined(__cpp_lib_to_underlying) && defined(__glibcxx_want_to_underlying) */
> > -#undef __glibcxx_want_to_underlying
> > +#endif /* !defined(__cpp_lib_ranges_to_container) && defined(__glibcxx_want_ranges_to_container) */
> > +#undef __glibcxx_want_ranges_to_container
> >
> > -// from version.def line 1451
> > -#if !defined(__cpp_lib_unreachable)
> > -# if (__cplusplus >= 202100L)
> > -#  define __glibcxx_unreachable 202202L
> > -#  if defined(__glibcxx_want_all) || defined(__glibcxx_want_unreachable)
> > -#   define __cpp_lib_unreachable 202202L
> > -#  endif
> > -# endif
> > -#endif /* !defined(__cpp_lib_unreachable) && defined(__glibcxx_want_unreachable) */
> > -#undef __glibcxx_want_unreachable
> > -
> > -// from version.def line 1459
> > +// from version.def line 1461
> >  #if !defined(__cpp_lib_ranges_zip)
> >  # if (__cplusplus >= 202100L)
> >  #  define __glibcxx_ranges_zip 202110L
> > @@ -2059,7 +2048,29 @@
> >  #endif /* !defined(__cpp_lib_string_resize_and_overwrite) && defined(__glibcxx_want_string_resize_and_overwrite) */
> >  #undef __glibcxx_want_string_resize_and_overwrite
> >
> > -// from version.def line 1687
> > +// from version.def line 1689
> > +#if !defined(__cpp_lib_to_underlying)
> > +# if (__cplusplus >= 202100L)
> > +#  define __glibcxx_to_underlying 202102L
> > +#  if defined(__glibcxx_want_all) || defined(__glibcxx_want_to_underlying)
> > +#   define __cpp_lib_to_underlying 202102L
> > +#  endif
> > +# endif
> > +#endif /* !defined(__cpp_lib_to_underlying) && defined(__glibcxx_want_to_underlying) */
> > +#undef __glibcxx_want_to_underlying
> > +
> > +// from version.def line 1697
> > +#if !defined(__cpp_lib_unreachable)
> > +# if (__cplusplus >= 202100L)
> > +#  define __glibcxx_unreachable 202202L
> > +#  if defined(__glibcxx_want_all) || defined(__glibcxx_want_unreachable)
> > +#   define __cpp_lib_unreachable 202202L
> > +#  endif
> > +# endif
> > +#endif /* !defined(__cpp_lib_unreachable) && defined(__glibcxx_want_unreachable) */
> > +#undef __glibcxx_want_unreachable
> > +
> > +// from version.def line 1705
> >  #if !defined(__cpp_lib_fstream_native_handle)
> >  # if (__cplusplus >  202302L) && _GLIBCXX_HOSTED
> >  #  define __glibcxx_fstream_native_handle 202306L
> > diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> > index 26d6c013ad0..63bea862c05 100644
> > --- a/libstdc++-v3/include/std/ranges
> > +++ b/libstdc++-v3/include/std/ranges
> > @@ -64,6 +64,7 @@
> >  #define __glibcxx_want_ranges_repeat
> >  #define __glibcxx_want_ranges_slide
> >  #define __glibcxx_want_ranges_stride
> > +#define __glibcxx_want_ranges_to_container
> >  #define __glibcxx_want_ranges_zip
> >  #include <bits/version.h>
> >
> > @@ -9213,8 +9214,366 @@ namespace views::__adaptor
> >
> >    namespace views = ranges::views;
> >
> > +#if __cpp_lib_ranges_to_container // C++ >= 23
> > +namespace ranges
> > +{
> > +/// @cond undocumented
> > +namespace __detail
> > +{
> > +  template<typename _Container>
> > +    constexpr bool __reservable_container
> > +      = sized_range<_Container>
> > +      && requires(_Container& __c, range_size_t<_Container> __n) {
> > +     __c.reserve(__n);
> > +     { __c.capacity() } -> same_as<decltype(__n)>;
> > +     { __c.max_size() } -> same_as<decltype(__n)>;
> > +      };
> > +
> > +  template<typename _Container, typename _Ref>
> > +    constexpr bool __container_insertable
> > +      = requires(_Container& __c, _Ref&& __ref) {
> > +     typename _Container::value_type;
> > +     requires (
> > +       requires { __c.push_back(std::forward<_Ref>(__ref)); }
> > +       || requires { __c.insert(__c.end(), std::forward<_Ref>(__ref)); }
> > +     );
> > +      };
> > +
> > +  template<typename _Ref, typename _Container>
> > +    constexpr auto
> > +    __container_inserter(_Container& __c)
> > +    {
> > +      if constexpr (requires { __c.push_back(std::declval<_Ref>()); })
> > +     return std::back_inserter(__c);
> > +      else
> > +     return std::inserter(__c, __c.end());
> > +    }
> > +
> > +  template<typename _Cont, typename _Range>
> > +    constexpr bool __toable = requires {
> > +      requires (!input_range<_Range>
>
> It seems this sholud be input_range<_Cont>?
>
> > +               || convertible_to<range_reference_t<_Range>,
> > +                                 range_value_t<_Cont>>);
> > +    };
> > +} // namespace __detail
> > +/// @endcond
> > +
> > +  /// Convert a range to a container.
> > +  /**
> > +   * @tparam _Cont A container type.
> > +   * @param __r A range that models the `input_range` concept.
> > +   * @param __args... Arguments to pass to the container constructor.
> > +   * @since C++23
> > +   *
> > +   * This function converts a range to the `_Cont` type.
> > +   *
> > +   * For example, `std::ranges::to<std::vector<int>>(some_view)`
> > +   * will convert the view to `std::vector<int>`.
> > +   *
> > +   * Additional constructor arguments for the container can be supplied after
> > +   * the input range argument, e.g.
> > +   * `std::ranges::to<std::vector<int, Alloc<int>>>(a_range, an_allocator)`.
> > +   */
> > +  template<typename _Cont, input_range _Rg, typename... _Args>
> > +    requires (!view<_Cont>)
> > +    constexpr _Cont
> > +    to [[nodiscard]] (_Rg&& __r, _Args&&... __args)
> > +    {
> > +      static_assert(!is_const_v<_Cont> && !is_volatile_v<_Cont>);
> > +      static_assert(is_class_v<_Cont>);
> > +
> > +      if constexpr (__detail::__toable<_Cont, _Rg>)
> > +     {
> > +       if constexpr (constructible_from<_Cont, _Rg, _Args...>)
> > +         return _Cont(std::forward<_Rg>(__r),
> > +                      std::forward<_Args>(__args)...);
> > +       else if constexpr (constructible_from<_Cont, from_range_t, _Rg, _Args...>)
> > +         return _Cont(from_range, std::forward<_Rg>(__r),
> > +                      std::forward<_Args>(__args)...);
> > +       else if constexpr (requires { common_range<_Rg>;
>
> Missing 'requires' before common_range?
>
> > +             typename __iter_category_t<iterator_t<_Rg>>;
> > +             requires derived_from<__iter_category_t<iterator_t<_Rg>>,
> > +                                   input_iterator_tag>;
> > +             requires constructible_from<_Cont, iterator_t<_Rg>,
> > +                                         sentinel_t<_Rg>, _Args...>;
> > +           })
> > +         return _Cont(ranges::begin(__r), ranges::end(__r),
> > +                      std::forward<_Args>(__args)...);
> > +       else
> > +         {
> > +           using __detail::__container_insertable;
> > +           using __detail::__reservable_container;
> > +           using _RefT = range_reference_t<_Rg>;
> > +           static_assert(constructible_from<_Cont, _Args...>);
> > +           static_assert(__container_insertable<_Cont, _RefT>);
> > +           _Cont __c(std::forward<_Args>(__args)...);
> > +           if constexpr (sized_range<_Rg> && __reservable_container<_Cont>)
> > +             __c.reserve(static_cast<range_size_t<_Cont>>(ranges::size(__r)));
> > +           auto __ins = __detail::__container_inserter<_RefT>(__c);
> > +           for (auto&& __e : __r)
> > +             *__ins++ = std::forward<decltype(__e)>(__e);
> > +           return __c;
> > +         }
> > +     }
> > +      else
> > +     {
> > +       static_assert(input_range<range_reference_t<_Rg>>);
> > +       // _GLIBCXX_RESOLVE_LIB_DEFECTS
> > +       // 3984. ranges::to's recursion branch may be ill-formed
> > +       return ranges::to<_Cont>(ref_view(__r) | views::transform(
> > +             []<typename _Elt>(_Elt&& __elem) {
> > +               using _ValT = range_value_t<_Cont>;
> > +               return ranges::to<_ValT>(std::forward<_Elt>(__elem));
> > +             }), std::forward<_Args>(__args)...);
> > +     }
> > +    }
> > +
> > +/// @cond undocumented
> > +namespace __detail
> > +{
> > +  template<typename _Rg>
> > +    struct _InputIter
> > +    {
> > +      using iterator_category = input_iterator_tag;
> > +      using value_type = range_value_t<_Rg>;
> > +      using difference_type = ptrdiff_t;
> > +      using pointer = add_pointer_t<range_reference_t<_Rg>>;
> > +      using reference = range_reference_t<_Rg>;
> > +      reference operator*() const;
> > +      pointer operator->() const;
> > +      _InputIter& operator++();
> > +      _InputIter operator++(int);
> > +      bool operator==(const _InputIter&) const;
> > +    };
> > +
> > +#if 0
> > +  template<template<typename...> typename _Cont, typename _Rg,
> > +        typename... _Args>
> > +    concept __deduce_expr_1 = requires {
> > +      _Cont(std::declval<_Rg>(), std::declval<_Args>()...);
> > +    };
> > +
> > +  template<template<typename...> typename _Cont, typename _Rg,
> > +        typename... _Args>
> > +    concept __deduce_expr_2 = requires {
> > +      _Cont(from_range, std::declval<_Rg>(), std::declval<_Args>()...);
> > +    };
> > +
> > +  template<template<typename...> typename _Cont, typename _Rg,
> > +        typename... _Args>
> > +    concept __deduce_expr_3 = requires(_InputIter<_Rg> __i) {
> > +      _Cont(std::move(__i), std::move(__i), std::declval<_Args>()...);
> > +    };
> > +#endif
> > +
> > +  template<template<typename...> typename _Cont, input_range _Rg,
> > +        typename... _Args>
> > +    using _DeduceExpr1
> > +      = decltype(_Cont(std::declval<_Rg>(), std::declval<_Args>()...));
> > +
> > +  template<template<typename...> typename _Cont, input_range _Rg,
> > +        typename... _Args>
> > +    using _DeduceExpr2
> > +      = decltype(_Cont(from_range, std::declval<_Rg>(),
> > +                    std::declval<_Args>()...));
> > +
> > +  template<template<typename...> typename _Cont, input_range _Rg,
> > +        typename... _Args>
> > +    using _DeduceExpr3
> > +      = decltype(_Cont(std::declval<_InputIter<_Rg>>(),
> > +                    std::declval<_InputIter<_Rg>>(),
> > +                    std::declval<_Args>()...));
> > +
> > +} // namespace __detail
> > +/// @endcond
> > +
> > +  template<template<typename...> typename _Cont, input_range _Rg,
> > +        typename... _Args>
> > +    constexpr auto
> > +    to [[nodiscard]] (_Rg&& __r, _Args&&... __args)
> > +    {
> > +      using __detail::_DeduceExpr1;
> > +      using __detail::_DeduceExpr2;
> > +      using __detail::_DeduceExpr3;
> > +      if constexpr (requires { typename _DeduceExpr1<_Cont, _Rg, _Args...>; })
> > +     return ranges::to<_DeduceExpr1<_Cont, _Rg, _Args...>>(
> > +         std::forward<_Rg>(__r), std::forward<_Args>(__args)...);
> > +      else if constexpr (requires { typename _DeduceExpr2<_Cont, _Rg, _Args...>; })
> > +     return ranges::to<_DeduceExpr2<_Cont, _Rg, _Args...>>(
> > +         std::forward<_Rg>(__r), std::forward<_Args>(__args)...);
> > +      else if constexpr (requires { typename _DeduceExpr3<_Cont, _Rg, _Args...>; })
> > +     return ranges::to<_DeduceExpr3<_Cont, _Rg, _Args...>>(
> > +         std::forward<_Rg>(__r), std::forward<_Args>(__args)...);
> > +      else
> > +     static_assert(false); // Cannot deduce container specialization.
> > +    }
> > +
> > +/// @cond undocumented
> > +namespace __detail
> > +{
> > +  template<typename _Cont, typename... _Args>
> > +    class _ToClosure
> > +    : public views::__adaptor::_RangeAdaptorClosure<_ToClosure<_Cont, _Args...>>
> > +    {
> > +      tuple<decay_t<_Args>...> _M_bound_args;
> > +
> > +    public:
> > +      _ToClosure(_Args&&... __args)
>
> Missing constexpr?
>
> > +      : _M_bound_args(std::forward<_Args>(__args)...)
> > +      { }
> > +
> > +      // TODO: use explicit object functions ("deducing this").
> > +
> > +      template<typename _Rg>
> > +     constexpr auto
> > +     operator()(_Rg&& __r) &
> > +     {
> > +       return std::apply([&__r]<typename... _Tp>(_Tp&&... __args) {
> > +         return ranges::to<_Cont>(std::forward<_Rg>(__r),
> > +                                  std::forward<_Tp>(__args)...);
> > +       }, _M_bound_args);
> > +     }
> > +
> > +      template<typename _Rg>
> > +     constexpr auto
> > +     operator()(_Rg&& __r) const &
> > +     {
> > +       return std::apply([&__r]<typename... _Tp>(_Tp&&... __args) {
> > +         return ranges::to<_Cont>(std::forward<_Rg>(__r),
> > +                                  std::forward<_Tp>(__args)...);
> > +       }, _M_bound_args);
> > +     }
> > +
> > +      template<typename _Rg>
> > +     constexpr auto
> > +     operator()(_Rg&& __r) &&
> > +     {
> > +       return std::apply([&__r]<typename... _Tp>(_Tp&&... __args) {
> > +         return ranges::to<_Cont>(std::forward<_Rg>(__r),
> > +                                  std::forward<_Tp>(__args)...);
> > +       }, std::move(_M_bound_args));
> > +     }
> > +
> > +      template<typename _Rg>
> > +     constexpr auto
> > +     operator()(_Rg&& __r) const &&
> > +     {
> > +       return std::apply([&__r]<typename... _Tp>(_Tp&&... __args) {
> > +         return ranges::to<_Cont>(std::forward<_Rg>(__r),
> > +                                  std::forward<_Tp>(__args)...);
> > +       }, std::move(_M_bound_args));
> > +     }
> > +    };
> > +} // namespace __detail
> > +/// @endcond
> > +
> > +  /// ranges::to adaptor for converting a range to a container type
> > +  /**
> > +   * @tparam _Cont A container type.
> > +   * @param __args... Arguments to pass to the container constructor.
> > +   * @since C++23
> > +   *
> > +   * This range adaptor returns a range adaptor closure object that converts
> > +   * a range to the `_Cont` type.
> > +   *
> > +   * For example, `some_view | std::ranges::to<std::vector<int>>()`
> > +   * will convert the view to `std::vector<int>`.
> > +   *
> > +   * Additional constructor arguments for the container can be supplied, e.g.
> > +   * `r | std::ranges::to<std::vector<int, Alloc<int>>>(an_allocator)`.
> > +   */
> > +  template<typename _Cont, typename... _Args>
> > +    requires (!view<_Cont>)
> > +    constexpr __detail::_ToClosure<_Cont, _Args...>
> > +    to [[nodiscard]] (_Args&&... __args)
> > +    { return {std::forward<_Args>(__args)...}; }
> > +
> > +/// @cond undocumented
> > +namespace __detail
> > +{
> > +  template<template<typename...> typename _Cont, typename... _Args>
> > +    class _ToClosure2
> > +    : public views::__adaptor::_RangeAdaptorClosure<_ToClosure2<_Cont, _Args...>>
> > +    {
> > +      tuple<decay_t<_Args>...> _M_bound_args;
> > +
> > +    public:
> > +      _ToClosure2(_Args&&... __args)
>
> Same here.

Thanks, those are all fixed at r14-6020-g18d8a50a042a7f

>
> Would it be possible to use _RangeAdaptor instead of _RangeAdaptorClosure to
> leverage the existing partial application/forwarding code?  IIUC we'd need to
> allow omitting the _S_arity member to mean accept any number of arguments.
> I can work on that if anything.

Sure, that might be an improvement. Thanks!