-
-
Notifications
You must be signed in to change notification settings - Fork 749
[RFC] Make the behavior of calling range primitives clear #4511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,16 +123,33 @@ r.popFront(); // can invoke popFront() | |
| auto h = r.front; // can get the front of the range of non-void type | ||
| ---- | ||
|
|
||
| The semantics of an input range (not checkable during compilation) are | ||
| assumed to be the following ($(D r) is an object of type $(D R)): | ||
|
|
||
| $(UL $(LI $(D r.empty) returns $(D false) iff there is more data | ||
| available in the range.) $(LI $(D r.front) returns the current | ||
| element in the range. It may return by value or by reference. Calling | ||
| $(D r.front) is allowed only if calling $(D r.empty) has, or would | ||
| have, returned $(D false).) $(LI $(D r.popFront) advances to the next | ||
| element in the range. Calling $(D r.popFront) is allowed only if | ||
| calling $(D r.empty) has, or would have, returned $(D false).)) | ||
| The following are rules of input ranges are assumed to hold true in all | ||
| Phobos code. These rules are not checkable at compile-time, so not conforming | ||
| to these rules when writing ranges or range based code will result in | ||
| undefined behavior. | ||
|
|
||
| $(UL | ||
| $(LI `r.empty` returns `false` if and only if there is more data | ||
| available in the range.) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "iff" is math speak. Maybe "if and only if" is more appropriate here?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was in the original. I think most programmers know what iff means.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @qznc that our docs shouldn't require math background - at least for such basic and common rules. They should be easy to understand for everyone out there.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's just go with "if and only if". It's come before, and if a couple of us mention it it's enough. Nobody will raise a brow at the explicit version. |
||
| $(LI `r.empty` evaluated multiple times, without calling | ||
| `r.popFront`, or otherwise mutating the range object or the | ||
| underlying data, yields the same result for every evaluation.) | ||
| $(LI `r.front` returns the current element in the range. | ||
| It may return by value or by reference.) | ||
| $(LI `r.front` can be legally evaluated if and only if evaluating | ||
| `r.empty` has, or would have, equaled `false`.) | ||
| $(LI `r.front` evaluated multiple times, without calling | ||
| `r.popFront`, or otherwise mutating the range object or the | ||
| underlying data, yields the same result for every evaluation.) | ||
| $(LI `r.popFront` advances to the next element in the range.) | ||
| $(LI `r.popFront` can be called if and only if evaluating `r.empty` | ||
| has, or would have, equaled `false`.) | ||
| ) | ||
|
|
||
| Also, note that Phobos code assumes that the primitives `r.front` and | ||
| `r.empty` are $(BIGOH 1) time complexity wise or "cheap" in terms of | ||
| running time. $(BIGOH) statements in the documentation of range functions | ||
| are made with this assumption. | ||
|
|
||
| Params: | ||
| R = type to be tested | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/undefined/undefined unless specified otherwise by a range type/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean derivative range types like bidirectional and random access?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, not kind. E.g. I mean "I define
MyRangethat does this and that."There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why this distinction should be made here. If a
MyRangeviolates these rules then a lot of the code in std.range isn't going to work, and they're allowed to throw anError, andException, or corrupt memory, etc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping @andralex
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Andrei is saying that a range type might always strengthen the guarantees to actually make the behaviour defined. But, of course, generic code still cannot make any such assumptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought so, but wanted to check whether the text should be modified accordingly, or is this PR ready to merge?