New issue from Hewill: "The past end issue for lazy_split_view"

Author: Dani-Hub

xml/issue4249.xml

@@ -0,0 +1,185 @@
+<?xml version='1.0' encoding='utf-8' standalone='no'?>
+<!DOCTYPE issue SYSTEM "lwg-issue.dtd">
+
+<issue num="4249" status="New">
+<title>The past end issue for `lazy_split_view`</title>
+<section>
+<sref ref="[range.lazy.split.outer]"/>
+</section>
+<submitter>Hewill Kang</submitter>
+<date>26 Apr 2025</date>
+<priority>99</priority>
+
+<discussion>
+<p>
+Consider (<a href="https://godbolt.org/z/eeMe8aTqv">demo</a>):
+</p>
+<blockquote><pre>
+#include &lt;print&gt;
+#include &lt;ranges&gt;
+#include &lt;sstream&gt;
+
+int main() {
+  std::istringstream is{"1 0 2 0 3"};
+  auto r = std::views::istream&lt;int&gt;(is)
+         | std::views::lazy_split(0)
+         | std::views::stride(2);
+  std::println("{}", r); // should print [[1], [3]]
+}
+</pre></blockquote>
+<p>
+The above leads to SIGSEGV in libstdc++, the reason is that we are iterating over the nested
+range as:
+</p>
+<blockquote><pre>
+for (auto&amp;&amp; inner : r) {
+  for (auto&amp;&amp; elem : inner) {
+    // [&hellip;]
+  }
+}
+</pre></blockquote>
+<p>
+which is disassembled as:
+</p>
+<blockquote><pre>
+auto outer_it = r.begin();
+std::default_sentinel_t out_end = r.end();
+for(; outer_it != out_end; ++outer_it) {
+  auto&amp;&amp; inner_r = *outer_it;
+  auto inner_it = inner_r.begin();
+  std::default_sentinel_t inner_end = inner_r.end();
+  for(; inner_it != inner_end; ++inner_it) {
+    auto&amp;&amp; elem = *inner_it;
+    // [&hellip;]
+  }
+}
+</pre></blockquote>
+<p>
+Since `inner_it` and `output_it` actually update the same iterator,
+when we back to the outer loop, <code>lazy_split_view::<i>outer-iterator</i></code> 
+is now equal to `default_sentinel`, which makes `output_it` reach the end,
+so `++outer_it` will increment the iterator past end, triggering the assertion.
+</p>
+<p>
+Note that this <a href="https://godbolt.org/z/6Tx61eeq8">also happens</a> in MSVC-STL 
+when `_ITERATOR_DEBUG_LEVEL` is turned on.
+</p>
+<p>
+It seems that extra flags are needed to fix this issue because `output_it` should not 
+be considered to reach the end when we back to the outer loop.
+</p>
+</discussion>
+
+<resolution>
+<p>
+This wording is relative to <paper num="N5008"/>.
+</p>
+
+<ol>
+
+<li><p>Modify <sref ref="[range.lazy.split.outer]"/> as indicated:</p>
+
+<blockquote>
+<blockquote>
+<pre>
+namespace std::ranges {
+  template&lt;input_range V, forward_range Pattern&gt;
+    requires view&lt;V&gt; &amp;&amp; view&lt;Pattern&gt; &amp;&amp;
+             indirectly_comparable&lt;iterator_t&lt;V&gt;, iterator_t&lt;Pattern&gt;, ranges::equal_to&gt; &amp;&amp;
+             (forward_range&lt;V&gt; || <i>tiny-range</i>&lt;Pattern&gt;)
+  template&lt;bool Const&gt;
+  struct lazy_split_view&lt;V, Pattern&gt;::<i>outer-iterator</i> {
+  private:
+    using <i>Parent</i> = <i>maybe-const</i>&lt;Const, lazy_split_view&gt;;     // <i>exposition only</i>
+    using <i>Base</i> = <i>maybe-const</i>&lt;Const, V&gt;;                     // <i>exposition only</i>
+    <i>Parent</i>* <i>parent_</i> = nullptr;                              // <i>exposition only</i>
+
+    iterator_t&lt;<i>Base</i>&gt; <i>current_</i> = iterator_t&lt;<i>Base</i>&gt;();         // <i>exposition only, present only</i>
+                                                            // <i>if V models forward_range</i>
+
+    bool <i>trailing_empty_</i> = false;                           // <i>exposition only</i>
+    <ins>bool <i>has_next_</i> = false;                                 // <i>exposition only, present only</i></ins>
+                                                            <ins>// <i>if forward_range&lt;<i>V</i>&gt; is false</i></ins>
+  public:
+    [&hellip;]
+  };
+}
+</pre>
+</blockquote>
+[&hellip;]
+<pre>
+constexpr explicit <i>outer-iterator</i>(<i>Parent</i>&amp; parent)
+  requires (!forward_range&lt;<i>Base</i>&gt;);
+</pre>
+<blockquote>
+<p>
+-2- <i>Effects:</i> Initializes <code><i>parent_</i></code> with `addressof(parent)` <ins>and
+<code><i>has_next_</i></code> with <code><i>current</i> != ranges::end(<i>parent_</i>-&gt;<i>base_</i>)</code></ins>.
+</p>
+</blockquote>
+[&hellip;]
+<pre>
+constexpr <i>outer-iterator</i>&amp; operator++();
+</pre>
+<blockquote>
+<p>
+-6- <i>Effects:</i> Equivalent to:
+</p>
+<blockquote><pre>
+const auto end = ranges::end(<i>parent_</i>-&gt;<i>base_</i>);
+if (<i>current</i> == end) {
+  <i>trailing_empty_</i> = false;
+  <ins>if constexpr (!forward_range&lt;V&gt;)
+    <i>has_next_</i> = false;</ins>
+  return <code>*this</code>;
+}
+const auto [pbegin, pend] = subrange{<i>parent_</i>-&gt;<i>pattern_</i>};
+if (pbegin == pend) ++<i>current</i>;
+else if constexpr (<i>tiny-range</i>&lt;Pattern&gt;) {
+  <i>current</i> = ranges::find(std::move(<i>current</i>), end, *pbegin);
+  if (<i>current</i> != end) {
+    ++<i>current</i>;
+    if (<i>current</i> == end)
+      <i>trailing_empty_</i> = true;
+  }
+}
+else {
+  do {
+    auto [b, p] = ranges::mismatch(<i>current</i>, end, pbegin, pend);
+    if (p == pend) {
+      <i>current</i> = b;
+      if (<i>current</i> == end)
+        <i>trailing_empty_</i> = true;
+      break;            <i>// The pattern matched; skip it</i>
+    }
+  } while (++<i>current</i> != end);
+}
+<ins>if constexpr (!forward_range&lt;V&gt;)
+  if (<i>current</i> == end)
+    <i>has_next_</i> = false;</ins>
+return *this;
+</pre></blockquote>
+</blockquote>
+[&hellip;]
+<pre>
+friend constexpr bool operator==(const <i>outer-iterator</i>&amp; x, default_sentinel_t);
+</pre>
+<blockquote>
+<p>
+-8- <i>Effects:</i> Equivalent to:
+</p>
+<blockquote><pre>
+<ins>if constexpr (!forward_range&lt;V&gt;)
+  return !x.<i>has_next_</i> &amp;&amp; !x.<i>trailing_empty_</i>;
+else</ins>
+  return x.<i>current</i> == ranges::end(x.<i>parent_</i>-&gt;<i>base_</i>) &amp;&amp; !x.<i>trailing_empty_</i>;
+</pre></blockquote>
+</blockquote>
+</blockquote>
+
+</li>
+
+</ol>
+</resolution>
+
+</issue>