Skip to content

Change core::iter::Fuse's Default impl to do what its docs say it does #140985

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+23 −1
Open

Change core::iter::Fuse's Default impl to do what its docs say it does #140985

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion library/core/src/iter/adapters/fuse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,30 @@ impl<I: Default> Default for Fuse<I> {
/// let iter: Fuse<slice::Iter<'_, u8>> = Default::default();
/// assert_eq!(iter.len(), 0);
/// ```
///
/// This is equivalent to `I::default().fuse()`[^fuse_note]; e.g. if
/// `I::default()` is not an empty iterator, then this will not be
/// an empty iterator.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this is pointing out an arbitrary case that is in no way surprising (at least not to someone unfamiliar with the implementation history). I think what would be more helpful is to say that this function is equivalent to IteratorType::default().fused().

Copy link
Contributor Author

@zachs18 zachs18 Jun 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not mentioning non-empty I::default() at all would IMO make the doctest seem like a non-sequitur (and the doctest should remain so this change doesn't get accidentally undone later), so I think it should still be mentioned as an aside.

I changed this to say

This is equivalent to I::default().fuse(); e.g. if I::default() is not an empty iterator, then this will not be an empty iterator.

with a footnote that the equivalence assumes I::fuse is not overridden. Does that sound good?

(Github is being wierd for me and not updating properly: the sentence I changed this from was "Note that if I's default value is not an empty iterator, then this will not be an empty iterator.")

///
/// ```
/// # use std::iter::Fuse;
/// #[derive(Default)]
/// struct Fourever;
///
/// impl Iterator for Fourever {
/// type Item = u32;
/// fn next(&mut self) -> Option<u32> {
/// Some(4)
/// }
/// }
///
/// let mut iter: Fuse<Fourever> = Default::default();
/// assert_eq!(iter.next(), Some(4));
/// ```
///
/// [^fuse_note]: if `I` does not override `Iterator::fuse`'s default implementation
fn default() -> Self {
Fuse { iter: Default::default() }
Fuse { iter: Some(I::default()) }
}
}

Expand Down
Loading