Implement std::iter::once/empty (RFC 771) #25817
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see CONTRIBUTING.md for more information. |
| /// An iterator that yields an element exactly once. | ||
| #[unstable(feature="core", reason = "new addition")] | ||
| pub struct Once<T> { | ||
| inner: ::option::IntoIter<T> |
There was a problem hiding this comment.
I guess a use option at the beginning would be nicer than the global path here.
There was a problem hiding this comment.
::option::IntoIter<T> isn't really necessary. You could just use Option + take().
There was a problem hiding this comment.
I would recommend using IntoIter (or some other iterator adapator) as it allows proxying the next, next_back, and size_hint functions (primarily size_hint)
There was a problem hiding this comment.
That's three levels of indirection (IntoIter -> Item -> Option) but I guess they'll just get optimized away anyways.
|
These iterators should implement |
|
IMHO, they should also implement Default so that structs that include them can derive Default. |
| @@ -3030,6 +3030,45 @@ pub fn repeat<T: Clone>(elt: T) -> Repeat<T> { | |||
| Repeat{element: elt} | |||
| } | |||
|
|
|||
| /// An iterator that yields nothing. | |||
| #[unstable(feature="core", reason = "new addition")] | |||
There was a problem hiding this comment.
Could this use a different feature name than "core" here as well? Something like iter_empty and iter_once should be fine. Also, can you tag the impl blocks below with #[unstable] as well?
|
I agree with @bluss that it'd be nice to see the other common iterator traits implemented for these primitives as well (they should be pretty easy). |
|
@alexcrichton This good? |
|
You could also impl Default on |
| /// An iterator that yields an element exactly once. | ||
| #[unstable(feature="iter_once", reason = "new addition")] | ||
| pub struct Once<T> { | ||
| inner: ::option::IntoIter<T> |
There was a problem hiding this comment.
I guess a
use optionat the beginning would be nicer than the global path here.
There was a problem hiding this comment.
I'd do that if I used it twice, but adding an import to a 3000+ line file to save two colons on one line seemed excessive.
There was a problem hiding this comment.
It's unusual to use a global path, though.
There was a problem hiding this comment.
Alternatively just store Option and return self.inner.take() on next?
|
Thank you @XMPPwocky I've needed these iterators! |
|
This looks good to me, thanks @XMPPwocky! r=me with a squash |
| @@ -167,6 +167,7 @@ mod core { | |||
| pub use marker; | |||
| pub use option; | |||
| pub use iter; | |||
| pub use default; | |||
There was a problem hiding this comment.
Looks like it's not.
On Wed, May 27, 2015 at 3:30 PM, Alex Crichton notifications@github.com
wrote:
In src/libcore/lib.rs
#25817 (comment):@@ -167,6 +167,7 @@ mod core {
pub use marker;
pub use option;
pub use iter;
- pub use default;
Is this still necessary?
—
Reply to this email directly or view it on GitHub
https://github.com/rust-lang/rust/pull/25817/files#r31187466.
|
☔ The latest upstream changes (presumably #25747) made this pull request unmergeable. Please resolve the merge conflicts. |
|
⌛ Testing commit 103e79d with merge e67cab4... |
|
⛄ The build was interrupted to prioritize another pull request. |
| #[test] | ||
| fn test_empty() { | ||
| let mut it = empty::<i32>(); | ||
| assert_eq!(it.next(), None); |
There was a problem hiding this comment.
Maybe add another assert_eq to check that the iterators are fused? And above for once.
Closes #24443.