On Tue, 22 Oct 2024 at 19:22, Patrick Palka <[email protected]> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
OK, thanks.
>
> -- >8 --
>
> This patch proactively implements the proposed resolution for this LWG
> issue, which seems straightforward and slated to be approved as-is.
>
> I opted to not add a _GLIBCXX_RESOLVE_LIB_DEFECTS code comment for this
> since concat_view is C++26 and hasn't been shipped in a GCC release yet.
Yup, that's fine. The LWG issue isn't a DR against a published
standard, so once we get a finished C++26 standard, this issue will
already be part of that standard.
>
> libstdc++-v3/ChangeLog:
>
> * include/std/ranges (concat_view::begin): Add space after
> 'requires' starting a requires-clause.
> (concat_view::end): Refine condition for returning an iterator
> rather than default_sentinel as per LWG 4166.
> * testsuite/std/ranges/concat/1.cc (test03): Verify LWG 4166
> example.
> ---
> libstdc++-v3/include/std/ranges | 10 ++++----
> libstdc++-v3/testsuite/std/ranges/concat/1.cc | 23 +++++++++++++++++++
> 2 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index 5b455888536..a6c4d66520c 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -9687,7 +9687,7 @@ namespace ranges
> { }
>
> constexpr iterator<false>
> - begin() requires(!(__detail::__simple_view<_Vs> && ...))
> + begin() requires (!(__detail::__simple_view<_Vs> && ...))
> {
> iterator<false> __it(this, in_place_index<0>,
> ranges::begin(std::get<0>(_M_views)));
> __it.template _M_satisfy<0>();
> @@ -9703,9 +9703,10 @@ namespace ranges
> }
>
> constexpr auto
> - end() requires(!(__detail::__simple_view<_Vs> && ...))
> + end() requires (!(__detail::__simple_view<_Vs> && ...))
> {
> - if constexpr (__detail::__last_is_common<_Vs...>::value)
> + if constexpr ((semiregular<iterator_t<_Vs>> && ...)
> + && __detail::__last_is_common<_Vs...>::value)
> {
> constexpr auto __n = sizeof...(_Vs);
> return iterator<false>(this, in_place_index<__n - 1>,
> @@ -9718,7 +9719,8 @@ namespace ranges
> constexpr auto
> end() const requires (range<const _Vs> && ...) &&
> __detail::__concatable<const _Vs...>
> {
> - if constexpr (__detail::__last_is_common<const _Vs...>::value)
> + if constexpr ((semiregular<iterator_t<const _Vs>> && ...)
> + && __detail::__last_is_common<const _Vs...>::value)
> {
> constexpr auto __n = sizeof...(_Vs);
> return iterator<true>(this, in_place_index<__n - 1>,
> diff --git a/libstdc++-v3/testsuite/std/ranges/concat/1.cc
> b/libstdc++-v3/testsuite/std/ranges/concat/1.cc
> index 6ffe28ece51..511a7e8a858 100644
> --- a/libstdc++-v3/testsuite/std/ranges/concat/1.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/concat/1.cc
> @@ -10,6 +10,7 @@
> #include <algorithm>
> #include <vector>
> #include <array>
> +#include <sstream>
> #include <utility>
> #include <testsuite_hooks.h>
> #include <testsuite_iterators.h>
> @@ -66,9 +67,31 @@ test02()
> VERIFY( ranges::equal(v | views::drop(1), x) );
> }
>
> +void
> +test03()
> +{
> + // LWG 4166 - concat_view::end() should be more constrained in order to
> + // support noncopyable iterators
> + auto range_copyable_it = std::vector<int>{1, 2, 3};
> +
> + std::stringstream ss{"4 5 6"};
> + auto range_noncopyable_it = views::istream<int>(ss);
> +
> + auto view1 = views::concat(range_copyable_it, range_noncopyable_it);
> + static_assert( ranges::range<decltype(view1)> );
> + VERIFY( ranges::equal(view1, std::vector{1, 2, 3, 4, 5, 6}) );
> +
> + ss = std::stringstream{"4 5 6"};
> + range_noncopyable_it = views::istream<int>(ss);
> + auto view2 = views::concat(range_noncopyable_it, range_copyable_it);
> + static_assert( ranges::range<decltype(view2)> );
> + VERIFY( ranges::equal(view2, std::vector{4, 5, 6, 1, 2, 3}) );
> +}
> +
> int
> main()
> {
> static_assert(test01());
> test02();
> + test03();
> }
> --
> 2.47.0.107.g34b6ce9b30
>
