Skip to content

Commit

Permalink
Auto merge of #150 - ehuss:extend-resume, r=emilio
Browse files Browse the repository at this point in the history
Fix `extend` from assuming a fused iterator.

Iterators may resume after returning None.  I think it is generally expected that `extend` should stop on the first None.

Fixes #147.  Specifically, in rustc, there are some situations where an iterator of Results are collected into a `Result<SmallVec, _>` (such as [here](https://github.com/rust-lang/rust/blob/c1c60d292e2dd2deff7084208274f9a02f750d43/src/librustc/ty/relate.rs#L139-L142) which ends up in [this call to `collect`](https://github.com/rust-lang/rust/blob/c1c60d292e2dd2deff7084208274f9a02f750d43/src/librustc/ty/context.rs#L3002)).  The [Result adapter](https://github.com/rust-lang/rust/blob/c1c60d292e2dd2deff7084208274f9a02f750d43/src/libcore/result.rs#L1245-L1258) returns None for the first `Err` in the sequence.  However, it is possible for an iterator to have additional elements after the first Err.  With this bug, SmallVec was falling through to the slow-path `for` loop, and resuming the iterator grabbing too many elements.  I believe this was having some bad interactions with the type interner where the additional elements after the `Err` were getting processed (via a `map()`) and interned when they shouldn't be (or otherwise having some side effects from the `map`).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/150)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Jun 8, 2019
2 parents f96322b + 1fbaf10 commit 88b62b6
Showing 1 changed file with 13 additions and 1 deletion.
14 changes: 13 additions & 1 deletion lib.rs
Expand Up @@ -1356,7 +1356,7 @@ impl<A: Array> Extend<A::Item> for SmallVec<A> {
ptr::write(ptr.offset(len.get() as isize), out);
len.increment_len(1);
} else {
break;
return;
}
}
}
Expand Down Expand Up @@ -2329,4 +2329,16 @@ mod tests {
v.push(4);
assert_eq!(v[..], [4]);
}

#[test]
fn resumable_extend() {
let s = "a b c";
// This iterator yields: (Some('a'), None, Some('b'), None, Some('c')), None
let it = s
.chars()
.scan(0, |_, ch| if ch.is_whitespace() { None } else { Some(ch) });
let mut v: SmallVec<[char; 4]> = SmallVec::new();
v.extend(it);
assert_eq!(v[..], ['a']);
}
}

0 comments on commit 88b62b6

Please sign in to comment.