public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely.gcc@gmail.com>
To: Patrick Palka <ppalka@redhat.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
	"libstdc++" <libstdc++@gcc.gnu.org>
Subject: Re: [PATCH] libstdc++: Implement P2415R2 "What is a view?"
Date: Mon, 21 Feb 2022 20:55:41 +0000	[thread overview]
Message-ID: <CAH6eHdTca426gWASAr1d26vTcCa4T3nrGm-9s0YprbXWO0Gm1w@mail.gmail.com> (raw)
In-Reply-To: <20220221193925.599224-1-ppalka@redhat.com>

On Mon, 21 Feb 2022 at 19:39, Patrick Palka via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>

OK, thanks.



>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/ranges_base.h (__detail::__is_initializer_list):
>         Define.
>         (viewable_range): Adjust as per P2415R2.
>         * include/std/ranges (owning_view): Define as per P2415R2.
>         (enable_borrowed_range<owning_view>): Likewise.
>         (views::__detail::__can_subrange): Replace with ...
>         (views::__detail::__can_owning_view): ... this.
>         (views::_All::_S_noexcept): Sync with operator().
>         (views::_All::operator()): Use owning_view instead of subrange
>         as per P2415R2.
>         * testsuite/std/ranges/adaptors/all.cc (test06): Adjust now that
>         views::all uses owning_view instead of subrange.
>         (test08): New test.
>         * testsuite/std/ranges/adaptors/lazy_split.cc (test09): Adjust
>         now that rvalue non-view non-borrowed ranges are viewable.
>         * testsuite/std/ranges/adaptors/split.cc (test06): Likewise.
> ---
>  libstdc++-v3/include/bits/ranges_base.h       | 16 +++-
>  libstdc++-v3/include/std/ranges               | 89 ++++++++++++++++++-
>  .../testsuite/std/ranges/adaptors/all.cc      | 59 ++++++++----
>  .../std/ranges/adaptors/lazy_split.cc         | 13 ++-
>  .../testsuite/std/ranges/adaptors/split.cc    | 13 ++-
>  5 files changed, 157 insertions(+), 33 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/ranges_base.h
> b/libstdc++-v3/include/bits/ranges_base.h
> index 3c5f4b1790a..38db33fd2ce 100644
> --- a/libstdc++-v3/include/bits/ranges_base.h
> +++ b/libstdc++-v3/include/bits/ranges_base.h
> @@ -634,7 +634,7 @@ namespace ranges
>      template<typename _Tp>
>        concept __is_derived_from_view_interface
>         = requires (_Tp __t) { __is_derived_from_view_interface_fn(__t,
> __t); };
> -  }
> +  } // namespace __detail
>
>    /// [range.view] The ranges::view_base type.
>    struct view_base { };
> @@ -689,11 +689,23 @@ namespace ranges
>      concept common_range
>        = range<_Tp> && same_as<iterator_t<_Tp>, sentinel_t<_Tp>>;
>
> +  namespace __detail
> +  {
> +    template<typename _Tp>
> +      inline constexpr bool __is_initializer_list = false;
> +
> +    template<typename _Tp>
> +      inline constexpr bool __is_initializer_list<initializer_list<_Tp>>
> = true;
> +  } // namespace __detail
> +
>    /// A range which can be safely converted to a view.
>    template<typename _Tp>
>      concept viewable_range = range<_Tp>
>        && ((view<remove_cvref_t<_Tp>> &&
> constructible_from<remove_cvref_t<_Tp>, _Tp>)
> -         || (!view<remove_cvref_t<_Tp>> && borrowed_range<_Tp>));
> +         || (!view<remove_cvref_t<_Tp>>
> +             && (is_lvalue_reference_v<_Tp>
> +                 || (movable<remove_reference_t<_Tp>>
> +                     &&
> !__detail::__is_initializer_list<remove_cvref_t<_Tp>>))));
>
>    // [range.iter.ops] range iterator operations
>
> diff --git a/libstdc++-v3/include/std/ranges
> b/libstdc++-v3/include/std/ranges
> index ac85907f129..3e71ecb32b7 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -1144,6 +1144,87 @@ namespace views::__adaptor
>    template<typename _Tp>
>      inline constexpr bool enable_borrowed_range<ref_view<_Tp>> = true;
>
> +  template<range _Range>
> +    requires movable<_Range>
> +      && (!__detail::__is_initializer_list<remove_cv_t<_Range>>)
> +    class owning_view : public view_interface<owning_view<_Range>>
> +    {
> +    private:
> +      _Range _M_r = _Range();
> +
> +    public:
> +      owning_view() requires default_initializable<_Range> = default;
> +
> +      constexpr
> +      owning_view(_Range&& __t)
> +      noexcept(is_nothrow_move_constructible_v<_Range>)
> +       : _M_r(std::move(__t))
> +      { }
> +
> +      owning_view(owning_view&&) = default;
> +      owning_view& operator=(owning_view&&) = default;
> +
> +      constexpr _Range&
> +      base() & noexcept
> +      { return _M_r; }
> +
> +      constexpr const _Range&
> +      base() const& noexcept
> +      { return _M_r; }
> +
> +      constexpr _Range&&
> +      base() && noexcept
> +      { return std::move(_M_r); }
> +
> +      constexpr const _Range&&
> +      base() const&& noexcept
> +      { return std::move(_M_r); }
> +
> +      constexpr iterator_t<_Range>
> +      begin()
> +      { return ranges::begin(_M_r); }
> +
> +      constexpr sentinel_t<_Range>
> +      end()
> +      { return ranges::end(_M_r); }
> +
> +      constexpr auto
> +      begin() const requires range<const _Range>
> +      { return ranges::begin(_M_r); }
> +
> +      constexpr auto
> +      end() const requires range<const _Range>
> +      { return ranges::end(_M_r); }
> +
> +      constexpr bool
> +      empty() requires requires { ranges::empty(_M_r); }
> +      { return ranges::empty(_M_r); }
> +
> +      constexpr bool
> +      empty() const requires requires { ranges::empty(_M_r); }
> +      { return ranges::empty(_M_r); }
> +
> +      constexpr auto
> +      size() requires sized_range<_Range>
> +      { return ranges::size(_M_r); }
> +
> +      constexpr auto
> +      size() const requires sized_range<const _Range>
> +      { return ranges::size(_M_r); }
> +
> +      constexpr auto
> +      data() requires contiguous_range<_Range>
> +      { return ranges::data(_M_r); }
> +
> +      constexpr auto
> +      data() const requires contiguous_range<const _Range>
> +      { return ranges::data(_M_r); }
> +    };
> +
> +  template<typename _Tp>
> +    inline constexpr bool enable_borrowed_range<owning_view<_Tp>>
> +      = enable_borrowed_range<_Tp>;
> +
>    namespace views
>    {
>      namespace __detail
> @@ -1152,7 +1233,7 @@ namespace views::__adaptor
>         concept __can_ref_view = requires {
> ref_view{std::declval<_Range>()}; };
>
>        template<typename _Range>
> -       concept __can_subrange = requires {
> subrange{std::declval<_Range>()}; };
> +       concept __can_owning_view = requires {
> owning_view{std::declval<_Range>()}; };
>      } // namespace __detail
>
>      struct _All : __adaptor::_RangeAdaptorClosure
> @@ -1166,13 +1247,13 @@ namespace views::__adaptor
>           else if constexpr (__detail::__can_ref_view<_Range>)
>             return true;
>           else
> -           return noexcept(subrange{std::declval<_Range>()});
> +           return noexcept(owning_view{std::declval<_Range>()});
>         }
>
>        template<viewable_range _Range>
>         requires view<decay_t<_Range>>
>           || __detail::__can_ref_view<_Range>
> -         || __detail::__can_subrange<_Range>
> +         || __detail::__can_owning_view<_Range>
>         constexpr auto
>         operator() [[nodiscard]] (_Range&& __r) const
>         noexcept(_S_noexcept<_Range>())
> @@ -1182,7 +1263,7 @@ namespace views::__adaptor
>           else if constexpr (__detail::__can_ref_view<_Range>)
>             return ref_view{std::forward<_Range>(__r)};
>           else
> -           return subrange{std::forward<_Range>(__r)};
> +           return owning_view{std::forward<_Range>(__r)};
>         }
>
>        static constexpr bool _S_has_simple_call_op = true;
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
> b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
> index c25d972274f..9d4e714b3e9 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/all.cc
> @@ -19,7 +19,9 @@
>  // { dg-do run { target c++2a } }
>
>  #include <algorithm>
> +#include <array>
>  #include <ranges>
> +#include <vector>
>  #include <testsuite_hooks.h>
>  #include <testsuite_iterators.h>
>
> @@ -130,20 +132,6 @@ test05()
>    static_assert(!requires { 0 | all; });
>  }
>
> -template<bool B1, bool B2>
> -struct BorrowedRange
> -{
> -  int* ptr = nullptr;
> -
> -  BorrowedRange(int (&arr)[3]) noexcept : ptr(arr) { }
> -
> -  int* begin() const noexcept(B1) { return ptr; }
> -  int* end() const noexcept(B2) { return ptr + 3; }
> -};
> -
> -template<bool B1, bool B2>
> -const bool std::ranges::enable_borrowed_range<BorrowedRange<B1, B2>> =
> true;
> -
>  void
>  test06()
>  {
> @@ -152,11 +140,11 @@ test06()
>    // Using ref_view:
>    static_assert(noexcept(views::all(x)));
>
> -  // Using subrange:
> -  static_assert(noexcept(views::all(BorrowedRange<true, true>(x))));
> -  static_assert(!noexcept(views::all(BorrowedRange<true, false>(x))));
> -  static_assert(!noexcept(views::all(BorrowedRange<false, true>(x))));
> -  static_assert(!noexcept(views::all(BorrowedRange<false, false>(x))));
> +  // Using owning_view:
> +  struct A { A(); A(const A&); };
> +  static_assert(noexcept(views::all(std::array<int, 3>{})));
> +  static_assert(!std::is_nothrow_move_constructible_v<std::array<A, 3>>);
> +  static_assert(!noexcept(views::all(std::array<A, 3>{})));
>  }
>
>  void
> @@ -173,6 +161,38 @@ test07()
>    static_assert(!ranges::viewable_range<view_t&>);
>  }
>
> +constexpr bool
> +test08()
> +{
> +  // Verify P2415R2 "What is a view?" changes
> +  // Namely, rvalue non-view non-borrowed ranges are now viewable.
> +  static_assert(ranges::viewable_range<std::vector<int>&&>);
> +  static_assert(!ranges::viewable_range<const std::vector<int>&&>);
> +
> +  static_assert(ranges::viewable_range<std::initializer_list<int>&>);
> +  static_assert(ranges::viewable_range<const
> std::initializer_list<int>&>);
> +  static_assert(!ranges::viewable_range<std::initializer_list<int>&&>);
> +  static_assert(!ranges::viewable_range<const
> std::initializer_list<int>&&>);
> +
> +  using type = views::all_t<std::vector<int>&&>;
> +  using type = ranges::owning_view<std::vector<int>>;
> +
> +  std::same_as<type> auto v = std::vector<int>{{1,2,3}} | views::all;
> +
> +  VERIFY( ranges::equal(v, (int[]){1,2,3}) );
> +  VERIFY( ranges::size(v) == 3 );
> +  VERIFY( !ranges::empty(v) );
> +  VERIFY( ranges::data(v) == &v[0] );
> +
> +  const auto w = std::move(v);
> +  VERIFY( ranges::equal(w, (int[]){1,2,3}) );
> +  VERIFY( ranges::size(w) == 3 );
> +  VERIFY( !ranges::empty(w) );
> +  VERIFY( ranges::data(w) == &w[0] );
> +
> +  return true;
> +}
> +
>  int
>  main()
>  {
> @@ -183,4 +203,5 @@ main()
>    test05();
>    test06();
>    test07();
> +  static_assert(test08());
>  }
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/lazy_split.cc
> b/libstdc++-v3/testsuite/std/ranges/adaptors/lazy_split.cc
> index a7d3dd6a23e..e0279dc2a4a 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/lazy_split.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/lazy_split.cc
> @@ -163,10 +163,15 @@ test09()
>    static_assert(!requires { lazy_split(p)(); });
>    static_assert(!requires { s | lazy_split; });
>
> -  static_assert(!requires { s | lazy_split(p); });
> -  static_assert(!requires { lazy_split(p)(s); });
> -  static_assert(!requires { s | (lazy_split(p) | views::all); });
> -  static_assert(!requires { (lazy_split(p) | views::all)(s); });
> +  // Test the case where the closure object is used as an rvalue and
> therefore
> +  // the copy of p is forwarded as an rvalue.
> +  // This used to be invalid, but is well-formed after P2415R2 relaxed the
> +  // requirements of viewable_range to permit rvalue non-view non-borrowed
> +  // ranges such as std::string&&.
> +  static_assert(requires { s | lazy_split(p); });
> +  static_assert(requires { lazy_split(p)(s); });
> +  static_assert(requires { s | (lazy_split(p) | views::all); });
> +  static_assert(requires { (lazy_split(p) | views::all)(s); });
>
>    static_assert(requires { s | lazy_split(views::all(p)); });
>    static_assert(requires { lazy_split(views::all(p))(s); });
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
> b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
> index 44245f5071a..4f8dddeb410 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/split.cc
> @@ -145,10 +145,15 @@ test06()
>    static_assert(!requires { split(p)(); });
>    static_assert(!requires { s | split; });
>
> -  static_assert(!requires { s | split(p); });
> -  static_assert(!requires { split(p)(s); });
> -  static_assert(!requires { s | (split(p) | views::all); });
> -  static_assert(!requires { (split(p) | views::all)(s); });
> +  // Test the case where the closure object is used as an rvalue and
> therefore
> +  // the copy of p is forwarded as an rvalue.
> +  // This used to be invalid, but is well-formed after P2415R2 relaxed the
> +  // requirements of viewable_range to permit rvalue non-view non-borrowed
> +  // ranges such as std::string&&.
> +  static_assert(requires { s | split(p); });
> +  static_assert(requires { split(p)(s); });
> +  static_assert(requires { s | (split(p) | views::all); });
> +  static_assert(requires { (split(p) | views::all)(s); });
>
>    static_assert(requires { s | split(views::all(p)); });
>    static_assert(requires { split(views::all(p))(s); });
> --
> 2.35.1.225.ge2ac9141e6
>
>

      reply	other threads:[~2022-02-21 20:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-21 19:39 Patrick Palka
2022-02-21 20:55 ` Jonathan Wakely [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAH6eHdTca426gWASAr1d26vTcCa4T3nrGm-9s0YprbXWO0Gm1w@mail.gmail.com \
    --to=jwakely.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    --cc=ppalka@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).