-
-
Notifications
You must be signed in to change notification settings - Fork 746
Fix 12564, 14493: add std.range.walkBack to get the last element of an input range #5153
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 |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| Added `std.range.walkBack` which returns the last element of any given range. | ||
|
|
||
| $(REF walkBack, std, range) is intended for input ranges that can only be | ||
| iterated linearly. Optionally to support infinite ranges, a maximal `upTo` | ||
| parameter can be supplied. | ||
|
|
||
| ----- | ||
| import std.algorithm.iteration : splitter; | ||
|
|
||
| assert("a;b;c".splitter(";").walkBack == "c"); | ||
| assert("a;b;c;d;e".splitter(";").walkBack(2) == "b"); | ||
| ----- |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,6 +97,9 @@ $(BOOKTABLE , | |
| $(TR $(TD $(D $(LREF walkLength))) | ||
| $(TD Computes the length of any _range in O(n) time. | ||
| )) | ||
| $(TR $(TD $(D $(LREF walkBack))) | ||
| $(TD Returns the back of any _range in O(n) time. | ||
| )) | ||
| ) | ||
|
|
||
| Source: $(PHOBOSSRC std/range/_primitives.d) | ||
|
|
@@ -1605,6 +1608,13 @@ upTo) steps have been taken and returns $(D upTo). | |
|
|
||
| Infinite ranges are compatible, provided the parameter $(D upTo) is | ||
| specified, in which case the implementation simply returns upTo. | ||
|
|
||
| Params | ||
| r = range to walk through | ||
|
|
||
| Returns: the length of the range | ||
|
|
||
| See_Also: $(REF walkBack, std, range, primitives) | ||
| */ | ||
| auto walkLength(Range)(Range range) | ||
| if (isInputRange!Range && !isInfinite!Range) | ||
|
|
@@ -1663,6 +1673,154 @@ if (isInputRange!Range) | |
| assert(fibs.walkLength(55) == 55); | ||
| } | ||
|
|
||
| /** | ||
| This is a best-effort implementation of `back` for any kind of | ||
| range. It expects a non-empty range. | ||
|
|
||
| If `isBidirectional!Range`, simply returns `range.back` without | ||
| checking `upTo) (when specified). | ||
|
|
||
| Otherwise, walks the range through its length and returns the last element seen. | ||
| Performes $(BIGOH n) evaluations of `range.empty` and `range.popFront()`, | ||
| where `n` is the effective length of `range`. | ||
|
|
||
| The `upTo` parameter is useful to "cut the losses" in case | ||
| the interest is in seeing whether the range has at least some number | ||
| of elements. If the parameter `upTo` is specified, stops if `upTo` | ||
| steps have been taken and returns `upTo`. | ||
|
|
||
| Infinite ranges are compatible, provided the parameter `upTo` is | ||
| specified, in which case the implementation simply returns `upTo`. | ||
|
|
||
| Params: | ||
| range = range to walk through | ||
|
|
||
| Returns: last element of the range | ||
|
|
||
| See_Also: $(REF walkLength, std, range, primitives) | ||
| */ | ||
| auto walkBack(Range)(Range range) | ||
| if ((isInputRange!Range || isIterable!Range) && !isInfinite!Range) | ||
| in | ||
| { | ||
|
|
||
| assert(!range.empty, "Can't walkBack an empty range"); | ||
| } | ||
| body | ||
| { | ||
| static if (isBidirectionalRange!Range) | ||
| { | ||
| return range.back; | ||
| } | ||
| else | ||
| { | ||
| // Rebindable doesn't support all types as of now | ||
| Unqual!(ElementType!Range) last; | ||
| foreach (e; range) | ||
| last = cast(Unqual!(ElementType!Range)) e; | ||
|
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. This can be done without Unqual or Rebindable. while(!range.empty){
auto element = range.front;
range.popFront();
if(range.empty) return element;
}
Contributor
Author
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. Yeah, but unfortunately that is slower:
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'm having issues testing using your code with DMD. As it was, the first I fixed it by preceding each call with I would've expected that rewriting like so might make their performance more similar, but I'm surprised the compiler wasn't able to optimize the structure to look more like this anyway: bool empty = range.empty;
while(!empty){
auto element = range.front;
range.popFront();
empty = range.empty;
if(empty) return element;
}@UplinkCoder can you explain what's going on 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. the foreachloop should be rewritten to this:
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. Rather this: |
||
|
|
||
| return cast(ElementType!Range) last; | ||
| } | ||
| } | ||
|
|
||
| /// ditto | ||
| auto walkBack(Range)(Range range, size_t upTo) | ||
| if ((isInputRange!Range || isIterable!Range)) | ||
| in | ||
| { | ||
|
|
||
| assert(!range.empty, "Can't walkBack an empty range"); | ||
| static if (!isBidirectionalRange!Range) | ||
| { | ||
| assert(upTo > 0, "walkBack needs to see at least one element"); | ||
| } | ||
| } | ||
| body | ||
| { | ||
| static if (isBidirectionalRange!Range) | ||
| { | ||
| return range.back; | ||
| } | ||
| else | ||
| { | ||
| // Rebindable doesn't support all types as of now | ||
| Unqual!(ElementType!Range) last; | ||
| for ( ; upTo > 0 && !range.empty; range.popFront(), upTo--) | ||
| last = cast(Unqual!(ElementType!Range)) range.front; | ||
|
|
||
| return cast(ElementType!Range) last; | ||
| } | ||
| } | ||
|
|
||
| /// | ||
| @safe nothrow pure unittest | ||
| { | ||
| import std.algorithm.iteration : splitter; | ||
|
|
||
| assert([1, 2, 3].walkBack == 3); | ||
| assert("a;b;c".splitter(";").walkBack == "c"); | ||
| assert("a;b;c;d;e".splitter(";").walkBack(2) == "b"); | ||
| } | ||
|
|
||
| // @nogc | ||
| @safe @nogc nothrow pure unittest | ||
| { | ||
| import std.algorithm.iteration : filter; | ||
|
|
||
| static immutable arr = [1, 2, 3]; | ||
| assert(arr.walkBack == 3); | ||
| assert(arr.walkBack(1) == 3); // arr isBidirectional | ||
| assert(arr.filter!(a => a < 10).walkBack(1) == 1); | ||
| assert(arr.filter!(a => a < 10).walkBack(4) == 3); | ||
| } | ||
|
|
||
| // all dummy ranges | ||
| @safe nothrow pure unittest | ||
| { | ||
| import std.internal.test.dummyrange : AllDummyRanges; | ||
|
|
||
| foreach (DummyType; AllDummyRanges) | ||
| { | ||
| DummyType dummyRange; | ||
| assert(dummyRange.walkBack == 10); | ||
| static if (isBidirectionalRange!DummyType) | ||
| assert(dummyRange.walkBack(2) == 10); | ||
| else | ||
| assert(dummyRange.walkBack(2) == 2); | ||
|
|
||
| assert(dummyRange.walkBack(20) == 10); | ||
| } | ||
| } | ||
|
|
||
| // test const | ||
| @safe nothrow pure unittest | ||
| { | ||
| import std.algorithm.iteration : filter; | ||
|
|
||
| static struct Foo { | ||
| int m; | ||
| this(int m) { this.m = m; } | ||
| } | ||
| const(Foo)[] arr = [Foo(1), Foo(2), Foo(3), Foo(4), Foo(5)]; | ||
| assert(arr.walkBack == Foo(5)); | ||
| assert(arr.walkBack(2) == Foo(5)); // is bidirectional | ||
| assert(arr.filter!(f => f.m < 10).walkBack(2) == Foo(2)); | ||
| assert(arr.filter!(f => f.m < 10).walkBack == Foo(5)); | ||
| } | ||
|
|
||
| // strings | ||
| @safe pure unittest | ||
| { | ||
| import std.algorithm.iteration : filter; | ||
|
|
||
| static assert(is(typeof("abc".walkBack) == dchar)); | ||
| static assert(is(typeof("abc".walkBack(2)) == dchar)); | ||
| assert("a$b€c☢".walkBack == '☢'); | ||
| assert("a$b€c☢".walkBack(2) == '☢'); | ||
| assert("a$b€c☢".filter!(f => true).walkBack == '☢'); | ||
| assert("a$b€c☢".filter!(f => true).walkBack(2) == '$'); | ||
| } | ||
|
|
||
| /** | ||
| Eagerly advances $(D r) itself (not a copy) up to $(D n) times (by | ||
| calling $(D r.popFront)). $(D popFrontN) takes $(D r) by $(D ref), | ||
|
|
||
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.
It would be good to provide another overload which accepts a fallback value returned when the input is empty rather than causing an error
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.
There are at least these options:
.ifEmpty(value)) method that will yield a range of length 1 with the default value if an empty range is passed in and the otherwise the range unmodifiedenforceinstead (generally disliked, because exceptions allocate)seedThe best precedent I found is maxElement, so I guess we should follow this then.
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.
See #5154 for further discussion about handling empty ranges.
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.
orElseyum