Auto merge of #87408 - kornelski:try_reserve_error, r=yaahc

Hide allocator details from TryReserveError

I think there's [no need for TryReserveError to carry detailed information](#48043 (comment)), but I wouldn't want that issue to delay stabilization of the `try_reserve` feature.

So I'm proposing to stabilize `try_reserve` with a `TryReserveError` as an opaque structure, and if needed, expose error details later.

This PR moves the `enum` to an unstable inner `TryReserveErrorKind` that lives under a separate feature flag. `TryReserveErrorKind` could possibly be left as an implementation detail forever, and the `TryReserveError` get methods such as `allocation_size() -> Option<usize>` or `layout() -> Option<Layout>` instead, or the details could be dropped completely to make try-reserve errors just a unit struct, and thus smaller and cheaper.

Author: bors

compiler/rustc_middle/src/lib.rs

@@ -51,6 +51,7 @@
 #![feature(iter_zip)]
 #![feature(thread_local_const_init)]
 #![feature(try_reserve)]
+#![feature(try_reserve_kind)]
 #![feature(nonzero_ops)]
 #![recursion_limit = "512"]
 

compiler/rustc_mir/src/lib.rs

@@ -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]

library/alloc/src/collections/mod.rs

@@ -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,
@@ -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()
     }
 }
 
@@ -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)
     }

library/alloc/src/collections/vec_deque/mod.rs

@@ -19,6 +19,7 @@ use core::slice;
 
 use crate::alloc::{Allocator, Global};
 use crate::collections::TryReserveError;
+use crate::collections::TryReserveErrorKind;
 use crate::raw_vec::RawVec;
 use crate::vec::Vec;
 
@@ -773,7 +774,7 @@ impl<T, A: Allocator> VecDeque<T, A> {
         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)?;

library/alloc/src/raw_vec.rs

@@ -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;
@@ -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.
@@ -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)?;
@@ -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(())
@@ -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> {
@@ -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 */ }
@@ -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(())
     }

library/alloc/tests/lib.rs

@@ -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)]

library/alloc/tests/string.rs

@@ -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;
@@ -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!")
             }
@@ -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!")
         }
@@ -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!")
             }
@@ -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!")
         }