`concat_view::end()` should be more constrained in order to support non-copyable iterators

[yaito3014]

I found a possible issue in concat_view's implementation.

There is a case that concat(a, b) compiles but concat(b, a) does not.

auto range_copyable_it = std::vector<int>{1, 2, 3};

std::stringstream ss{"4 5 6"};
auto range_noncopyable_it = std::views::istream<int>(ss);

auto view1 = std::views::concat(range_copyable_it, range_noncopyable_it);
static_assert(std::ranges::range<decltype(view1)>);               // ok
assert(std::ranges::equal(view1, std::vector{1, 2, 3, 4, 5, 6})); // ok

auto view2 = std::views::concat(range_noncopyable_it, range_copyable_it);
// static_assert(std::ranges::range<decltype(view2)>);               // error
// assert(std::ranges::equal(view2, std::vector{4, 5, 6, 1, 2, 3})); // error

The reason behind this is as follows:

Firstly, if all Views... satisfy the std::ranges::range concept, then concat_view should also satisfy it. However, if any of the Views... have a non-copyable iterator and the last view is common_range, the proposed concat_view fails to model a range.

For concat_view to model a range, its sentinel must satisfy std::semiregular, but concat_view::end() returns a concat_view::iterator, which is non-copyable if the underlying iterator is non-copyable. This issue arises from the proposed implementation, where the iterator uses std::variant. Although this specification is exposition-only, even if an alternative type-erasure mechanism is used, copying is still required if the user attempts to copy an iterator.

To resolve the issue, concat_view::end() can fallback to returning std::default_sentinel in such cases.

// before
if constexpr (common_range<maybe-const<is-const, Views...[N - 1]>>) {
// after
if constexpr ((semiregular<iterator_t<Views>> && ...) && common_range<maybe-const<is-const, Views...[N - 1]>>) {

Unfortunately, as a side effect, this fix would prevent concat_view from being a common_range in certain situations.
According to P2542R8:

concat_view can be common_range if the last underlying range models common_range

However, this is no longer true after applying my fix. On the other hand, the current draft (N4988) does not mention when concat_view can model common_range. I assume this omission was for simplicity, but I think the new behavior should be documented as a side note.

That said, these two issues cannot be resolved simultaneously due to implementability.
Therefore, I suggest applying the fix regardless and accepting that concat_view will not always inherit common_range.

I think this issue should be re-evaluated by LEWG or LWG before C++26 is shipped.
Could I ask you to review the problem as the author?

[huixie90]

Hello, @yaito3014 , thanks for reporting the issue. Could you please submit an LWG issue? For bug fixes, we don’t have to go through LEWG.

https://cplusplus.github.io/LWG/lwg-active.html#submit_issue
The email address for submitting issue is on the top of that page

the issue will be then discussed in the reflector and if enough people voted P0, potentially it be tentatively ready for next committee meeting

[yaito3014]

Thank you for response. I'm currently writing an email. 🚀

P.S. I have submitted the issue to the list.

[saki7]

Hi, I'm the co-author of this report.
We're happy to inform you that our report has been published on the LWG active issues list, and GCC has already implemented the resolution with it being "slated to get approved as-is."

• LWG4166
• [PATCH] libstdc++: Implement LWG 4166 changes to concat_view::end()
  • GitHub mirror: gcc-mirror/gcc@f191c83

@huixie90 I believe we can mark this issue as resolved. Thank you for your attention, and for guiding us through the proper procedure.

[huixie90]

@saki7 @yaito3014 after more than a year, this LWG issue is finally Tentatively Ready. It has a new resolution as more issues have been found during discussion on the refactor (see LWG issue link above). It will very likely be voted in and merged in working draft in two weeks time in Kona