On Wed, 5 Mar 2025 at 15:37, Patrick Palkawrote:
>
> On Mon, 17 Feb 2025, Patrick Palka wrote:
>
> > Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> >
> > -- >8 --
> >
> > - Use __builtin_unreachable to suppress a false-positive "control
> > reaches end of non-void function" warning in the recursive lambda
> > (which the existing tests failed to notice since test01 wasn't
> > being called at runtime)
> > - Relax the constraints on views::concat in the single-argument case
> > as per PR115215
> > - Add an input_range requirement to that same case as per LWG 4082
> > - In the const-converting constructor of concat_view's iterator,
> > don't require the first iterator to be default constructible
> >
> > PR libstdc++/115215
> > PR libstdc++/115218
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/std/ranges
> > (concat_view::iterator::_S_invoke_with_runtime_index): Use
> > __builtin_unreachable in recursive lambda to certify it always
> > exits via 'return'.
> > (concat_view::iterator::iterator): In the const-converting
> > constructor, direct initialize _M_it.
> > (views::_Concat::operator()): Adjust constraints in the
> > single-argument case as per LWG 4082.
> > * testsuite/std/ranges/concat/1.cc (test01): Call it at runtime
> > too.
> > (test04): New test.
> > ---
> > libstdc++-v3/include/std/ranges | 28 +++++++++----------
> > libstdc++-v3/testsuite/std/ranges/concat/1.cc | 16 +++++++++++
> > 2 files changed, 30 insertions(+), 14 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/std/ranges
> > b/libstdc++-v3/include/std/ranges
> > index 22e0c9cae44..a56dae43625 100644
> > --- a/libstdc++-v3/include/std/ranges
> > +++ b/libstdc++-v3/include/std/ranges
> > @@ -9919,6 +9919,7 @@ namespace ranges
> > return __f.template operator()<_Idx>();
> > if constexpr (_Idx + 1 < sizeof...(_Vs))
> > return __self.template operator()<_Idx + 1>();
> > + __builtin_unreachable();
> > }.template operator()<0>();
> > }
> >
> > @@ -9940,12 +9941,12 @@ namespace ranges
> > constexpr
> > iterator(iterator<!_Const> __it)
> > requires _Const && (convertible_to<iterator_t<_Vs>, iterator_t<const
> > _Vs>> && ...)
> > - : _M_parent(__it._M_parent)
> > - {
> > - _M_invoke_with_runtime_index([this, &__it]<size_t _Idx>() {
> > - _M_it.template emplace<_Idx>(std::get<_Idx>(std::move(__it._M_it)));
> > - });
> > - }
> > + : _M_parent(__it._M_parent),
> > + _M_it(_S_invoke_with_runtime_index([this, &__it]<size_t _Idx>() {
> > + return __base_iter(in_place_index<_Idx>,
> > + std::get<_Idx>(std::move(__it._M_it)));
> > + }, __it._M_it.index()))
> > + { }
> >
> > constexpr decltype(auto)
> > operator*() const
> > @@ -10179,16 +10180,15 @@ namespace ranges
> >
> > struct _Concat
> > {
> > - template<typename... _Ts>
> > - requires __detail::__can_concat_view<_Ts...>
> > + template<__detail::__can_concat_view... _Ts>
>
> As pointed out by Tomasz, this change is incorrect and unnecessary.
> The former checks __can_concat_view<_Ts...> whereas the latter checks
> (__can_concat_view<Ts> && ...). Here's v2 with this change reverted:
I was just looking at that exact line and scratching my head.
Patch v2 is OK for trunk, thanks.
>
> -- >8 --
>
> Subject: [PATCH v2] libstdc++: Some concat_view bugfixes [PR115215, PR115218,
> LWG
> 4082]
>
> - Use __builtin_unreachable to suppress a false-positive "control
> reaches end of non-void function" warning in the recursive lambda
> (which the existing tests failed to notice since test01 wasn't
> being called at runtime)
> - Relax the constraints on views::concat in the single-argument case
> as per PR115215
> - Add an input_range requirement to that same case as per LWG 4082
> - In the const-converting constructor of concat_view's iterator,
> don't require the first iterator to be default constructible
>
> PR libstdc++/115215
> PR libstdc++/115218
>
> libstdc++-v3/ChangeLog:
>
> * include/std/ranges
> (concat_view::iterator::_S_invoke_with_runtime_index): Use
> __builtin_unreachable in recursive lambda to certify it always
> exits via 'return'.
> (concat_view::iterator::iterator): In the const-converting
> constructor, direct initialize _M_it.
> (views::_Concat::operator()): Adjust constraints in the
> single-argument case as per LWG 4082.
> * testsuite/std/ranges/concat/1.cc (test01): Call it at runtime
> too.
> (test04): New test.
> ---
> libstdc++-v3/include/std/ranges | 25 ++++++++++---------
> libstdc++-v3/testsuite/std/ranges/concat/1.cc | 16 ++++++++++++
> 2 files changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index 6c65722b687..2228ab8c5ee 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -9919,6 +9919,7 @@ namespace ranges
> return __f.template operator()<_Idx>();
> if constexpr (_Idx + 1 < sizeof...(_Vs))
> return __self.template operator()<_Idx + 1>();
> + __builtin_unreachable();
> }.template operator()<0>();
> }
>
> @@ -9940,12 +9941,12 @@ namespace ranges
> constexpr
> _Iterator(_Iterator<!_Const> __it)
> requires _Const && (convertible_to<iterator_t<_Vs>, iterator_t<const
> _Vs>> && ...)
> - : _M_parent(__it._M_parent)
> - {
> - _M_invoke_with_runtime_index([this, &__it]<size_t _Idx>() {
> - _M_it.template emplace<_Idx>(std::get<_Idx>(std::move(__it._M_it)));
> - });
> - }
> + : _M_parent(__it._M_parent),
> + _M_it(_S_invoke_with_runtime_index([this, &__it]<size_t _Idx>() {
> + return __base_iter(in_place_index<_Idx>,
> + std::get<_Idx>(std::move(__it._M_it)));
> + }, __it._M_it.index()))
> + { }
>
> constexpr decltype(auto)
> operator*() const
> @@ -10183,12 +10184,12 @@ namespace ranges
> requires __detail::__can_concat_view<_Ts...>
> constexpr auto
> operator() [[nodiscard]] (_Ts&&... __ts) const
> - {
> - if constexpr (sizeof...(_Ts) == 1)
> - return views::all(std::forward<_Ts>(__ts)...);
> - else
> - return concat_view(std::forward<_Ts>(__ts)...);
> - }
> + { return concat_view(std::forward<_Ts>(__ts)...); }
> +
> + template<input_range _Range>
> + constexpr auto
> + operator() [[nodiscard]] (_Range&& __t) const
> + { return views::all(std::forward<_Range>(__t)); }
> };
>
> inline constexpr _Concat concat;
> diff --git a/libstdc++-v3/testsuite/std/ranges/concat/1.cc
> b/libstdc++-v3/testsuite/std/ranges/concat/1.cc
> index e5d10f476e9..16721912a37 100644
> --- a/libstdc++-v3/testsuite/std/ranges/concat/1.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/concat/1.cc
> @@ -85,10 +85,26 @@ test03()
> VERIFY( ranges::equal(view2, std::vector{4, 5, 6, 1, 2, 3}) );
> }
>
> +void
> +test04()
> +{
> + // PR libstdc++/115215 - views::concat rejects non-movable reference
> + int x[] = {1,2,3};
> + struct nomove {
> + nomove() = default;
> + nomove(const nomove&) = delete;
> + };
> + auto v = x | views::transform([](int) { return nomove{}; });
> + using type = decltype(views::concat(v));
> + using type = decltype(v);
> +}
> +
> int
> main()
> {
> static_assert(test01());
> + test01();
> test02();
> test03();
> + test04();
> }
> --
> 2.49.0.rc0.57.gdb91954e18
>
