Doc trivial ecs unsafe (#22014)

# Objective

- Trying to doc most of the unsafe in the ecs crate so we can turn on
`unsafe_op_in_unsafe_fn`.

## Solution

- Unfortunately reviewing the unsafe docs will probably not be trivial
if we try to do it all in one pr. There are 400+ warnings when you turn
on the lint. So we need to break it up as much as possible as reviewing
the safety contracts in some cases isn't the easiest.
- This pr includes two types of unsafe blocks.
1. Blocks that already had safety comments, but were missing the `unsafe
{}` block.
2. Unsafe functions that have the same safety contract as their unsafe
parent function and are very short. (Usually just the call to the
function).

Author: hymm

crates/bevy_ecs/src/bundle/impls.rs

@@ -138,8 +138,14 @@ macro_rules! tuple_impl {
                 bevy_ptr::deconstruct_moving_ptr!({
                     let tuple { $($index: $alias,)* } = ptr;
                 });
+                #[allow(
+                    unused_unsafe,
+                    reason = "Zero-length tuples will generate a function body equivalatent to (); however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case."
+                )]
                 // SAFETY: Caller ensures requirements for calling `get_components` are met.
-                $( $name::get_components($alias, func); )*
+                unsafe {
+                    $( $name::get_components($alias, func); )*
+                }
             }
 
             #[allow(
@@ -151,8 +157,14 @@ macro_rules! tuple_impl {
                 bevy_ptr::deconstruct_moving_ptr!({
                     let MaybeUninit::<tuple> { $($index: $alias,)* } = ptr;
                 });
+                #[allow(
+                    unused_unsafe,
+                    reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case."
+                )]
                 // SAFETY: Caller ensures requirements for calling `apply_effect` are met.
-                $( $name::apply_effect($alias, entity); )*
+                unsafe {
+                    $( $name::apply_effect($alias, entity); )*
+                }
             }
         }
 

crates/bevy_ecs/src/bundle/insert.rs

@@ -535,13 +535,15 @@ impl BundleInfo {
                 };
             };
             // SAFETY: ids in self must be valid
-            let (new_archetype_id, is_new_created) = archetypes.get_id_or_insert(
-                components,
-                observers,
-                table_id,
-                table_components,
-                sparse_set_components,
-            );
+            let (new_archetype_id, is_new_created) = unsafe {
+                archetypes.get_id_or_insert(
+                    components,
+                    observers,
+                    table_id,
+                    table_components,
+                    sparse_set_components,
+                )
+            };
 
             // Add an edge from the old archetype to the new archetype.
             archetypes[archetype_id]

crates/bevy_ecs/src/component/required.rs

@@ -390,11 +390,12 @@ impl Components {
         let old_required_count = required_components.all.len();
 
         // SAFETY: the caller guarantees that `requiree` is valid in `self`.
-        self.required_components_scope(requiree, |this, required_components| {
-            // SAFETY: the caller guarantees that `required` is valid for type `R` in `self`
-            unsafe { required_components.register_by_id(required, this, constructor) };
-        });
-
+        unsafe {
+            self.required_components_scope(requiree, |this, required_components| {
+                // SAFETY: the caller guarantees that `required` is valid for type `R` in `self`
+                required_components.register_by_id(required, this, constructor);
+            });
+        }
         // Third step: update the required components and required_by of all the indirect requirements/requirees.
 
         // Borrow again otherwise it conflicts with the `self.required_components_scope` call.
@@ -435,11 +436,13 @@ impl Components {
         // Skip the first one (requiree) because we already updates it.
         for &indirect_requiree in &new_requiree_components[1..] {
             // SAFETY: `indirect_requiree` comes from `self` so it must be valid.
-            self.required_components_scope(indirect_requiree, |this, required_components| {
-                // Rebuild the inherited required components.
-                // SAFETY: `required_components` comes from `self`, so all its components must have be valid in `self`.
-                unsafe { required_components.rebuild_inherited_required_components(this) };
-            });
+            unsafe {
+                self.required_components_scope(indirect_requiree, |this, required_components| {
+                    // Rebuild the inherited required components.
+                    // SAFETY: `required_components` comes from `self`, so all its components must have be valid in `self`.
+                    required_components.rebuild_inherited_required_components(this);
+                });
+            }
         }
 
         // Update the `required_by` of all the components that were newly required (directly or indirectly).

crates/bevy_ecs/src/entity/mod.rs

@@ -993,7 +993,7 @@ impl Entities {
     ) -> Option<EntityLocation> {
         self.ensure_index_index_is_valid(index);
         // SAFETY: We just did `ensure_index`
-        self.update_existing_location(index, location)
+        unsafe { self.update_existing_location(index, location) }
     }
 
     /// Ensures the index is within the bounds of [`Self::meta`], expanding it if necessary.

crates/bevy_ecs/src/query/state.rs

@@ -1099,7 +1099,8 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
         world: UnsafeWorldCell<'w>,
         entity: Entity,
     ) -> Result<D::Item<'w, '_>, QueryEntityError> {
-        self.query_unchecked(world).get_inner(entity)
+        // SAFETY: Upheld by caller
+        unsafe { self.query_unchecked(world) }.get_inner(entity)
     }
 
     /// Returns an [`Iterator`] over the query results for the given [`World`].
@@ -1314,7 +1315,8 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
         &'s mut self,
         world: UnsafeWorldCell<'w>,
     ) -> QueryIter<'w, 's, D, F> {
-        self.query_unchecked(world).into_iter()
+        // SAFETY: Upheld by caller
+        unsafe { self.query_unchecked(world) }.into_iter()
     }
 
     /// Returns an [`Iterator`] over all possible combinations of `K` query results for the
@@ -1333,7 +1335,8 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
         &'s mut self,
         world: UnsafeWorldCell<'w>,
     ) -> QueryCombinationIter<'w, 's, D, F, K> {
-        self.query_unchecked(world).iter_combinations_inner()
+        // SAFETY: Upheld by caller
+        unsafe { self.query_unchecked(world) }.iter_combinations_inner()
     }
 
     /// Returns a parallel iterator over the query results for the given [`World`].
@@ -1755,7 +1758,8 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
         &mut self,
         world: UnsafeWorldCell<'w>,
     ) -> Result<D::Item<'w, '_>, QuerySingleError> {
-        self.query_unchecked(world).single_inner()
+        // SAFETY: Upheld by caller
+        unsafe { self.query_unchecked(world) }.single_inner()
     }
 
     /// Returns a query result when there is exactly one entity matching the query,
@@ -1780,8 +1784,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
         // SAFETY:
         // - The caller ensured we have the correct access to the world.
         // - The caller ensured that the world matches.
-        self.query_unchecked_manual_with_ticks(world, last_run, this_run)
-            .single_inner()
+        unsafe { self.query_unchecked_manual_with_ticks(world, last_run, this_run) }.single_inner()
     }
 }
 

crates/bevy_ecs/src/schedule/executor/mod.rs

@@ -260,7 +260,8 @@ mod __rust_begin_short_backtrace {
         system: &mut ScheduleSystem,
         world: UnsafeWorldCell,
     ) -> Result<(), RunSystemError> {
-        let result = system.run_unsafe((), world);
+        // SAFETY: Upheld by caller
+        let result = unsafe { system.run_unsafe((), world) };
         // Call `black_box` to prevent this frame from being tail-call optimized away
         black_box(());
         result
@@ -276,7 +277,8 @@ mod __rust_begin_short_backtrace {
         world: UnsafeWorldCell,
     ) -> Result<O, RunSystemError> {
         // Call `black_box` to prevent this frame from being tail-call optimized away
-        black_box(system.run_unsafe((), world))
+        // SAFETY: Upheld by caller
+        black_box(unsafe { system.run_unsafe((), world) })
     }
 
     #[cfg(feature = "std")]

crates/bevy_ecs/src/spawn.rs

@@ -327,7 +327,7 @@ impl<R: Relationship, L: SpawnableList<R>> DynamicBundle for SpawnRelatedBundle<
         //   called exactly once for each component being fetched with the correct `StorageType`
         // - `Effect: !NoBundleEffect`, which means the caller is responsible for calling this type's `apply_effect`
         //   at least once before returning to safe code.
-        <R::RelationshipTarget as DynamicBundle>::get_components(target, func);
+        unsafe { <R::RelationshipTarget as DynamicBundle>::get_components(target, func) };
         // Forget the pointer so that the value is available in `apply_effect`.
         mem::forget(ptr);
     }
@@ -372,7 +372,7 @@ impl<R: Relationship, B: Bundle> DynamicBundle for SpawnOneRelated<R, B> {
         //   called exactly once for each component being fetched with the correct `StorageType`
         // - `Effect: !NoBundleEffect`, which means the caller is responsible for calling this type's `apply_effect`
         //   at least once before returning to safe code.
-        <R::RelationshipTarget as DynamicBundle>::get_components(target, func);
+        unsafe { <R::RelationshipTarget as DynamicBundle>::get_components(target, func) };
         // Forget the pointer so that the value is available in `apply_effect`.
         mem::forget(ptr);
     }

crates/bevy_ecs/src/storage/blob_array.rs

@@ -54,7 +54,8 @@ impl BlobArray {
                 capacity,
             }
         } else {
-            let mut arr = Self::with_capacity(item_layout, drop_fn, 0);
+            // SAFETY: Upheld by caller
+            let mut arr = unsafe { Self::with_capacity(item_layout, drop_fn, 0) };
             // SAFETY: `capacity` > 0
             unsafe { arr.alloc(NonZeroUsize::new_unchecked(capacity)) }
             arr

crates/bevy_ecs/src/storage/mod.rs

@@ -102,7 +102,8 @@ impl<T> VecExtensions<T> for Vec<T> {
         // SAFETY: We replace self[index] with the last element. The caller must ensure that
         // both the last element and `index` must be valid and cannot point to the same place.
         unsafe { core::ptr::copy_nonoverlapping(base_ptr.add(len - 1), base_ptr.add(index), 1) };
-        self.set_len(len - 1);
+        // SAFETY: Upheld by caller
+        unsafe { self.set_len(len - 1) };
         value
     }
 }

crates/bevy_ecs/src/storage/table/column.rs

@@ -336,23 +336,26 @@ impl Column {
     /// - `T` must match the type of data that's stored in this [`Column`]
     /// - `len` must match the actual length of this column (number of elements stored)
     pub unsafe fn get_data_slice<T>(&self, len: usize) -> &[UnsafeCell<T>] {
-        self.data.get_sub_slice(len)
+        // SAFETY: Upheld by caller
+        unsafe { self.data.get_sub_slice(len) }
     }
 
     /// Get a slice to the added [`ticks`](Tick) in this [`Column`].
     ///
     /// # Safety
     /// - `len` must match the actual length of this column (number of elements stored)
     pub unsafe fn get_added_ticks_slice(&self, len: usize) -> &[UnsafeCell<Tick>] {
-        self.added_ticks.as_slice(len)
+        // SAFETY: Upheld by caller
+        unsafe { self.added_ticks.as_slice(len) }
     }
 
     /// Get a slice to the changed [`ticks`](Tick) in this [`Column`].
     ///
     /// # Safety
     /// - `len` must match the actual length of this column (number of elements stored)
     pub unsafe fn get_changed_ticks_slice(&self, len: usize) -> &[UnsafeCell<Tick>] {
-        self.changed_ticks.as_slice(len)
+        // SAFETY: Upheld by caller
+        unsafe { self.changed_ticks.as_slice(len) }
     }
 
     /// Get a slice to the calling locations that last changed each value in this [`Column`]
@@ -402,7 +405,8 @@ impl Column {
     /// `row` must be within the range `[0, self.len())`.
     #[inline]
     pub unsafe fn get_added_tick_unchecked(&self, row: TableRow) -> &UnsafeCell<Tick> {
-        self.added_ticks.get_unchecked(row.index())
+        // SAFETY: Upheld by caller
+        unsafe { self.added_ticks.get_unchecked(row.index()) }
     }
 
     /// Fetches the "changed" change detection tick for the value at `row`
@@ -412,7 +416,8 @@ impl Column {
     /// `row` must be within the range `[0, self.len())`.
     #[inline]
     pub unsafe fn get_changed_tick_unchecked(&self, row: TableRow) -> &UnsafeCell<Tick> {
-        self.changed_ticks.get_unchecked(row.index())
+        // SAFETY: Upheld by caller
+        unsafe { self.changed_ticks.get_unchecked(row.index()) }
     }
 
     /// Fetches the change detection ticks for the value at `row`.