-
Notifications
You must be signed in to change notification settings - Fork 4
Description
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})); // errorThe 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?