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

[zachs18]

The docs on impl<I: Default> Default for core::iter::Fuse<I> 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¹, 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 (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 #140961

impl<I: Default> Default for Fuse<I> was added in 1.70.0 (#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).

Footnotes

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 https://github.com/rust-lang/libs-team/issues/77#issuecomment-1194681709 , 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. ↩

[hanna-kruppe]

Either way (fix the impl or fix the docs) this should probably gain a test since it’s extremely non-obvious from the code alone.

[scottmcm]

And maybe change the code to either Option::default() just None or to I::default() to help emphasize, too.

[tgross35]

I doubt this will have much of a noticeable impact but we may as well try to measure it.

@bors try

[bors]

⌛ Trying commit fe06fc3 with merge 17092ad...

[bors]

☀️ Try build successful - checks-actions
Build commit: 17092ad (17092ad00ed8fe1e7f81a7e38238ff70779034b1)

[tgross35]

@craterbot run mode=build-and-test

[craterbot]

👌 Experiment pr-140985 created and queued.
🤖 Automatically detected try build 17092ad
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-140985 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-140985 is completed!
📊 208 regressed and 149 fixed (631768 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[zachs18]

I've looked at the crates.io regressions:

Definitely spurious

Some form of "no space left on device":

• aeiou-macros
• april
• argsyn
• art_dice
• async-peek
• badmf
• mu_pi
• puzz-http
• rstats
• rusty_audio
• safe-regex
• simple-cache-rs
• standard_card
• timely-master
• wallpaper_rs

Some form of "failed to create temporary directory":

• ambiq-apollo3p-pac
• async-pidfd-next
• banka
• simpl_cache
• storage-path-generator

"Error building SSL dependencies": artcode

Other OS error ("Resource temporarily unavailable" when opening a file?): cannyls

I think are spurious:

• due to measuring runtimes or similar: async-retry, dust_dds, ratelimit, shared-resource-pool-builder, simple-clock, slow_function_warning, stringsort, tokio-context
• due to using env vars from multiple tests (and thus multiple threads): fuel-streams-nats, should-color
• due to relying on hashmap iteration order: axsys-noun, serde-command-opts
• due to relying on non-hashmap randomness: rs-tool

Otherwise unreliable/incorrect tests: rayoff, samus, sigalign-core (see linked PRs/issue)

I haven't figured out yet: gouth, rs-store, rust-netrc, sigio, sitrep, there. None seem to use Fuse directly at least (rg -iw fuse (case-insensitive, word-based search) on their sources yields no results).

[tgross35]

Thanks for the analysis. I think we can hand this off to libs-api.

@rustbot label +I-libs-api-nominated +T-libs-api -T-libs

[joshtriplett]

This makes sense, and it sounds like it doesn't have any issues in crater, so:

@rfcbot merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[tgross35]

@zachs18 could you apply the suggestions from @scottmcm and @hanna-kruppe above? We can then merge this after the FCP completes.