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 LWG 3549 changes to ranges::enable_view
Date: Tue, 19 Oct 2021 20:58:22 +0100 [thread overview]
Message-ID: <CAH6eHdQMWuH0VdNquXnvzA0_Yqr=2yt-GJA6V14jRPjd3Ug8xA@mail.gmail.com> (raw)
In-Reply-To: <20211019143521.1572302-1-ppalka@redhat.com>
On Tue, 19 Oct 2021 at 15:35, Patrick Palka via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:
> This patch also reverts the r11-3504 workaround since it's made obsolete
> by this resolution.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk only?
>
OK, thanks.
> libstdc++-v3/ChangeLog:
>
> * include/bits/ranges_base.h (view_interface): Forward declare.
> (__detail::__is_derived_from_view_interface_fn): Declare
> (__detail::__is_derived_from_view_interface): Define as per LWG
> 3549.
> (enable_view): Adjust as per LWG 3549.
> * include/bits/ranges_util.h (view_interface): Don't derive from
> view_base.
> * include/std/ranges (filter_view): Revert r11-3504 change.
> (transform_view): Likewise.
> (take_view): Likewise.
> (take_while_view): Likewise.
> (drop_view): Likewise.
> (drop_while_view): Likewise.
> (join_view): Likewise.
> (split_view): Likewise.
> (reverse_view): Likewise.
> * testsuite/std/ranges/adaptors/sizeof.cc: Update expected
> sizes.
> * testsuite/std/ranges/view.cc (test_view::test_view): Remove
> now that views no longer need to be default-initializable.
> (test01): New test.
> ---
> libstdc++-v3/include/bits/ranges_base.h | 21 ++++++++-
> libstdc++-v3/include/bits/ranges_util.h | 2 +-
> libstdc++-v3/include/std/ranges | 45 +++++++++----------
> .../testsuite/std/ranges/adaptors/sizeof.cc | 6 +--
> libstdc++-v3/testsuite/std/ranges/view.cc | 28 ++++++++++--
> 5 files changed, 70 insertions(+), 32 deletions(-)
>
> diff --git a/libstdc++-v3/include/bits/ranges_base.h
> b/libstdc++-v3/include/bits/ranges_base.h
> index 01d0c35f4b4..7801b2fd023 100644
> --- a/libstdc++-v3/include/bits/ranges_base.h
> +++ b/libstdc++-v3/include/bits/ranges_base.h
> @@ -614,12 +614,31 @@ namespace ranges
> template<sized_range _Range>
> using range_size_t = decltype(ranges::size(std::declval<_Range&>()));
>
> + template<typename _Derived>
> + requires is_class_v<_Derived> && same_as<_Derived,
> remove_cv_t<_Derived>>
> + class view_interface; // defined in <bits/ranges_util.h>
> +
> + namespace __detail
> + {
> + template<typename _Tp, typename _Up>
> + requires (!same_as<_Tp, view_interface<_Up>>)
> + void __is_derived_from_view_interface_fn(const _Tp&,
> + const
> view_interface<_Up>&); // not defined
> +
> + // Returns true iff _Tp has exactly one public base class that's a
> + // specialization of view_interface.
> + template<typename _Tp>
> + concept __is_derived_from_view_interface
> + = requires (_Tp __t) { __is_derived_from_view_interface_fn(__t,
> __t); };
> + }
> +
> /// [range.view] The ranges::view_base type.
> struct view_base { };
>
> /// [range.view] The ranges::enable_view boolean.
> template<typename _Tp>
> - inline constexpr bool enable_view = derived_from<_Tp, view_base>;
> + inline constexpr bool enable_view = derived_from<_Tp, view_base>
> + || __detail::__is_derived_from_view_interface<_Tp>;
>
> /// [range.view] The ranges::view concept.
> template<typename _Tp>
> diff --git a/libstdc++-v3/include/bits/ranges_util.h
> b/libstdc++-v3/include/bits/ranges_util.h
> index 1afa66d298c..5c0bef26220 100644
> --- a/libstdc++-v3/include/bits/ranges_util.h
> +++ b/libstdc++-v3/include/bits/ranges_util.h
> @@ -61,7 +61,7 @@ namespace ranges
> /// The ranges::view_interface class template
> template<typename _Derived>
> requires is_class_v<_Derived> && same_as<_Derived,
> remove_cv_t<_Derived>>
> - class view_interface : public view_base
> + class view_interface
> {
> private:
> constexpr _Derived& _M_derived() noexcept
> diff --git a/libstdc++-v3/include/std/ranges
> b/libstdc++-v3/include/std/ranges
> index 64396027c1b..e47fc075bbe 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -1533,9 +1533,9 @@ namespace views::__adaptor
> { return __y.__equal(__x); }
> };
>
> + _Vp _M_base = _Vp();
> [[no_unique_address]] __detail::__box<_Pred> _M_pred;
> [[no_unique_address]] __detail::_CachedPosition<_Vp>
> _M_cached_begin;
> - _Vp _M_base = _Vp();
>
> public:
> filter_view() requires (default_initializable<_Vp>
> @@ -1544,7 +1544,7 @@ namespace views::__adaptor
>
> constexpr
> filter_view(_Vp __base, _Pred __pred)
> - : _M_pred(std::move(__pred)), _M_base(std::move(__base))
> + : _M_base(std::move(__base)), _M_pred(std::move(__pred))
> { }
>
> constexpr _Vp
> @@ -1900,8 +1900,8 @@ namespace views::__adaptor
> friend _Sentinel<!_Const>;
> };
>
> - [[no_unique_address]] __detail::__box<_Fp> _M_fun;
> _Vp _M_base = _Vp();
> + [[no_unique_address]] __detail::__box<_Fp> _M_fun;
>
> public:
> transform_view() requires (default_initializable<_Vp>
> @@ -1910,7 +1910,7 @@ namespace views::__adaptor
>
> constexpr
> transform_view(_Vp __base, _Fp __fun)
> - : _M_fun(std::move(__fun)), _M_base(std::move(__base))
> + : _M_base(std::move(__base)), _M_fun(std::move(__fun))
> { }
>
> constexpr _Vp
> @@ -2037,15 +2037,15 @@ namespace views::__adaptor
> friend _Sentinel<!_Const>;
> };
>
> - range_difference_t<_Vp> _M_count = 0;
> _Vp _M_base = _Vp();
> + range_difference_t<_Vp> _M_count = 0;
>
> public:
> take_view() requires default_initializable<_Vp> = default;
>
> constexpr
> take_view(_Vp base, range_difference_t<_Vp> __count)
> - : _M_count(std::move(__count)), _M_base(std::move(base))
> + : _M_base(std::move(base)), _M_count(std::move(__count))
> { }
>
> constexpr _Vp
> @@ -2268,8 +2268,8 @@ namespace views::__adaptor
> friend _Sentinel<!_Const>;
> };
>
> - [[no_unique_address]] __detail::__box<_Pred> _M_pred;
> _Vp _M_base = _Vp();
> + [[no_unique_address]] __detail::__box<_Pred> _M_pred;
>
> public:
> take_while_view() requires (default_initializable<_Vp>
> @@ -2278,7 +2278,7 @@ namespace views::__adaptor
>
> constexpr
> take_while_view(_Vp base, _Pred __pred)
> - : _M_pred(std::move(__pred)), _M_base(std::move(base))
> + : _M_base(std::move(base)), _M_pred(std::move(__pred))
> { }
>
> constexpr _Vp
> @@ -2349,8 +2349,8 @@ namespace views::__adaptor
> class drop_view : public view_interface<drop_view<_Vp>>
> {
> private:
> - range_difference_t<_Vp> _M_count = 0;
> _Vp _M_base = _Vp();
> + range_difference_t<_Vp> _M_count = 0;
>
> // ranges::next(begin(base), count, end(base)) is O(1) if _Vp
> satisfies
> // both random_access_range and sized_range. Otherwise, cache its
> result.
> @@ -2366,7 +2366,7 @@ namespace views::__adaptor
>
> constexpr
> drop_view(_Vp __base, range_difference_t<_Vp> __count)
> - : _M_count(__count), _M_base(std::move(__base))
> + : _M_base(std::move(__base)), _M_count(__count)
> { __glibcxx_assert(__count >= 0); }
>
> constexpr _Vp
> @@ -2503,9 +2503,9 @@ namespace views::__adaptor
> class drop_while_view : public view_interface<drop_while_view<_Vp,
> _Pred>>
> {
> private:
> + _Vp _M_base = _Vp();
> [[no_unique_address]] __detail::__box<_Pred> _M_pred;
> [[no_unique_address]] __detail::_CachedPosition<_Vp>
> _M_cached_begin;
> - _Vp _M_base = _Vp();
>
> public:
> drop_while_view() requires (default_initializable<_Vp>
> @@ -2514,7 +2514,7 @@ namespace views::__adaptor
>
> constexpr
> drop_while_view(_Vp __base, _Pred __pred)
> - : _M_pred(std::move(__pred)), _M_base(std::move(__base))
> + : _M_base(std::move(__base)), _M_pred(std::move(__pred))
> { }
>
> constexpr _Vp
> @@ -2850,9 +2850,9 @@ namespace views::__adaptor
> friend _Sentinel<!_Const>;
> };
>
> + _Vp _M_base = _Vp();
> [[no_unique_address]]
> __detail::__non_propagating_cache<remove_cv_t<_InnerRange>>
> _M_inner;
> - _Vp _M_base = _Vp();
>
> public:
> join_view() requires default_initializable<_Vp> = default;
> @@ -3288,12 +3288,12 @@ namespace views::__adaptor
> { ranges::iter_swap(__x._M_i_current(), __y._M_i_current()); }
> };
>
> + _Vp _M_base = _Vp();
> _Pattern _M_pattern = _Pattern();
> // XXX: _M_current is "present only if !forward_range<V>"
> [[no_unique_address]]
> __detail::__maybe_present_t<!forward_range<_Vp>,
> __detail::__non_propagating_cache<iterator_t<_Vp>>> _M_current;
> - _Vp _M_base = _Vp();
>
>
> public:
> @@ -3303,7 +3303,7 @@ namespace views::__adaptor
>
> constexpr
> lazy_split_view(_Vp __base, _Pattern __pattern)
> - : _M_pattern(std::move(__pattern)), _M_base(std::move(__base))
> + : _M_base(std::move(__base)), _M_pattern(std::move(__pattern))
> { }
>
> template<input_range _Range>
> @@ -3311,8 +3311,8 @@ namespace views::__adaptor
> && constructible_from<_Pattern,
> single_view<range_value_t<_Range>>>
> constexpr
> lazy_split_view(_Range&& __r, range_value_t<_Range> __e)
> - : _M_pattern(views::single(std::move(__e))),
> - _M_base(views::all(std::forward<_Range>(__r)))
> + : _M_base(views::all(std::forward<_Range>(__r))),
> + _M_pattern(views::single(std::move(__e)))
> { }
>
> constexpr _Vp
> @@ -3410,9 +3410,9 @@ namespace views::__adaptor
> class split_view : public view_interface<split_view<_Vp, _Pattern>>
> {
> private:
> + _Vp _M_base = _Vp();
> _Pattern _M_pattern = _Pattern();
> __detail::__non_propagating_cache<subrange<iterator_t<_Vp>>>
> _M_cached_begin;
> - _Vp _M_base = _Vp();
>
> struct _Iterator;
> struct _Sentinel;
> @@ -3424,8 +3424,7 @@ namespace views::__adaptor
>
> constexpr
> split_view(_Vp __base, _Pattern __pattern)
> - : _M_pattern(std::move(__pattern)),
> - _M_base(std::move(__base))
> + : _M_base(std::move(__base)), _M_pattern(std::move(__pattern))
> { }
>
> template<forward_range _Range>
> @@ -3433,8 +3432,8 @@ namespace views::__adaptor
> && constructible_from<_Pattern, single_view<range_value_t<_Range>>>
> constexpr
> split_view(_Range&& __r, range_value_t<_Range> __e)
> - : _M_pattern(views::single(std::move(__e))),
> - _M_base(views::all(std::forward<_Range>(__r)))
> + : _M_base(views::all(std::forward<_Range>(__r))),
> + _M_pattern(views::single(std::move(__e)))
> { }
>
> constexpr _Vp
> @@ -3759,11 +3758,11 @@ namespace views::__adaptor
> && sized_sentinel_for<sentinel_t<_Vp>,
> iterator_t<_Vp>>);
>
> + _Vp _M_base = _Vp();
> [[no_unique_address]]
> __detail::__maybe_present_t<_S_needs_cached_begin,
> __detail::_CachedPosition<_Vp>>
> _M_cached_begin;
> - _Vp _M_base = _Vp();
>
> public:
> reverse_view() requires default_initializable<_Vp> = default;
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
> b/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
> index 219e2a61f07..61524d4c0ad 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/sizeof.cc
> @@ -42,11 +42,11 @@ static_assert(sizeof(ranges::drop_while_view<V,
> decltype(&pred_f)>) == 4*ptr);
> static_assert(sizeof(ranges::transform_view<V, decltype(&func_f)>) ==
> 3*ptr);
>
> static_assert(sizeof(ranges::filter_view<V, decltype(pred_l)>) == 3*ptr);
> -static_assert(sizeof(ranges::take_while_view<V, decltype(pred_l)>) ==
> 3*ptr);
> +static_assert(sizeof(ranges::take_while_view<V, decltype(pred_l)>) ==
> 2*ptr);
> static_assert(sizeof(ranges::drop_while_view<V, decltype(pred_l)>) ==
> 3*ptr);
> -static_assert(sizeof(ranges::transform_view<V, decltype(func_l)>) ==
> 3*ptr);
> +static_assert(sizeof(ranges::transform_view<V, decltype(func_l)>) ==
> 2*ptr);
>
> static_assert(sizeof(ranges::lazy_split_view<V, std::string_view>) ==
> 4*ptr);
>
> static_assert
> - (sizeof(ranges::reverse_view<ranges::filter_view<V, decltype(pred_l)>>)
> == 4*ptr);
> + (sizeof(ranges::reverse_view<ranges::filter_view<V, decltype(pred_l)>>)
> == 3*ptr);
> diff --git a/libstdc++-v3/testsuite/std/ranges/view.cc
> b/libstdc++-v3/testsuite/std/ranges/view.cc
> index dd8258220ed..98a162ec866 100644
> --- a/libstdc++-v3/testsuite/std/ranges/view.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/view.cc
> @@ -52,10 +52,7 @@
> static_assert(!std::ranges::view<__gnu_test::test_random_access_range<int>>);
> template<typename T>
> struct test_view
> : __gnu_test::test_random_access_range<T>, std::ranges::view_base
> -{
> - // views must be default-initializable:
> - test_view() : __gnu_test::test_random_access_range<T>(nullptr, nullptr)
> { }
> -};
> +{ };
>
> static_assert(std::ranges::view<test_view<int>>);
>
> @@ -63,3 +60,26 @@ template<>
> constexpr bool std::ranges::enable_view<test_view<long>> = false;
>
> static_assert(!std::ranges::view<test_view<long>>);
> +
> +void
> +test01()
> +{
> + // Verify LWG 3549 changes to ranges::enable_view.
> + using std::ranges::view_interface;
> +
> + struct v1
> + : __gnu_test::test_random_access_range<int>, view_interface<v1> { };
> + static_assert(!std::derived_from<v1, std::ranges::view_base>);
> + static_assert(std::ranges::enable_view<v1>);
> +
> + struct v2 : v1, view_interface<v2> { };
> + static_assert(!std::derived_from<v2, std::ranges::view_base>);
> + static_assert(!std::ranges::enable_view<v2>);
> +
> + struct v3 : __gnu_test::test_random_access_range<int> { };
> + static_assert(!std::derived_from<v3, std::ranges::view_base>);
> + static_assert(!std::ranges::enable_view<v3>);
> +
> + struct v4 { };
> + static_assert(!std::ranges::enable_view<view_interface<v4>>);
> +}
> --
> 2.33.1.711.g9d530dc002
>
>
prev parent reply other threads:[~2021-10-19 19:58 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-19 14:35 Patrick Palka
2021-10-19 19:58 ` 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='CAH6eHdQMWuH0VdNquXnvzA0_Yqr=2yt-GJA6V14jRPjd3Ug8xA@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).