Skip to content

Conversation

aG0aep6G
Copy link
Contributor

+/
auto refRange(R)(R* range)
if(isForwardRange!R && !is(R == class))
if(!is(R == class))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if(isInputRange!R && !is(R == class))
or
static assert(isInputRange!R, "refRange only works with ranges");

Wrong types should be detected as early as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@kuettler
Copy link
Contributor

I suspect this is unhelpful. I might be wrong.

The difference between a ForwardRange and a InputRange is not the mere existence of a save method. There is a huge semantic difference: a InputRange is a range that cannot have a save method, because is consumes input. Think reading from a socket. Therefore, a InputRange already has reference semantics.

Thus, this PR fixes a problem that does not exist. There is no improvement here.

@growlercab
Copy link
Contributor

I believe the ForwardRange is just an InputRange with the save method to take a snapshot of its current state. You can see this in the Phobos snippet:

template isForwardRange(R)
{
    enum bool isForwardRange = isInputRange!R && is(typeof(
    (inout int = 0)
    {
        R r1 = R.init;
        static assert (is(typeof(r1.save) == R));
    }));
}

As to whether this is a useful change, I'll leave that to those better versed in D.

@kuettler
Copy link
Contributor

You are both right and missing the point. The save method is not optional, if it can be implemented. For example, take a look at std.algorithm.iteration.chunkBy to see the difference between ForwardRange and InputRange in action.

@aG0aep6G
Copy link
Contributor Author

a InputRange is a range that cannot have a save method, because is consumes input. Think reading from a socket. Therefore, a InputRange already has reference semantics.

When reading from a socket, you have to save the read data somewhere. It should be fine to store it directly in the range struct. The struct then doesn't have reference semantics: popFront on one copy won't change the front of another copy.

@kuettler
Copy link
Contributor

A InputRange is what is called a One-Pass Range in On Iteration. In contrast to that, a ForwardRange supports "strictly forward iteration over an in-memory entity".

Certainly, you can store data you read from a socket. If you store the data, however, you can (and need to) supply a save method.

@aG0aep6G
Copy link
Contributor Author

A InputRange is what is called a One-Pass Range in On Iteration. In contrast to that, a ForwardRange supports "strictly forward iteration over an in-memory entity".

Certainly, you can store data you read from a socket. If you store the data, however, you can (and need to) supply a save method.

From the article: "IsDone checks for end-of-input and fills a one-element buffer held inside the range object, CurrentItem returns the buffer"

It's that one-element buffer I'm talking about. If it's part of the range struct, then popFront on a copy doesn't alter the original front => no reference semantics.

@kuettler
Copy link
Contributor

You are certainly right here. A one element cache changes the behavior. My perception is, implicit caching in input ranges does not work too well and tends to be squeezed out of phobos. See for example the non-caching map algorithm and the explicit cache in std.algorithm.iteration. In my view algorithms on input ranges should be oblivious to caching (like chunkBy).

Still, I stand corrected. Your patch addresses an issue. I am not convinced the issue should be addressed, however. On the contrary, I feel this does not belong to the standard library. Your mileage may vary.

@MartinNowak
Copy link
Member

Makes sense for InputRanges with buffering, e.g. vibe.d's StreamInputRange.
Buffered InputRanges are the common choice to do I/O, because supporting infinite lookahead to promote an InputRange to a ForwardRange is often difficult and inefficient, e.g. because of unwanted save by copying a forward range.

@MartinNowak
Copy link
Member

Auto-merge toggled on

MartinNowak added a commit that referenced this pull request Apr 17, 2015
fix Issue 14373 - std.range.refRange doesn't work on mere input ranges
@MartinNowak MartinNowak merged commit f4894c6 into dlang:master Apr 17, 2015
@aG0aep6G aG0aep6G deleted the 14373 branch April 19, 2015 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants