Implement more Iterator methods on core::iter::Repeat #85338
Conversation
`core::iter::Repeat` always returns the same element, which means we can do better than implementing most `Iterator` methods in terms of `Iterator::next`. Fixes #81292.
|
r? @dtolnay (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| } | ||
|
|
||
| fn last(self) -> Option<A> { | ||
| loop {} |
There was a problem hiding this comment.
Wouldn't self.element be the last one?
Just optimized from running infinite time to a no-op :D
There was a problem hiding this comment.
Repeat is an infinite iterator so it has no last element.
There was a problem hiding this comment.
But it has last element according to next_back implementation.
There was a problem hiding this comment.
Can we please have this behavior instead? I am using it to represent something like [[*] in tr where it expands however much is required. To make this easier I need last() to actually return something to act as a fallback in case the sequences in set1 is longer than set2.
Is there something to be gain from this being an infinite loop?
There was a problem hiding this comment.
This code does not change the behavior of repeat when calling last from Repeat as it existed prior to this PR. The default implementation it replaces calls next until None is returned, which never happens for Repeat.
|
@bors r+ |
|
📌 Commit 963bd3b has been approved by |
bors
added
S-waiting-on-bors
…gh-81292, r=joshtriplett Implement more Iterator methods on core::iter::Repeat `core::iter::Repeat` always returns the same element, which means we can do better than implementing most `Iterator` methods in terms of `Iterator::next`. Fixes rust-lang#81292. rust-lang#81292 raises the question of whether these changes violate the contract of `core::iter::Repeat`, but as far as I can tell `core::iter::repeat` doesn't make any guarantees around how it calls `Clone::clone`.
…laumeGomez Rollup of 7 pull requests Successful merges: - rust-lang#84587 (rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (`INVALID_RUST_CODEBLOCKS`)) - rust-lang#85280 (Toggle-wrap items differently than top-doc.) - rust-lang#85338 (Implement more Iterator methods on core::iter::Repeat) - rust-lang#85339 (Report an error if a lang item has the wrong number of generic arguments) - rust-lang#85369 (Suggest borrowing if a trait implementation is found for &/&mut <type>) - rust-lang#85393 (Suppress spurious errors inside `async fn`) - rust-lang#85415 (Clean up remnants of BorrowOfPackedField) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
| fn advance_by(&mut self, n: usize) -> Result<(), usize> { | ||
| // Advancing an infinite iterator of a single element is a no-op. | ||
| let _ = n; | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[inline] | ||
| fn nth(&mut self, n: usize) -> Option<A> { | ||
| let _ = n; |
There was a problem hiding this comment.
These two functions, as well as their counterparts in DoubleEndedIterator are incorrect. It changes behavior because there can be an arbitrary Drop impl for A, which means the side effects are removed by this PR.
10 participants
core::iter::Repeatalways returns the same element, which means we cando better than implementing most
Iteratormethods in terms ofIterator::next.Fixes #81292.
#81292 raises the question of whether these changes violate the contract of
core::iter::Repeat, but as far as I can tellcore::iter::repeatdoesn't make any guarantees around how it callsClone::clone.