On 10/19/2017 03:12 PM, Steven Schveighoffer wrote:
> On 10/19/17 7:13 AM, Martin Nowak wrote:
>> On 10/13/2017 08:39 PM, Steven Schveighoffer wrote:
>>> What would be nice is a mechanism to detect this situation, since the
>>> above is both un-@safe and incorrect code.
>>>
>>> Possibly you could instrument a window with a mechanism to check to see
>>> if it's still correct on every access, to be used when compiled in
>>> non-release mode for checking program correctness.
>>>
>>> But in terms of @safe code in release mode, I think the only option is
>>> really to rely on the GC or reference counting to allow the window to
>>> still exist.
>>
>> We should definitely find a @nogc solution to this, but it's a good
>> litmus test for the RC compiler support I'll work on.
>> Why do IOPipe have to hand over the window to the caller?
>> They could just implement the RandomAccessRange interface themselves.
>>
>> Instead of
>> ```d
>> auto w = f.window();
>> f.extend(random());
>> w[0];
>> ```
>> you could only do
>> ```d
>> f[0];
>> f.extend(random());
>> f[0]; // bug, but no memory corruption
>> ```
> 
> So the idea here (If I understand correctly) is to encapsulate the
> window into the pipe, such that you don't need to access the buffer
> separately? I'm not quite sure because of that last comment. If f[0] is
> equivalent to previous code f.window[0], then the second f[0] is not a
> bug, it's valid, and accessing the first element of the window (which
> may have moved).

The above sample with the window is a bug and memory corruption because
of iterator/window invalidation by extend.
If you didn't thought of the invalidation, then the latter example would
still be a bug to you, but not a memory corruption.

>> This problem seems to be very similar to the Range vs. Iterators
>> difference, the former can perform bounds checks on indexing, the later
>> are inherently unsafe (with expensive runtime debug checks e.g. in VC++).
> 
> But ranges have this same problem.
> 
> For instance:
> const(char[])[] lines = stdin.byLine.array;
> 
> Here, since byLine uses GC buffering, it's @safe (but wrong). If non-GC
> buffers are used, then it's not @safe.
> 
> I think as long as the windows are backed by GC data, it should be
> @safe. In this sense, your choice of buffering scheme can make something
> @safe or not @safe. I'm OK with that, as long as iopipes can be @safe in
> some way (and that happens to be the default).
> 
>> Similarly always accessing the buffer through IOPipe would allow cheap
>> bounds checking, and sure you could still offer IOPipe.ptr for unsafe
>> code.
> 
> It's an interesting idea to simply make the iopipe the window, not just
> for @safety reasons:
> 
> 1. this means the iopipe itself *is* a random access range, allowing it
> to automatically fit into existing algorithms.
> 2. Existing random-access ranges can be easily shoehorned into being
> ranges (I already did it with arrays, and it's not much harder with
> popFrontN). Alternatively, code that uses iopipes can simply check for
> the existence of iopipe-like methods, and use them if they are present.
> 3. Less verbose usage, and more uniform access. For instance if an
> iopipe defines opIndex, then iopipe.window[0] and iopipe[0] are possibly
> different things, which would be confusing.
> 
> Some downsides however:
> 
> 1. iopipes can be complex and windows are not. They were a fixed view of
> the current buffer. The idea that I can fetch a window of data from an
> iopipe and then deal simply with that part of the data was attractive.

You could still have a window internally and just forward to that.

> 2. The iopipe is generally not copyable once usage begins. In other
> words, the feature of ranges that you can copy them and they just work,
> would be difficult to replicate in iopipe.

That's a general problem. Unique ownership is really useful, but most
phobos range methods don't care, and assume copying is implicit saving.
Not too nice and I guess this will bite us again with RC/Unique/Weak.

The current workaround for this is `refRange`.

> A possible way forward could be:
> 
> * iopipe is a random-access range (not necessarily a forward range).
> * iopipe.window returns a non-extendable window of the buffer itself,
> which is a forward/random-access range. If backed by the GC or some form
> of RC, everything is @safe.
> * Functions which now take iopipes could be adjusted to take
> random-access ranges, and if they are also iopipes, could use the extend
> features to get more data.
> * iopipe.release(size_t) could be hooked by popFrontN. I don't like the
> idea of supporting slicing on iopipes, for the non-forward aspect of
> iopipe. Much better to have an internal hook that modifies the range
> in-place.
> 
> This would make iopipes fit right into the range hierarchy, and
> therefore could be integrated easily into Phobos.

I made an interesting experiment with buffered input ranges quite a
while ago.
https://gist.github.com/MartinNowak/1257196

This would use popFront to fetch new data and ref-counts a list of
buffers depending on older saved ranges still using earlier buffers.
With a bit of creative use, the existing Range primitives could be used
to implement infinite look-ahead.

auto beg = rng.save;
auto end = rng.find("bla");
auto window = beg[0 .. end]; // get a random access window

The main problem with this has been, that the many implicit copies (e.g.
in foreach) bump the reference-count, so the RC buffer release would
often not work.
Could be avoided by making them non-copyable, but again phobos and
foreach currently don't support this hybrid of input (consuming) and
forward (saveable) range.

-Martin

Reply via email to