Fixes in Cycle #1149
Fixes in Cycle #1149
Conversation
| @@ -286,7 +286,7 @@ module std.range; | |||
| public import std.array; | |||
| import core.bitop, core.exception; | |||
| import std.algorithm, std.conv, std.exception, std.functional, | |||
| std.traits, std.typecons, std.typetuple, std.string; | |||
| std.traits, std.typecons, std.typetuple, std.string, std.utf; | |||
|
Rebased. Added fix for: |
|
LGTM, but could you make the |
Done. Also, added 10845 unittest. |
| @@ -3842,7 +3853,7 @@ struct Cycle(Range) | |||
|
|
|||
| static if (is(typeof((cast(const R)_original)[0])) && | |||
| is(typeof((cast(const R)_original).length))) | |||
| { | |||
| { | |||
|
LGTM otherwise. Could you fix that indentation change and rebase so we can finally get this pulled? 6 months is a ridiculously long period of time for something like this to remain open… |
Ping. |
| @property dchar front() const | ||
| { | ||
| import std.utf : decode; | ||
| size_t copy = _index; |
There was a problem hiding this comment.
You're only using this variable once, maybe you should just do:
return decode(_ptr[0 .. R.length], _index);
There was a problem hiding this comment.
The point is that decode takes the index by reference, and increases it to the index after the decoded char. So copy is necessary to not modify _index. That said, instead of re-inventing the whell, I might as well just write:
return _ptr[_index .. $].front;There was a problem hiding this comment.
Ah ref arguments. I think there was actually a pull by Dennis recently which introduced a getRef template or something of that sort.
|
Apart from the comments, this looks good and almost ready to be merged (A squash will be necessary later). |
The problem is that if the cycle is iterated upon for more than size_t.max, the index makes a jump and becomes out of sync, given the modulo approach to indexing. This is especially relevant, since cycle is supposed to be used for circular buffers, which can and are iterated upon for potentially MASSIVE amounts of data. The new approach is to always bind _index between 0 and range.length, so that _index never actually overflows. This has the added advantage of shifting the modulo cost from front to popFront(), which is more efficient. Tweaked a few other things.
|
This pull has gone stale. Closing for re-write. |
Fixes and improves a couple of points in cycle:
The changes mostly shuffles and/or changes the indentation of the code, Github's diff is very bad with this. It may be worth looking into each diff individually to see what changed, and what was just moved for each edit.