Explain why the two alloc_from_iter functions are so different.

Author: nnethercote

compiler/rustc_arena/src/lib.rs

@@ -155,10 +155,10 @@ where
     // This default collects into a `SmallVec` and then allocates by copying
     // from it. The specializations below for types like `Vec` are more
     // efficient, copying directly without the intermediate collecting step.
-    // This default could be made more efficient, like
-    // `DroplessArena::alloc_from_iter`, but it's not hot enough to bother.
     #[inline]
     default fn alloc_from_iter(self, arena: &TypedArena<T>) -> &mut [T] {
+        // Safety: if iteration panics, `self` is untouched, and thus left in a
+        // good state.
         let vec: SmallVec<[_; 8]> = self.into_iter().collect();
         vec.alloc_from_iter(arena)
     }
@@ -270,6 +270,20 @@ impl<T> TypedArena<T> {
 
     #[inline]
     pub fn alloc_from_iter<I: IntoIterator<Item = T>>(&self, iter: I) -> &mut [T] {
+        // This implementation is entirely separate to
+        // `DroplessIterator::alloc_from_iter`, even though conceptually they
+        // are the same.
+        //
+        // `DroplessIterator` (in the fast case) writes elements from the
+        // iterator one at a time into the allocated memory. That's easy
+        // because the elements don't implement `Drop`. But for `TypedArena`
+        // they do implement `Drop`, which means that if the iterator panics we
+        // could end up with some allocated-but-uninitialized elements, which
+        // will then cause UB in `TypedArena::drop`.
+        //
+        // Instead we use an approach where any iterator panic will occur
+        // before the memory is allocated, with some specialization for common
+        // cases like `Vec` and `SmallVec`.
         assert!(mem::size_of::<T>() != 0);
         iter.alloc_from_iter(self)
     }