Add ExactChunks::remainder and ExactChunks::into_remainder #51339
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
1 task
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
sdroege
force-pushed
the
exact-chunks-remainder
branch
2 times, most recently
from
62eae2a to
73d609a
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
If someone has suggestions how to make rustdoc happy, please tell me :) In the meantime I'll try different things. Locally the docs links seem fine when rendered, but the linkchecker still complains |
These allow to get the leftover items of the slice that are not being iterated as part of the iterator due to not filling a complete chunk. The mutable version consumes the slice because otherwise we would either a) have to borrow the iterator instead of taking the lifetime of the underlying slice, which is not what *any* of the other iterator functions is doing, or b) would allow returning multiple mutable references to the same data The current behaviour of consuming the iterator is consistent with IterMut::into_slice for the normal iterator.
sdroege
force-pushed
the
exact-chunks-remainder
branch
from
73d609a to
903624f
Compare
sdroege
commented
Jun 4, 2018
| @@ -729,7 +729,8 @@ impl<T> [T] { | |||
| /// Returns an iterator over `chunk_size` elements of the slice at a | |||
| /// time. The chunks are slices and do not overlap. If `chunk_size` does | |||
| /// not divide the length of the slice, then the last up to `chunk_size-1` | |||
| /// elements will be omitted. | |||
| /// elements will be omitted and can be retrieved from the `remainder` | |||
There was a problem hiding this comment.
This and the same for exact_chunks_mut can't be linked to the documentation of the function. It would have to be ../std/slice/etc for the direct documentation here and ../../std/slice/etc for the "copied" documentation in e.g. Vec (for its AsRef<_> impl)
Just omitting the link for now
There was a problem hiding this comment.
#32130 Is the relevant issue for this AFAIU
|
Once this is in it would be possible to implement the other chunks iterators on top of this, which is probably faster than the current ones still. I'll play with that at some point in the next days. |
|
The reviewer is away, maybe someone else from @rust-lang/libs can review this? |
1 similar comment
|
The reviewer is away, maybe someone else from @rust-lang/libs can review this? |
alexcrichton
added
T-libs-api
TimNN
removed
the
S-waiting-on-review
|
Egad sorry for the delay! @bors: r+ |
|
📌 Commit 903624f has been approved by |
bors
added
S-waiting-on-bors
No worries! Thanks for the review and merging :) |
bors
added a commit
that referenced
this pull request
Jul 12, 2018
Add ExactChunks::remainder and ExactChunks::into_remainder These allow to get the leftover items of the slice that are not being iterated as part of the iterator due to not filling a complete chunk. The mutable version consumes the slice because otherwise we would either a) have to borrow the iterator instead of taking the lifetime of the underlying slice, which is not what *any* of the other iterator functions is doing, or b) would allow returning multiple mutable references to the same data The current behaviour of consuming the iterator is consistent with IterMut::into_slice for the normal iterator. ---- This is related to #47115 (comment) and the following comments. While there the discussion was first about a way to get the "tail" of the iterator (everything from the slice that is still not iterated yet), this gives kind of unintuitive behaviour and is inconsistent with how the other slice iterators work. Unintuitive because the `next_back` would have no effect on the tail (or otherwise the tail could not include the remainder items), inconsistent because a) generally the idea of the slice iterators seems to be to only ever return items that were not iterated yet (and don't provide a way to access the same item twice) and b) we would return a "flat" `&[T]` slice but the iterator's shape is `&[[T]]` instead, c) the mutable variant would have to borrow from the iterator instead of the underlying slice (all other iterator functions borrow from the underlying slice!) As such, I've only implemented functions to get the remainder. This also allows the implementation to be completely safe still (and around slices instead of raw pointers), while getting the tail would either be inefficient or would have to be implemented around raw pointers. CC @Kerollmops
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These allow to get the leftover items of the slice that are not being
iterated as part of the iterator due to not filling a complete chunk.
The mutable version consumes the slice because otherwise we would either
a) have to borrow the iterator instead of taking the lifetime of
the underlying slice, which is not what any of the other iterator
functions is doing, or
b) would allow returning multiple mutable references to the same data
The current behaviour of consuming the iterator is consistent with
IterMut::into_slice for the normal iterator.
This is related to #47115 (comment) and the following comments.
While there the discussion was first about a way to get the "tail" of the iterator (everything from the slice that is still not iterated yet), this gives kind of unintuitive behaviour and is inconsistent with how the other slice iterators work.
Unintuitive because the
next_backwould have no effect on the tail (or otherwise the tail could not include the remainder items), inconsistent because a) generally the idea of the slice iterators seems to be to only ever return items that were not iterated yet (and don't provide a way to access the same item twice) and b) we would return a "flat"&[T]slice but the iterator's shape is&[[T]]instead, c) the mutable variant would have to borrow from the iterator instead of the underlying slice (all other iterator functions borrow from the underlying slice!)As such, I've only implemented functions to get the remainder. This also allows the implementation to be completely safe still (and around slices instead of raw pointers), while getting the tail would either be inefficient or would have to be implemented around raw pointers.
CC @Kerollmops