printing a range of ranges consumes sub-ranges

[dlangBugzillaToGithub]

schveiguy (@schveiguy) reported this on 2024-05-27T14:57:08Z

Transfered from https://issues.dlang.org/show_bug.cgi?id=24570

CC List

• salihdb

Description

If you are printing something, you might correctly not expect the act of printing it to modify that type.

However, printing a range *requires* that you modify it. It's how you get at the elements inside. For this reason, a range must be treated as a *view* of data, not the thing to print.

If you print a range whose elements are ranges, those elements should behave as if you printed them. That is, the act of printing them should not modify the elements themselves.

However, currently `formatValue` will accept range elements by reference, and print them by iterating them. This means that a range-of-ranges where the outer range uses lvalue elements will consume all the nested ranges.

An example:

```d
import std.range;
import std.conv;
import std.algorithm;
struct R
{
    int* ptr;
    size_t len;
    int front() {return  *ptr;}
    void popFront() { ++ptr; --len; }
    bool empty() {return len == 0;}
    typeof(this) save() { return this; }
}

void main()
{
    import std.conv;
    auto intarr = [1, 2, 3];
    auto r = R(intarr.ptr, intarr.length);
    assert(text(r) == "[1, 2, 3]"); // ok, r is passed by value
    assert(text(r) == "[1, 2, 3]"); // still there
    auto ndarr = [r, r]; // a range of ranges
    assert(text(ndarr) == "[[1, 2, 3], [1, 2, 3]]"); // ok, but this consumed the subranges
    assert(text(ndarr) == "[[], []]"); // now they are gone
    ndarr = [r, r];

    // what phobos should do, is save every forward range before printing
    assert(text(ndarr.map!(e => e.save)) == "[[1, 2, 3], [1, 2, 3]]"); // ok
    assert(text(ndarr.map!(e => e.save)) == "[[1, 2, 3], [1, 2, 3]]"); // ok
}
```

Note: I used text instead of writeln so the asserts can be written, but the same thing happens with writeln.

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2024-05-27T14:58:33Z

A further note is that an array of arrays is not consumed when printed -- because formatValue has a specialized case for that.

[dlangBugzillaToGithub]

salihdb commented on 2024-05-28T06:05:31Z

(In reply to Steven Schveighoffer from comment #1)
> A further note is that an array of arrays is not consumed when printed --
> because formatValue has a specialized case for that.

If you do the following instead of `auto save() => return this;`, the problem is solved:

```
struct R
{
  wchar* ptr;
  size_t len;
  
  this(T)(T[] range)
  {
    ptr = cast(wchar*)range.ptr;
    len = range.length;
  }
  
  auto empty() => len == 0;
  auto front() => *ptr++;
  auto popFront() => len--;
  auto save()
  {
    auto r = R([]);
    r.len = len;
    r.ptr = ptr;
    return r;
  }
}

void main()
{
  auto c = ['€', '₺', '₽'];
  auto r = R(c);
  auto arr = [r, r, r];

  assert(!arr.empty);
  
  import std.conv : text;
  auto str = arr.text; // "€₺₽"
  
  assert(!arr.empty);
}
```

[dlangBugzillaToGithub]

salihdb commented on 2024-05-28T06:33:51Z

There's no reason why this issue can't be easily fixed. Because when you include narrow string or wchar, there is no problem of not being able to save(): https://forum.dlang.org/post/qgtrcupsniezqgazkztd@forum.dlang.org

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2024-05-28T23:29:14Z

(In reply to Salih Dincer from comment #2)

> If you do the following instead of `auto save() => return this;`, the
> problem is solved:
> 
> ```
> struct R
> {
>   wchar* ptr;
>   size_t len;
>   
>   this(T)(T[] range)
>   {
>     ptr = cast(wchar*)range.ptr;
>     len = range.length;
>   }
>   
>   auto empty() => len == 0;
>   auto front() => *ptr++;

this is an invalid implementation. Using `front` more than once is supported.

>   auto popFront() => len--;
>   auto save()
>   {
>     auto r = R([]);
>     r.len = len;
>     r.ptr = ptr;
>     return r;
>   }
> }
> 
> void main()
> {
>   auto c = ['€', '₺', '₽'];
>   auto r = R(c);
>   auto arr = [r, r, r];
> 
>   assert(!arr.empty);
>   
>   import std.conv : text;
>   auto str = arr.text; // "€₺₽"
>   
>   assert(!arr.empty);

But it was never arr that was empty. It's just an array of empty ranges at this point.

But the difference here is that you are using wchar and not short. format treats character data differently (as evidenced by the printout, it's not an array of items, but a string).

Yet another reason why this bug has gone undetected for so long.