Rollup merge of #151808 - Zalathar:alloc-raw-slice, r=Kivooeo

Document a safety condition for `TypedArena::alloc_raw_slice`

This method was marked safe in https://github.com/rust-lang/rust/pull/116224/commits/51edc219906f0973dd66b4b6ff5ff0ac857a4cc6, because there was no apparent reason for it to be unsafe.

However, I believe that `alloc_raw_slice` does actually impose a significant safety obligation on its caller, because the caller must ensure that each slot in the slice is properly initialized before the arena is dropped.

This is because the arena's Drop impl will unconditionally drop every storage slot that has been handed out, so it has no way to handle slots that were accidentally left uninitialized because a hypothetical caller of `alloc_raw_slice` panicked before initializing them.

compiler/rustc_arena/src/lib.rs

@@ -172,8 +172,22 @@ impl<T> TypedArena<T> {
         available_bytes >= additional_bytes
     }
 
+    /// Allocates storage for `len >= 1` values in this arena, and returns a
+    /// raw pointer to the first value's storage.
+    ///
+    /// # Safety
+    ///
+    /// Caller must initialize each of the `len` slots to a droppable value
+    /// before the arena is dropped.
+    ///
+    /// In practice, this typically means that the caller must be able to
+    /// raw-copy `len` already-initialized values into the slice without any
+    /// possibility of panicking.
+    ///
+    /// FIXME(Zalathar): This is *very* fragile; perhaps we need a different
+    /// approach to arena-allocating slices of droppable values.
     #[inline]
-    fn alloc_raw_slice(&self, len: usize) -> *mut T {
+    unsafe fn alloc_raw_slice(&self, len: usize) -> *mut T {
         assert!(size_of::<T>() != 0);
         assert!(len != 0);
 
@@ -208,7 +222,7 @@ impl<T> TypedArena<T> {
         &self,
         iter: impl IntoIterator<Item = Result<T, E>>,
     ) -> Result<&mut [T], E> {
-        // Despite the similarlty with `DroplessArena`, we cannot reuse their fast case. The reason
+        // Despite the similarity with `DroplessArena`, we cannot reuse their fast case. The reason
         // is subtle: these arenas are reentrant. In other words, `iter` may very well be holding a
         // reference to `self` and adding elements to the arena during iteration.
         //
@@ -229,9 +243,15 @@ impl<T> TypedArena<T> {
         }
         // Move the content to the arena by copying and then forgetting it.
         let len = vec.len();
-        let start_ptr = self.alloc_raw_slice(len);
+
+        // SAFETY: After allocating raw storage for exactly `len` values, we
+        // must fully initialize the storage without panicking, and we must
+        // also prevent the stale values in the vec from being dropped.
         Ok(unsafe {
+            let start_ptr = self.alloc_raw_slice(len);
+            // Initialize the newly-allocated storage without panicking.
             vec.as_ptr().copy_to_nonoverlapping(start_ptr, len);
+            // Prevent the stale values in the vec from being dropped.
             vec.set_len(0);
             slice::from_raw_parts_mut(start_ptr, len)
         })
@@ -584,7 +604,7 @@ impl DroplessArena {
         &self,
         iter: impl IntoIterator<Item = Result<T, E>>,
     ) -> Result<&mut [T], E> {
-        // Despite the similarlty with `alloc_from_iter`, we cannot reuse their fast case, as we
+        // Despite the similarity with `alloc_from_iter`, we cannot reuse their fast case, as we
         // cannot know the minimum length of the iterator in this case.
         assert!(size_of::<T>() != 0);
 

typos.toml

@@ -46,6 +46,7 @@ unstalled = "unstalled" # short for un-stalled
 #
 # tidy-alphabetical-start
 definitinon = "definition"
+similarlty = "similarity"
 # tidy-alphabetical-end
 
 [default.extend-identifiers]