Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide allocator details from TryReserveError #87408

Merged
merged 1 commit into from
Aug 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
kornelski marked this conversation as resolved.
Show resolved Hide resolved
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