New issue from Yaito Kakeyama & Nana Sakisaka: "concat_view::end() should be more constrained in order to support noncopyable iterators"

Author: Dani-Hub

xml/issue4166.xml

@@ -0,0 +1,98 @@
+<?xml version='1.0' encoding='utf-8' standalone='no'?>
+<!DOCTYPE issue SYSTEM "lwg-issue.dtd">
+
+<issue num="4166" status="New">
+<title>`concat_view::end()` should be more constrained in order to support noncopyable iterators</title>
+<section><sref ref="[range.concat.view]"/></section>
+<submitter>Yaito Kakeyama &amp; Nana Sakisaka</submitter>
+<date>13 Oct 2024</date>
+<priority>99</priority>
+
+<discussion>
+<p>
+There is a case that `concat(a, b)` compiles but `concat(b, a)` does not.
+</p>
+<blockquote><pre>
+auto range_copyable_it = std::vector&lt;int&gt;{1, 2, 3};
+
+std::stringstream ss{"4 5 6"};
+auto range_noncopyable_it = std::views::istream&lt;int&gt;(ss);
+
+auto view1 = std::views::concat(range_copyable_it, range_noncopyable_it);
+static_assert(std::ranges::range&lt;decltype(view1)&gt;);               // 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&lt;decltype(view2)&gt;);               // <span style="color:#C80000;font-weight:bolder">error</span>
+// assert(std::ranges::equal(view2, std::vector{4, 5, 6, 1, 2, 3})); // <span style="color:#C80000;font-weight:bolder">error</span>
+</pre></blockquote>
+<p>
+The reason behind this is as follows:
+<p/>
+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.
+<p/>
+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.
+<p/>
+To resolve the issue, `concat_view::end()` can and should fallback to returning `std::default_sentinel` 
+in such cases.
+<p/>
+Unfortunately, as a side effect, this fix would prevent `concat_view` from being a `common_range` in certain 
+situations. According to <paper num="P2542R8"/>:
+</p>
+<blockquote style="border-left: 3px solid #ccc;padding-left: 15px;">
+<p>
+`concat_view` can be `common_range` if the last underlying range models `common_range`
+</p>
+</blockquote>
+<p>
+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 (<paper num="N4988"/>) 
+does not explicitly specify when `concat_view` can model `common_range`, so no addition is required for 
+mentioning this point.
+<p/>
+A similar issue had been reported as <iref ref="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`.
+</p>
+</discussion>
+
+<resolution>
+<p>
+This wording is relative to <paper num="N4993"/>.
+</p>
+
+<ol>
+<li><p>Modify <sref ref="[range.concat.view]"/> as indicated:</p>
+
+<blockquote>
+<pre>
+constexpr auto end() const
+  requires (range&lt;const Views&gt; &amp;&amp; ...) &amp;&amp; <i>concatable</i>&lt;const Views...&gt;;
+</pre>
+<blockquote>
+<p>
+-7- <i>Effects</i>: Let <tt><i>is-const</i></tt> be `true` for the const-qualified overload, and `false` 
+otherwise. Equivalent to:
+</p>
+<blockquote><pre>
+constexpr auto N = sizeof...(Views);
+if constexpr (<ins>(semiregular&lt;iterator_t&lt;<i>maybe-const</i>&lt;<i>is-const</i>, Views&gt;&gt;&gt; &amp;&amp; ...) &amp;&amp;</ins> 
+              common_range&lt;<i>maybe-const</i>&lt;<i>is-const</i>, Views...[N - 1]&gt;&gt;) {
+  return <i>iterator</i>&lt;<i>is-const</i>&gt;(this, in_place_index&lt;N - 1&gt;,
+                            ranges::end(std::get&lt;N - 1&gt;(<i>views_</i>)));
+} else {
+  return default_sentinel;
+}
+</pre></blockquote>
+</blockquote>
+</blockquote>
+</li>
+</ol></resolution>
+
+</issue>