-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Default implementation on std::iter::Fuse should not requires Default on the inner iterator #140961
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
Comments
The problem is that @rustbot label +T-libs-api +C-feature-request +A-iterators -needs-triage |
Oh no… that’s what the documentation of |
For a workaround for the original code snippet, assuming you want an empty iterator in the If you don't mind an additional heap allocation and dynamic dispatch, you could use let mut items: Box<dyn Iterator<Item = _>> = match std::fs::read_dir(&path) {
Ok(v) => Box::new(v),
Err(e) if e.kind() == ErrorKind::NotFound => Box::new(std::iter::empty()),
Err(e) => return Err(e),
}; Or you could use let mut items = match std::fs::read_dir(&path) {
Ok(v) => Some(v).into_iter().flatten(),
Err(e) if e.kind() == ErrorKind::NotFound => None.into_iter().flatten(),
Err(e) => return Err(e),
}; Or you could use let mut items = match std::fs::read_dir(&path) {
Ok(v) => Either::Left(v),
Err(e) if e.kind() == ErrorKind::NotFound => Either::Right(std::iter::empty()),
Err(e) => return Err(e),
}; Or, if you are using Rust 1.79.0 or later and depending on how you use let items: &mut dyn Iterator<Item = _> = match std::fs::read_dir(&path) {
Ok(v) => &mut {v}, // the braces are required, to move `v` out of the variable into a temporary whose lifetime can be extended
Err(e) if e.kind() == ErrorKind::NotFound => &mut std::iter::empty(),
Err(e) => return Err(e),
}; |
Thanks for the idea. I don't mind using a workaround but I think it is better to be able to construct |
Change `core::iter::Fuse`'s `Default` impl to do what its docs say it does The [docs on `impl<I: Default> Default for core::iter::Fuse<I>`](https://doc.rust-lang.org/nightly/std/iter/struct.Fuse.html#impl-Default-for-Fuse%3CI%3E) say (as the `I: Default` bound implies) that `Fuse::<I>::default` "Creates a `Fuse` iterator from the default value of `I`". However, the implementation creates a `Fuse` with `Fuse { iter: Default::default() }`, and since the `iter` field is an `Option<I>`, this is actually `Fuse { iter: None }`, not `Fuse { iter: Some(I::default()) }`, so `Fuse::<I>::default()` always returns an empty iterator, even if `I::default()` would not be empty. This PR changes `Fuse`'s `Default` implementation to match the documentation. This will be a behavior change for anyone currently using `Fuse::<I>::default()` where `I::default()` is not an empty iterator[^1], as `Fuse::<I>::default()` will now also not be an empty iterator. (Alternately, the docs could be updated to reflect what the current implementation actually does, i.e. returns an always-exhausted iterator that never yields any items (even if `I::default()` would have yielded items). With this option, the `I: Default` bound could also be removed to reflect that no `I` is ever created.) [Current behavior example](https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=a1e0adc4badca3dc11bfb70a99213249) (maybe an example like this should be added to the docs either way?) This PR changes publicly observable behavior, so I think requires at least a T-libs-api FCP? r? libs-api cc rust-lang#140961 `impl<I: Default> Default for Fuse<I>` was added in 1.70.0 (rust-lang#99929), and it's docs and behavior do not appear to have changed since (`Fuse`'s `iter` field has been an `Option` since before the impl was added). [^1]: IIUC it is a "de facto" guideline for the stdlib that an iterator type's `default()` should be empty (and for iterators where that would not make sense, they should not implement `Default`): cc rust-lang/libs-team#77 (comment) , so for stdlib iterators, I don't think this would change anything. However, if a user has a custom `Iterator` type `I`, *and* they are using `Fuse<I>`, *and* they call `Fuse::<I>::default()`, this may change the behavior of their code.
So it can be constructed in the following code:
The text was updated successfully, but these errors were encountered: