Skip to content

Commit

Permalink
Rollup merge of rust-lang#61780 - SimonSapin:container-error, r=Amanieu
Browse files Browse the repository at this point in the history
Finalize the error type for `try_reserve`

See tracking issue comments from rust-lang#48043 (comment).

It is now:

```rust
/// The error type for `try_reserve` methods.
#[derive(Clone, PartialEq, Eq, Debug)]
#[unstable(feature = "try_reserve", reason = "new API", issue="48043")]
pub enum TryReserveError {
    /// Error due to the computed capacity exceeding the collection's maximum
    /// (usually `isize::MAX` bytes).
    CapacityOverflow,

    /// The memory allocator returned an error
    AllocError {
        /// The layout of allocation request that failed
        layout: Layout,

        #[doc(hidden)]
        #[unstable(feature = "container_error_extra", issue = "0", reason = "\
            Enable exposing the allocator’s custom error value \
            if an associated type is added in the future: \
            rust-lang/wg-allocators#23")]
        non_exhaustive: (),
    },
}

#[unstable(feature = "try_reserve", reason = "new API", issue="48043")]
impl From<LayoutErr> for TryReserveError {
    #[inline]
    fn from(_: LayoutErr) -> Self {
        TryReserveError::CapacityOverflow
    }
}
```

Changes:

* A `Layout` is included. Firefox wants to log the size of failed allocations. If this were not part of the return value of e.g. `HashMap::try_reserve`, users would only be able to estimate based on `HashMap::capacity` and assumptions about the allocation strategy of `HashMap`.

* There’s a dummy field that can stay unstable when `try_reserve` and the rest of this enum are stabilized. This forces non-exhaustive matching ~(rust-lang#44109 is not implemented yet for variants)~ and allows adding another field in the future if we want to expose custom error values from the allocator. See rust-lang/wg-allocators#23.

  - If the `Alloc` trait is stabilized without an associated error type and with a zero-size `AllocErr` type, we can simply remove this dummy field.
  - If an associated type is added, we can add a default type parameter to `ContainerError` and a generic field to the `AllocError` variant.

* ~Moved from the `collections` module to the `alloc` module, and replaced `Collection` in the enum name with `Container`. The wold collection implies a multiplicity of items which is not relevant to this type. For example we may want to use this error type in a future `Box::try_new` method.~

  - Renamed to `TryReserveError`, after the methods that involve this type: rust-lang#61780 (comment)

* Replaced `Err` with `Error` in the enum and variant names. There is more precedent for this in https://doc.rust-lang.org/std/error/trait.Error.html#implementors, `AllocErr` and `LayoutErr` are the odd ones.

* ~Dropped `Alloc` in the enum name. `ContainerAllocError` with a mouthful, and being in the `alloc` module already provides the same indication.~
  • Loading branch information
Centril committed Aug 16, 2019
2 parents e632daf + 59a3409 commit aec047e
Show file tree
Hide file tree
Showing 15 changed files with 97 additions and 95 deletions.
12 changes: 2 additions & 10 deletions Cargo.lock
Expand Up @@ -1138,19 +1138,12 @@ dependencies = [

[[package]]
name = "hashbrown"
version = "0.4.0"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"compiler_builtins 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-std-workspace-alloc 1.0.0",
"rustc-std-workspace-core 1.0.0",
]

[[package]]
name = "hashbrown"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"serde 1.0.92 (registry+https://github.com/rust-lang/crates.io-index)",
]

Expand Down Expand Up @@ -3534,7 +3527,7 @@ dependencies = [
"core 0.0.0",
"dlmalloc 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
"fortanix-sgx-abi 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
"hashbrown 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"hashbrown 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.60 (registry+https://github.com/rust-lang/crates.io-index)",
"panic_abort 0.0.0",
"panic_unwind 0.0.0",
Expand Down Expand Up @@ -4450,7 +4443,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum globset 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "ef4feaabe24a0a658fd9cf4a9acf6ed284f045c77df0f49020ba3245cfb7b454"
"checksum h2 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)" = "a539b63339fbbb00e081e84b6e11bd1d9634a82d91da2984a18ac74a8823f392"
"checksum handlebars 2.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "df044dd42cdb7e32f28557b661406fc0f2494be75199779998810dbc35030e0d"
"checksum hashbrown 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9529213c67695ca2d146e6f263b7b72df8fa973368beadf767e8ed80c03f2f36"
"checksum hashbrown 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e1de41fb8dba9714efd92241565cdff73f78508c95697dd56787d3cba27e2353"
"checksum heck 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ea04fa3ead4e05e51a7c806fc07271fdbde4e246a6c6d1efd52e72230b771b82"
"checksum hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "805026a5d0141ffc30abb3be3173848ad46a1b1664fe632428479619a3644d77"
Expand Down
31 changes: 17 additions & 14 deletions src/liballoc/collections/mod.rs
Expand Up @@ -41,32 +41,35 @@ pub use linked_list::LinkedList;
#[doc(no_inline)]
pub use vec_deque::VecDeque;

use crate::alloc::{AllocErr, LayoutErr};
use crate::alloc::{Layout, LayoutErr};

/// Augments `AllocErr` with a CapacityOverflow variant.
/// The error type for `try_reserve` methods.
#[derive(Clone, PartialEq, Eq, Debug)]
#[unstable(feature = "try_reserve", reason = "new API", issue="48043")]
pub enum CollectionAllocErr {
pub enum TryReserveError {
/// Error due to the computed capacity exceeding the collection's maximum
/// (usually `isize::MAX` bytes).
CapacityOverflow,
/// Error due to the allocator (see the `AllocErr` type's docs).
AllocErr,
}

#[unstable(feature = "try_reserve", reason = "new API", issue="48043")]
impl From<AllocErr> for CollectionAllocErr {
#[inline]
fn from(AllocErr: AllocErr) -> Self {
CollectionAllocErr::AllocErr
}
/// The memory allocator returned an error
AllocError {
/// The layout of allocation request that failed
layout: Layout,

#[doc(hidden)]
#[unstable(feature = "container_error_extra", issue = "0", reason = "\
Enable exposing the allocator’s custom error value \
if an associated type is added in the future: \
https://github.com/rust-lang/wg-allocators/issues/23")]
non_exhaustive: (),
},
}

#[unstable(feature = "try_reserve", reason = "new API", issue="48043")]
impl From<LayoutErr> for CollectionAllocErr {
impl From<LayoutErr> for TryReserveError {
#[inline]
fn from(_: LayoutErr) -> Self {
CollectionAllocErr::CapacityOverflow
TryReserveError::CapacityOverflow
}
}

Expand Down
16 changes: 8 additions & 8 deletions src/liballoc/collections/vec_deque.rs
Expand Up @@ -18,7 +18,7 @@ use core::ptr::{self, NonNull};
use core::slice;
use core::hash::{Hash, Hasher};

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

Expand Down Expand Up @@ -576,10 +576,10 @@ impl<T> VecDeque<T> {
///
/// ```
/// #![feature(try_reserve)]
/// use std::collections::CollectionAllocErr;
/// use std::collections::TryReserveError;
/// use std::collections::VecDeque;
///
/// fn process_data(data: &[u32]) -> Result<VecDeque<u32>, CollectionAllocErr> {
/// fn process_data(data: &[u32]) -> Result<VecDeque<u32>, TryReserveError> {
/// let mut output = VecDeque::new();
///
/// // Pre-reserve the memory, exiting if we can't
Expand All @@ -595,7 +595,7 @@ impl<T> VecDeque<T> {
/// # process_data(&[1, 2, 3]).expect("why is the test harness OOMing on 12 bytes?");
/// ```
#[unstable(feature = "try_reserve", reason = "new API", issue="48043")]
pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), CollectionAllocErr> {
pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), TryReserveError> {
self.try_reserve(additional)
}

Expand All @@ -614,10 +614,10 @@ impl<T> VecDeque<T> {
///
/// ```
/// #![feature(try_reserve)]
/// use std::collections::CollectionAllocErr;
/// use std::collections::TryReserveError;
/// use std::collections::VecDeque;
///
/// fn process_data(data: &[u32]) -> Result<VecDeque<u32>, CollectionAllocErr> {
/// fn process_data(data: &[u32]) -> Result<VecDeque<u32>, TryReserveError> {
/// let mut output = VecDeque::new();
///
/// // Pre-reserve the memory, exiting if we can't
Expand All @@ -633,12 +633,12 @@ impl<T> VecDeque<T> {
/// # process_data(&[1, 2, 3]).expect("why is the test harness OOMing on 12 bytes?");
/// ```
#[unstable(feature = "try_reserve", reason = "new API", issue="48043")]
pub fn try_reserve(&mut self, additional: usize) -> Result<(), CollectionAllocErr> {
pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> {
let old_cap = self.cap();
let used_cap = self.len() + 1;
let new_cap = used_cap.checked_add(additional)
.and_then(|needed_cap| needed_cap.checked_next_power_of_two())
.ok_or(CollectionAllocErr::CapacityOverflow)?;
.ok_or(TryReserveError::CapacityOverflow)?;

if new_cap > old_cap {
self.buf.try_reserve_exact(used_cap, new_cap - used_cap)?;
Expand Down
1 change: 1 addition & 0 deletions src/liballoc/lib.rs
Expand Up @@ -87,6 +87,7 @@
#![feature(const_in_array_repeat_expressions)]
#![feature(dispatch_from_dyn)]
#![feature(core_intrinsics)]
#![feature(container_error_extra)]
#![feature(dropck_eyepatch)]
#![feature(exact_size_is_empty)]
#![feature(fmt_internals)]
Expand Down
32 changes: 17 additions & 15 deletions src/liballoc/raw_vec.rs
Expand Up @@ -7,8 +7,8 @@ use core::ops::Drop;
use core::ptr::{self, NonNull, Unique};
use core::slice;

use crate::alloc::{Alloc, Layout, Global, handle_alloc_error};
use crate::collections::CollectionAllocErr::{self, *};
use crate::alloc::{Alloc, Layout, Global, AllocErr, handle_alloc_error};
use crate::collections::TryReserveError::{self, *};
use crate::boxed::Box;

#[cfg(test)]
Expand Down Expand Up @@ -385,7 +385,7 @@ impl<T, A: Alloc> RawVec<T, A> {

/// The same as `reserve_exact`, but returns on errors instead of panicking or aborting.
pub fn try_reserve_exact(&mut self, used_capacity: usize, needed_extra_capacity: usize)
-> Result<(), CollectionAllocErr> {
-> Result<(), TryReserveError> {

self.reserve_internal(used_capacity, needed_extra_capacity, Fallible, Exact)
}
Expand Down Expand Up @@ -413,7 +413,7 @@ impl<T, A: Alloc> RawVec<T, A> {
pub fn reserve_exact(&mut self, used_capacity: usize, needed_extra_capacity: usize) {
match self.reserve_internal(used_capacity, needed_extra_capacity, Infallible, Exact) {
Err(CapacityOverflow) => capacity_overflow(),
Err(AllocErr) => unreachable!(),
Err(AllocError { .. }) => unreachable!(),
Ok(()) => { /* yay */ }
}
}
Expand All @@ -422,7 +422,7 @@ impl<T, A: Alloc> RawVec<T, A> {
/// needed_extra_capacity` elements. This logic is used in amortized reserve methods.
/// Returns `(new_capacity, new_alloc_size)`.
fn amortized_new_size(&self, used_capacity: usize, needed_extra_capacity: usize)
-> Result<usize, CollectionAllocErr> {
-> Result<usize, TryReserveError> {

// Nothing we can really do about these checks :(
let required_cap = used_capacity.checked_add(needed_extra_capacity)
Expand All @@ -435,7 +435,7 @@ impl<T, A: Alloc> RawVec<T, A> {

/// The same as `reserve`, but returns on errors instead of panicking or aborting.
pub fn try_reserve(&mut self, used_capacity: usize, needed_extra_capacity: usize)
-> Result<(), CollectionAllocErr> {
-> Result<(), TryReserveError> {
self.reserve_internal(used_capacity, needed_extra_capacity, Fallible, Amortized)
}

Expand Down Expand Up @@ -494,7 +494,7 @@ impl<T, A: Alloc> RawVec<T, A> {
pub fn reserve(&mut self, used_capacity: usize, needed_extra_capacity: usize) {
match self.reserve_internal(used_capacity, needed_extra_capacity, Infallible, Amortized) {
Err(CapacityOverflow) => capacity_overflow(),
Err(AllocErr) => unreachable!(),
Err(AllocError { .. }) => unreachable!(),
Ok(()) => { /* yay */ }
}
}
Expand Down Expand Up @@ -640,10 +640,8 @@ impl<T, A: Alloc> RawVec<T, A> {
needed_extra_capacity: usize,
fallibility: Fallibility,
strategy: ReserveStrategy,
) -> Result<(), CollectionAllocErr> {
) -> Result<(), TryReserveError> {
unsafe {
use crate::alloc::AllocErr;

// NOTE: we don't early branch on ZSTs here because we want this
// to actually catch "asking for more than usize::MAX" in that case.
// If we make it past the first branch then we are guaranteed to
Expand Down Expand Up @@ -672,12 +670,16 @@ impl<T, A: Alloc> RawVec<T, A> {
None => self.a.alloc(new_layout),
};

match (&res, fallibility) {
let ptr = match (res, fallibility) {
(Err(AllocErr), Infallible) => handle_alloc_error(new_layout),
_ => {}
}
(Err(AllocErr), Fallible) => return Err(TryReserveError::AllocError {
layout: new_layout,
non_exhaustive: (),
}),
(Ok(ptr), _) => ptr,
};

self.ptr = res?.cast().into();
self.ptr = ptr.cast().into();
self.cap = new_cap;

Ok(())
Expand Down Expand Up @@ -737,7 +739,7 @@ unsafe impl<#[may_dangle] T, A: Alloc> Drop for RawVec<T, A> {
// all 4GB in user-space. e.g., PAE or x32

#[inline]
fn alloc_guard(alloc_size: usize) -> Result<(), CollectionAllocErr> {
fn alloc_guard(alloc_size: usize) -> Result<(), TryReserveError> {
if mem::size_of::<usize>() < 8 && alloc_size > core::isize::MAX as usize {
Err(CapacityOverflow)
} else {
Expand Down
14 changes: 7 additions & 7 deletions src/liballoc/string.rs
Expand Up @@ -56,7 +56,7 @@ use core::ptr;
use core::str::{pattern::Pattern, lossy};

use crate::borrow::{Cow, ToOwned};
use crate::collections::CollectionAllocErr;
use crate::collections::TryReserveError;
use crate::boxed::Box;
use crate::str::{self, from_boxed_utf8_unchecked, FromStr, Utf8Error, Chars};
use crate::vec::Vec;
Expand Down Expand Up @@ -937,9 +937,9 @@ impl String {
///
/// ```
/// #![feature(try_reserve)]
/// use std::collections::CollectionAllocErr;
/// use std::collections::TryReserveError;
///
/// fn process_data(data: &str) -> Result<String, CollectionAllocErr> {
/// fn process_data(data: &str) -> Result<String, TryReserveError> {
/// let mut output = String::new();
///
/// // Pre-reserve the memory, exiting if we can't
Expand All @@ -953,7 +953,7 @@ impl String {
/// # process_data("rust").expect("why is the test harness OOMing on 4 bytes?");
/// ```
#[unstable(feature = "try_reserve", reason = "new API", issue="48043")]
pub fn try_reserve(&mut self, additional: usize) -> Result<(), CollectionAllocErr> {
pub fn try_reserve(&mut self, additional: usize) -> Result<(), TryReserveError> {
self.vec.try_reserve(additional)
}

Expand All @@ -975,9 +975,9 @@ impl String {
///
/// ```
/// #![feature(try_reserve)]
/// use std::collections::CollectionAllocErr;
/// use std::collections::TryReserveError;
///
/// fn process_data(data: &str) -> Result<String, CollectionAllocErr> {
/// fn process_data(data: &str) -> Result<String, TryReserveError> {
/// let mut output = String::new();
///
/// // Pre-reserve the memory, exiting if we can't
Expand All @@ -991,7 +991,7 @@ impl String {
/// # process_data("rust").expect("why is the test harness OOMing on 4 bytes?");
/// ```
#[unstable(feature = "try_reserve", reason = "new API", issue="48043")]
pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), CollectionAllocErr> {
pub fn try_reserve_exact(&mut self, additional: usize) -> Result<(), TryReserveError> {
self.vec.try_reserve_exact(additional)
}

Expand Down
14 changes: 7 additions & 7 deletions src/liballoc/tests/string.rs
@@ -1,5 +1,5 @@
use std::borrow::Cow;
use std::collections::CollectionAllocErr::*;
use std::collections::TryReserveError::*;
use std::mem::size_of;
use std::{usize, isize};

Expand Down Expand Up @@ -566,11 +566,11 @@ fn test_try_reserve() {
} else { panic!("usize::MAX should trigger an overflow!") }
} else {
// Check isize::MAX + 1 is an OOM
if let Err(AllocErr) = empty_string.try_reserve(MAX_CAP + 1) {
if let Err(AllocError { .. }) = empty_string.try_reserve(MAX_CAP + 1) {
} else { panic!("isize::MAX + 1 should trigger an OOM!") }

// Check usize::MAX is an OOM
if let Err(AllocErr) = empty_string.try_reserve(MAX_USIZE) {
if let Err(AllocError { .. }) = empty_string.try_reserve(MAX_USIZE) {
} else { panic!("usize::MAX should trigger an OOM!") }
}
}
Expand All @@ -590,7 +590,7 @@ fn test_try_reserve() {
if let Err(CapacityOverflow) = ten_bytes.try_reserve(MAX_CAP - 9) {
} else { panic!("isize::MAX + 1 should trigger an overflow!"); }
} else {
if let Err(AllocErr) = ten_bytes.try_reserve(MAX_CAP - 9) {
if let Err(AllocError { .. }) = ten_bytes.try_reserve(MAX_CAP - 9) {
} else { panic!("isize::MAX + 1 should trigger an OOM!") }
}
// Should always overflow in the add-to-len
Expand Down Expand Up @@ -629,10 +629,10 @@ fn test_try_reserve_exact() {
if let Err(CapacityOverflow) = empty_string.try_reserve_exact(MAX_USIZE) {
} else { panic!("usize::MAX should trigger an overflow!") }
} else {
if let Err(AllocErr) = empty_string.try_reserve_exact(MAX_CAP + 1) {
if let Err(AllocError { .. }) = empty_string.try_reserve_exact(MAX_CAP + 1) {
} else { panic!("isize::MAX + 1 should trigger an OOM!") }

if let Err(AllocErr) = empty_string.try_reserve_exact(MAX_USIZE) {
if let Err(AllocError { .. }) = empty_string.try_reserve_exact(MAX_USIZE) {
} else { panic!("usize::MAX should trigger an OOM!") }
}
}
Expand All @@ -651,7 +651,7 @@ fn test_try_reserve_exact() {
if let Err(CapacityOverflow) = ten_bytes.try_reserve_exact(MAX_CAP - 9) {
} else { panic!("isize::MAX + 1 should trigger an overflow!"); }
} else {
if let Err(AllocErr) = ten_bytes.try_reserve_exact(MAX_CAP - 9) {
if let Err(AllocError { .. }) = ten_bytes.try_reserve_exact(MAX_CAP - 9) {
} else { panic!("isize::MAX + 1 should trigger an OOM!") }
}
if let Err(CapacityOverflow) = ten_bytes.try_reserve_exact(MAX_USIZE) {
Expand Down

0 comments on commit aec047e

Please sign in to comment.