4166. concat_view::end() should be more constrained in order to support noncopyable iterators

Section: 25.7.18.2 [range.concat.view] Status: New Submitter: Yaito Kakeyama & Nana Sakisaka Opened: 2024-10-13 Last modified: 2024-10-19

Priority: Not Prioritized

View other active issues in [range.concat.view].

View all other issues in [range.concat.view].

View all issues with New status.

Discussion:

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 noncopyable iterator and the last view is common_range, the current 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 noncopyable if the underlying iterator is noncopyable. 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 and should fallback to returning std::default_sentinel in such cases.

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 our fix. That said, these two issues cannot be resolved simultaneously due to implementability. Therefore, we suggest applying our fix regardless and accepting that concat_view will not always inherit common_range. Note that the current draft (N4988) does not explicitly specify when concat_view can model common_range, so no addition is required for mentioning this point.

A similar issue had been reported as 3385, which was eventually adopted as a C++20 DR. This DR indicates that LWG approved the decision to require copyable in order to model a common_iterator.

Proposed resolution:

This wording is relative to N4993.

1. Modify 25.7.18.2 [range.concat.view] as indicated:

   constexpr auto end() const
     requires (range<const Views> && ...) && concatable<const Views...>;
   

   -7- Effects: Let is-const be true for the const-qualified overload, and false otherwise. Equivalent to:

   constexpr auto N = sizeof...(Views);
   if constexpr ((semiregular<iterator_t<maybe-const<is-const, Views>>> && ...) && 
                 common_range<maybe-const<is-const, Views...[N - 1]>>) {
     return iterator<is-const>(this, in_place_index<N - 1>,
                               ranges::end(std::get<N - 1>(views_)));
   } else {
     return default_sentinel;
   }