Skip to content

Commit

Permalink
Hide allocator details from TryReserveError
Browse files Browse the repository at this point in the history
  • Loading branch information
kornelski committed Jul 24, 2021
1 parent bddb59c commit a294aa8
Show file tree
Hide file tree
Showing 13 changed files with 269 additions and 109 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
#![feature(iter_zip)]
#![feature(thread_local_const_init)]
#![feature(try_reserve)]
#![feature(try_reserve_kind)]
#![feature(nonzero_ops)]
#![recursion_limit = "512"]

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Rust MIR: a lowered representation of Rust.
#![feature(once_cell)]
#![feature(control_flow_enum)]
#![feature(try_reserve)]
#![feature(try_reserve_kind)]
#![recursion_limit = "256"]

#[macro_use]
Expand Down
49 changes: 43 additions & 6 deletions library/alloc/src/collections/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,31 @@ use core::fmt::Display;
/// The error type for `try_reserve` methods.
#[derive(Clone, PartialEq, Eq, Debug)]
#[unstable(feature = "try_reserve", reason = "new API", issue = "48043")]
pub enum TryReserveError {
pub struct TryReserveError {
kind: TryReserveErrorKind,
}

impl TryReserveError {
/// Details about the allocation that caused the error
#[inline]
#[unstable(
feature = "try_reserve_kind",
reason = "Uncertain how much info should be exposed",
issue = "48043"
)]
pub fn kind(&self) -> TryReserveErrorKind {
self.kind.clone()
}
}

/// Details of the allocation that caused a `TryReserveError`
#[derive(Clone, PartialEq, Eq, Debug)]
#[unstable(
feature = "try_reserve_kind",
reason = "Uncertain how much info should be exposed",
issue = "48043"
)]
pub enum TryReserveErrorKind {
/// Error due to the computed capacity exceeding the collection's maximum
/// (usually `isize::MAX` bytes).
CapacityOverflow,
Expand All @@ -81,12 +105,23 @@ pub enum TryReserveError {
},
}

#[unstable(
feature = "try_reserve_kind",
reason = "Uncertain how much info should be exposed",
issue = "48043"
)]
impl From<TryReserveErrorKind> for TryReserveError {
fn from(kind: TryReserveErrorKind) -> Self {
Self { kind }
}
}

#[unstable(feature = "try_reserve", reason = "new API", issue = "48043")]
impl From<LayoutError> for TryReserveError {
/// Always evaluates to [`TryReserveError::CapacityOverflow`].
/// Always evaluates to [`TryReserveErrorKind::CapacityOverflow`].
#[inline]
fn from(_: LayoutError) -> Self {
TryReserveError::CapacityOverflow
TryReserveErrorKind::CapacityOverflow.into()
}
}

Expand All @@ -97,11 +132,13 @@ impl Display for TryReserveError {
fmt: &mut core::fmt::Formatter<'_>,
) -> core::result::Result<(), core::fmt::Error> {
fmt.write_str("memory allocation failed")?;
let reason = match &self {
TryReserveError::CapacityOverflow => {
let reason = match self.kind {
TryReserveErrorKind::CapacityOverflow => {
" because the computed capacity exceeded the collection's maximum"
}
TryReserveError::AllocError { .. } => " because the memory allocator returned a error",
TryReserveErrorKind::AllocError { .. } => {
" because the memory allocator returned a error"
}
};
fmt.write_str(reason)
}
Expand Down
3 changes: 2 additions & 1 deletion library/alloc/src/collections/vec_deque/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use core::ptr::{self, NonNull};
use core::slice;

use crate::collections::TryReserveError;
use crate::collections::TryReserveErrorKind;
use crate::raw_vec::RawVec;
use crate::vec::Vec;

Expand Down Expand Up @@ -724,7 +725,7 @@ impl<T> VecDeque<T> {
let new_cap = used_cap
.checked_add(additional)
.and_then(|needed_cap| needed_cap.checked_next_power_of_two())
.ok_or(TryReserveError::CapacityOverflow)?;
.ok_or(TryReserveErrorKind::CapacityOverflow)?;

if new_cap > old_cap {
self.buf.try_reserve_exact(used_cap, new_cap - used_cap)?;
Expand Down
20 changes: 10 additions & 10 deletions library/alloc/src/raw_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use core::slice;
use crate::alloc::handle_alloc_error;
use crate::alloc::{Allocator, Global, Layout};
use crate::boxed::Box;
use crate::collections::TryReserveError::{self, *};
use crate::collections::TryReserveError;
use crate::collections::TryReserveErrorKind::*;

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -425,7 +426,7 @@ impl<T, A: Allocator> RawVec<T, A> {
if mem::size_of::<T>() == 0 {
// Since we return a capacity of `usize::MAX` when `elem_size` is
// 0, getting to here necessarily means the `RawVec` is overfull.
return Err(CapacityOverflow);
return Err(CapacityOverflow.into());
}

// Nothing we can really do about these checks, sadly.
Expand All @@ -451,7 +452,7 @@ impl<T, A: Allocator> RawVec<T, A> {
if mem::size_of::<T>() == 0 {
// Since we return a capacity of `usize::MAX` when the type size is
// 0, getting to here necessarily means the `RawVec` is overfull.
return Err(CapacityOverflow);
return Err(CapacityOverflow.into());
}

let cap = len.checked_add(additional).ok_or(CapacityOverflow)?;
Expand All @@ -471,10 +472,9 @@ impl<T, A: Allocator> RawVec<T, A> {

let ptr = unsafe {
let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
self.alloc.shrink(ptr, layout, new_layout).map_err(|_| TryReserveError::AllocError {
layout: new_layout,
non_exhaustive: (),
})?
self.alloc
.shrink(ptr, layout, new_layout)
.map_err(|_| AllocError { layout: new_layout, non_exhaustive: () })?
};
self.set_ptr(ptr);
Ok(())
Expand Down Expand Up @@ -510,7 +510,7 @@ where
alloc.allocate(new_layout)
};

memory.map_err(|_| AllocError { layout: new_layout, non_exhaustive: () })
memory.map_err(|_| AllocError { layout: new_layout, non_exhaustive: () }.into())
}

unsafe impl<#[may_dangle] T, A: Allocator> Drop for RawVec<T, A> {
Expand All @@ -526,7 +526,7 @@ unsafe impl<#[may_dangle] T, A: Allocator> Drop for RawVec<T, A> {
#[cfg(not(no_global_oom_handling))]
#[inline]
fn handle_reserve(result: Result<(), TryReserveError>) {
match result {
match result.map_err(|e| e.kind()) {
Err(CapacityOverflow) => capacity_overflow(),
Err(AllocError { layout, .. }) => handle_alloc_error(layout),
Ok(()) => { /* yay */ }
Expand All @@ -545,7 +545,7 @@ fn handle_reserve(result: Result<(), TryReserveError>) {
#[inline]
fn alloc_guard(alloc_size: usize) -> Result<(), TryReserveError> {
if usize::BITS < 64 && alloc_size > isize::MAX as usize {
Err(CapacityOverflow)
Err(CapacityOverflow.into())
} else {
Ok(())
}
Expand Down
1 change: 1 addition & 0 deletions library/alloc/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#![feature(pattern)]
#![feature(trusted_len)]
#![feature(try_reserve)]
#![feature(try_reserve_kind)]
#![feature(unboxed_closures)]
#![feature(associated_type_bounds)]
#![feature(binary_heap_into_iter_sorted)]
Expand Down
74 changes: 51 additions & 23 deletions library/alloc/tests/string.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::borrow::Cow;
use std::cell::Cell;
use std::collections::TryReserveError::*;
use std::collections::TryReserveErrorKind::*;
use std::ops::Bound;
use std::ops::Bound::*;
use std::ops::RangeBounds;
Expand Down Expand Up @@ -703,35 +703,42 @@ fn test_try_reserve() {
let mut empty_string: String = String::new();

// Check isize::MAX doesn't count as an overflow
if let Err(CapacityOverflow) = empty_string.try_reserve(MAX_CAP) {
if let Err(CapacityOverflow) = empty_string.try_reserve(MAX_CAP).map_err(|e| e.kind()) {
panic!("isize::MAX shouldn't trigger an overflow!");
}
// Play it again, frank! (just to be sure)
if let Err(CapacityOverflow) = empty_string.try_reserve(MAX_CAP) {
if let Err(CapacityOverflow) = empty_string.try_reserve(MAX_CAP).map_err(|e| e.kind()) {
panic!("isize::MAX shouldn't trigger an overflow!");
}

if guards_against_isize {
// Check isize::MAX + 1 does count as overflow
if let Err(CapacityOverflow) = empty_string.try_reserve(MAX_CAP + 1) {
if let Err(CapacityOverflow) =
empty_string.try_reserve(MAX_CAP + 1).map_err(|e| e.kind())
{
} else {
panic!("isize::MAX + 1 should trigger an overflow!")
}

// Check usize::MAX does count as overflow
if let Err(CapacityOverflow) = empty_string.try_reserve(MAX_USIZE) {
if let Err(CapacityOverflow) = empty_string.try_reserve(MAX_USIZE).map_err(|e| e.kind())
{
} else {
panic!("usize::MAX should trigger an overflow!")
}
} else {
// Check isize::MAX + 1 is an OOM
if let Err(AllocError { .. }) = empty_string.try_reserve(MAX_CAP + 1) {
if let Err(AllocError { .. }) =
empty_string.try_reserve(MAX_CAP + 1).map_err(|e| e.kind())
{
} else {
panic!("isize::MAX + 1 should trigger an OOM!")
}

// Check usize::MAX is an OOM
if let Err(AllocError { .. }) = empty_string.try_reserve(MAX_USIZE) {
if let Err(AllocError { .. }) =
empty_string.try_reserve(MAX_USIZE).map_err(|e| e.kind())
{
} else {
panic!("usize::MAX should trigger an OOM!")
}
Expand All @@ -742,25 +749,27 @@ fn test_try_reserve() {
// Same basic idea, but with non-zero len
let mut ten_bytes: String = String::from("0123456789");

if let Err(CapacityOverflow) = ten_bytes.try_reserve(MAX_CAP - 10) {
if let Err(CapacityOverflow) = ten_bytes.try_reserve(MAX_CAP - 10).map_err(|e| e.kind()) {
panic!("isize::MAX shouldn't trigger an overflow!");
}
if let Err(CapacityOverflow) = ten_bytes.try_reserve(MAX_CAP - 10) {
if let Err(CapacityOverflow) = ten_bytes.try_reserve(MAX_CAP - 10).map_err(|e| e.kind()) {
panic!("isize::MAX shouldn't trigger an overflow!");
}
if guards_against_isize {
if let Err(CapacityOverflow) = ten_bytes.try_reserve(MAX_CAP - 9) {
if let Err(CapacityOverflow) = ten_bytes.try_reserve(MAX_CAP - 9).map_err(|e| e.kind())
{
} else {
panic!("isize::MAX + 1 should trigger an overflow!");
}
} else {
if let Err(AllocError { .. }) = ten_bytes.try_reserve(MAX_CAP - 9) {
if let Err(AllocError { .. }) = ten_bytes.try_reserve(MAX_CAP - 9).map_err(|e| e.kind())
{
} else {
panic!("isize::MAX + 1 should trigger an OOM!")
}
}
// Should always overflow in the add-to-len
if let Err(CapacityOverflow) = ten_bytes.try_reserve(MAX_USIZE) {
if let Err(CapacityOverflow) = ten_bytes.try_reserve(MAX_USIZE).map_err(|e| e.kind()) {
} else {
panic!("usize::MAX should trigger an overflow!")
}
Expand All @@ -782,30 +791,40 @@ fn test_try_reserve_exact() {
{
let mut empty_string: String = String::new();

if let Err(CapacityOverflow) = empty_string.try_reserve_exact(MAX_CAP) {
if let Err(CapacityOverflow) = empty_string.try_reserve_exact(MAX_CAP).map_err(|e| e.kind())
{
panic!("isize::MAX shouldn't trigger an overflow!");
}
if let Err(CapacityOverflow) = empty_string.try_reserve_exact(MAX_CAP) {
if let Err(CapacityOverflow) = empty_string.try_reserve_exact(MAX_CAP).map_err(|e| e.kind())
{
panic!("isize::MAX shouldn't trigger an overflow!");
}

if guards_against_isize {
if let Err(CapacityOverflow) = empty_string.try_reserve_exact(MAX_CAP + 1) {
if let Err(CapacityOverflow) =
empty_string.try_reserve_exact(MAX_CAP + 1).map_err(|e| e.kind())
{
} else {
panic!("isize::MAX + 1 should trigger an overflow!")
}

if let Err(CapacityOverflow) = empty_string.try_reserve_exact(MAX_USIZE) {
if let Err(CapacityOverflow) =
empty_string.try_reserve_exact(MAX_USIZE).map_err(|e| e.kind())
{
} else {
panic!("usize::MAX should trigger an overflow!")
}
} else {
if let Err(AllocError { .. }) = empty_string.try_reserve_exact(MAX_CAP + 1) {
if let Err(AllocError { .. }) =
empty_string.try_reserve_exact(MAX_CAP + 1).map_err(|e| e.kind())
{
} else {
panic!("isize::MAX + 1 should trigger an OOM!")
}

if let Err(AllocError { .. }) = empty_string.try_reserve_exact(MAX_USIZE) {
if let Err(AllocError { .. }) =
empty_string.try_reserve_exact(MAX_USIZE).map_err(|e| e.kind())
{
} else {
panic!("usize::MAX should trigger an OOM!")
}
Expand All @@ -815,24 +834,33 @@ fn test_try_reserve_exact() {
{
let mut ten_bytes: String = String::from("0123456789");

if let Err(CapacityOverflow) = ten_bytes.try_reserve_exact(MAX_CAP - 10) {
if let Err(CapacityOverflow) =
ten_bytes.try_reserve_exact(MAX_CAP - 10).map_err(|e| e.kind())
{
panic!("isize::MAX shouldn't trigger an overflow!");
}
if let Err(CapacityOverflow) = ten_bytes.try_reserve_exact(MAX_CAP - 10) {
if let Err(CapacityOverflow) =
ten_bytes.try_reserve_exact(MAX_CAP - 10).map_err(|e| e.kind())
{
panic!("isize::MAX shouldn't trigger an overflow!");
}
if guards_against_isize {
if let Err(CapacityOverflow) = ten_bytes.try_reserve_exact(MAX_CAP - 9) {
if let Err(CapacityOverflow) =
ten_bytes.try_reserve_exact(MAX_CAP - 9).map_err(|e| e.kind())
{
} else {
panic!("isize::MAX + 1 should trigger an overflow!");
}
} else {
if let Err(AllocError { .. }) = ten_bytes.try_reserve_exact(MAX_CAP - 9) {
if let Err(AllocError { .. }) =
ten_bytes.try_reserve_exact(MAX_CAP - 9).map_err(|e| e.kind())
{
} else {
panic!("isize::MAX + 1 should trigger an OOM!")
}
}
if let Err(CapacityOverflow) = ten_bytes.try_reserve_exact(MAX_USIZE) {
if let Err(CapacityOverflow) = ten_bytes.try_reserve_exact(MAX_USIZE).map_err(|e| e.kind())
{
} else {
panic!("usize::MAX should trigger an overflow!")
}
Expand Down
Loading

0 comments on commit a294aa8

Please sign in to comment.