Skip to content

Only implement FusedIterator for TakeWhileInclusive when the inner iterator is fused#1110

Merged
phimuemue merged 1 commit into
rust-itertools:masterfrom
patrickwehbe:fix/take-while-inclusive-fused
Jun 25, 2026
Merged

Only implement FusedIterator for TakeWhileInclusive when the inner iterator is fused#1110
phimuemue merged 1 commit into
rust-itertools:masterfrom
patrickwehbe:fix/take-while-inclusive-fused

Conversation

@patrickwehbe

Copy link
Copy Markdown

Fixes #1088.

TakeWhileInclusive sets its done flag only when the predicate rejects an element. It does not set it when the inner iterator returns None, so with a non-fused inner iterator the adaptor can return Some again after a None. The FusedIterator impl is currently unconditional (where I: Iterator), so that contract is violated.

This bounds the impl on I: FusedIterator, which is the same approach std::iter::TakeWhile takes (option 3 in the issue) and matches how the other conditionally-fused adaptors here are written (for example InterleaveShortest).

Note this is technically breaking: TakeWhileInclusive no longer implements FusedIterator when the inner iterator isn't fused. The previous impl was unsound in that case, so I left the changelog placement for release prep.

Added a fused_take_while_inclusive quickcheck test mirroring fused_interleave_shortest: not fused over the deliberately non-fused Iter, fused once the inner iterator is .fuse()d. cargo test, cargo fmt --check, and cargo clippy --all-features --all-targets all pass.

…erator is fused

TakeWhileInclusive sets its `done` flag only when the predicate rejects an
element, not when the inner iterator returns None. With a non-fused inner
iterator it can therefore yield Some after returning None, which breaks the
FusedIterator contract it currently claims for every Iterator.

Bound the impl on `I: FusedIterator`, matching std::iter::TakeWhile.

Fixes rust-itertools#1088
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.93%. Comparing base (6814180) to head (9e1e14c).
⚠️ Report is 209 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1110      +/-   ##
==========================================
- Coverage   94.38%   93.93%   -0.46%     
==========================================
  Files          48       52       +4     
  Lines        6665     6679      +14     
==========================================
- Hits         6291     6274      -17     
- Misses        374      405      +31     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@phimuemue phimuemue left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you, @patrickwehbe

@phimuemue phimuemue added this pull request to the merge queue Jun 25, 2026
Merged via the queue into rust-itertools:master with commit 75ac95b Jun 25, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect FusedIterator implementation on TakeWhileInclusive

2 participants