diff options
| author | bors <bors@rust-lang.org> | 2021-04-01 07:55:00 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-04-01 07:55:00 +0000 |
| commit | 803ddb83598838fb9de308d283b759ba463e5e80 (patch) | |
| tree | ed7c809ab360f62fe6f9dcb5336813d50b2665ce | |
| parent | 49e1ec09952c5ab7798addd29532d44dc020283f (diff) | |
| parent | ad3a791e2a8c0300090fa73d4ce414a738346a41 (diff) | |
| download | rust-803ddb83598838fb9de308d283b759ba463e5e80.tar.gz rust-803ddb83598838fb9de308d283b759ba463e5e80.zip | |
Auto merge of #83726 - the8472:large-trustedlen-fail-fast, r=kennytm
panic early when `TrustedLen` indicates a `length > usize::MAX` Changes `TrustedLen` specializations to immediately panic when `size_hint().1 == None`. As far as I can tell this is ~not a change~ a minimal change in observable behavior for anything except ZSTs because the fallback path would go through `extend_desugared()` which tries to `reserve(lower_bound)` which already is `usize::MAX` and that would also lead to a panic. Before it might have popped somewhere between zero and a few elements from the iterator before panicking while it now panics immediately. Overall this should reduce codegen by eliminating the fallback paths. While looking into the `with_capacity()` behavior I also noticed that its documentation didn't have a *Panics* section, so I added that.
| -rw-r--r-- | library/alloc/src/rc.rs | 7 | ||||
| -rw-r--r-- | library/alloc/src/sync.rs | 7 | ||||
| -rw-r--r-- | library/alloc/src/vec/mod.rs | 8 | ||||
| -rw-r--r-- | library/alloc/src/vec/spec_extend.rs | 7 | ||||
| -rw-r--r-- | library/alloc/src/vec/spec_from_iter_nested.rs | 9 |
5 files changed, 30 insertions, 8 deletions
diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index dac4acc4692..2ee5e6c791c 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -1848,8 +1848,11 @@ impl<T, I: iter::TrustedLen<Item = T>> ToRcSlice<T> for I { Rc::from_iter_exact(self, low) } } else { - // Fall back to normal implementation. - self.collect::<Vec<T>>().into() + // TrustedLen contract guarantees that `upper_bound == `None` implies an iterator + // length exceeding `usize::MAX`. + // The default implementation would collect into a vec which would panic. + // Thus we panic here immediately without invoking `Vec` code. + panic!("capacity overflow"); } } } diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index aeae888dddc..1b7e656cefd 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -2481,8 +2481,11 @@ impl<T, I: iter::TrustedLen<Item = T>> ToArcSlice<T> for I { Arc::from_iter_exact(self, low) } } else { - // Fall back to normal implementation. - self.collect::<Vec<T>>().into() + // TrustedLen contract guarantees that `upper_bound == `None` implies an iterator + // length exceeding `usize::MAX`. + // The default implementation would collect into a vec which would panic. + // Thus we panic here immediately without invoking `Vec` code. + panic!("capacity overflow"); } } } diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 20c2aae1789..91c3b16deee 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -410,6 +410,10 @@ impl<T> Vec<T> { /// /// [Capacity and reallocation]: #capacity-and-reallocation /// + /// # Panics + /// + /// Panics if the new capacity exceeds `isize::MAX` bytes. + /// /// # Examples /// /// ``` @@ -541,6 +545,10 @@ impl<T, A: Allocator> Vec<T, A> { /// /// [Capacity and reallocation]: #capacity-and-reallocation /// + /// # Panics + /// + /// Panics if the new capacity exceeds `isize::MAX` bytes. + /// /// # Examples /// /// ``` diff --git a/library/alloc/src/vec/spec_extend.rs b/library/alloc/src/vec/spec_extend.rs index b6186a7ebaf..e132befcfa5 100644 --- a/library/alloc/src/vec/spec_extend.rs +++ b/library/alloc/src/vec/spec_extend.rs @@ -47,7 +47,12 @@ where }); } } else { - self.extend_desugared(iterator) + // Per TrustedLen contract a `None` upper bound means that the iterator length + // truly exceeds usize::MAX, which would eventually lead to a capacity overflow anyway. + // Since the other branch already panics eagerly (via `reserve()`) we do the same here. + // This avoids additional codegen for a fallback code path which would eventually + // panic anyway. + panic!("capacity overflow"); } } } diff --git a/library/alloc/src/vec/spec_from_iter_nested.rs b/library/alloc/src/vec/spec_from_iter_nested.rs index ec390c62165..948cf044197 100644 --- a/library/alloc/src/vec/spec_from_iter_nested.rs +++ b/library/alloc/src/vec/spec_from_iter_nested.rs @@ -46,10 +46,13 @@ where fn from_iter(iterator: I) -> Self { let mut vector = match iterator.size_hint() { (_, Some(upper)) => Vec::with_capacity(upper), - _ => Vec::new(), + // TrustedLen contract guarantees that `size_hint() == (_, None)` means that there + // are more than `usize::MAX` elements. + // Since the previous branch would eagerly panic if the capacity is too large + // (via `with_capacity`) we do the same here. + _ => panic!("capacity overflow"), }; - // must delegate to spec_extend() since extend() itself delegates - // to spec_from for empty Vecs + // reuse extend specialization for TrustedLen vector.spec_extend(iterator); vector } |
