bevy_ecs address trivial cases of unsafe_op_in_unsafe_fn (bevyengin…

…e#11861)

# Objective

- Part of bevyengine#11590
- Fix `unsafe_op_in_unsafe_fn` for trivial cases in bevy_ecs

## Solution

Fix `unsafe_op_in_unsafe_fn` in bevy_ecs for trivial cases, i.e., add an
`unsafe` block when the safety comment already exists or add a comment
like "The invariants are uphold by the caller".

---------

Co-authored-by: James Liu <contact@jamessliu.com>

crates/bevy_ecs/src/archetype.rs

@@ -134,7 +134,7 @@ impl BundleComponentStatus for AddBundle {
     #[inline]
     unsafe fn get_status(&self, index: usize) -> ComponentStatus {
         // SAFETY: caller has ensured index is a valid bundle index for this bundle
-        *self.bundle_status.get_unchecked(index)
+        unsafe { *self.bundle_status.get_unchecked(index) }
     }
 }
 

crates/bevy_ecs/src/bundle.rs

@@ -192,8 +192,9 @@ unsafe impl<C: Component> Bundle for C {
         F: for<'a> FnMut(&'a mut T) -> OwningPtr<'a>,
         Self: Sized,
     {
+        let ptr = func(ctx);
         // Safety: The id given in `component_ids` is for `Self`
-        func(ctx).read()
+        unsafe { ptr.read() }
     }
 }
 
@@ -224,9 +225,10 @@ macro_rules! tuple_impl {
             where
                 F: FnMut(&mut T) -> OwningPtr<'_>
             {
-                // Rust guarantees that tuple calls are evaluated 'left to right'.
+                #[allow(unused_unsafe)]
+                // SAFETY: Rust guarantees that tuple calls are evaluated 'left to right'.
                 // https://doc.rust-lang.org/reference/expressions.html#evaluation-order-of-operands
-                ($(<$name as Bundle>::from_components(ctx, func),)*)
+                unsafe { ($(<$name as Bundle>::from_components(ctx, func),)*) }
             }
         }
 
@@ -466,7 +468,8 @@ impl BundleInfo {
                         // the target table contains the component.
                         unsafe { table.get_column_mut(component_id).debug_checked_unwrap() };
                     // SAFETY: bundle_component is a valid index for this bundle
-                    match bundle_component_status.get_status(bundle_component) {
+                    let status = unsafe { bundle_component_status.get_status(bundle_component) };
+                    match status {
                         ComponentStatus::Added => {
                             column.initialize(table_row, component_ptr, change_tick);
                         }
@@ -702,9 +705,11 @@ impl<'a, 'b> BundleInserter<'a, 'b> {
                         new_archetype
                     } else {
                         // SAFETY: the only two borrowed archetypes are above and we just did collision checks
-                        &mut *self
-                            .archetypes_ptr
-                            .add(swapped_location.archetype_id.index())
+                        unsafe {
+                            &mut *self
+                                .archetypes_ptr
+                                .add(swapped_location.archetype_id.index())
+                        }
                     };
 
                     self.entities.set(
@@ -787,7 +792,9 @@ impl<'a, 'b> BundleSpawner<'a, 'b> {
     pub unsafe fn spawn<T: Bundle>(&mut self, bundle: T) -> Entity {
         let entity = self.entities.alloc();
         // SAFETY: entity is allocated (but non-existent), `T` matches this BundleInfo's type
-        self.spawn_non_existent(entity, bundle);
+        unsafe {
+            self.spawn_non_existent(entity, bundle);
+        }
         entity
     }
 }

crates/bevy_ecs/src/change_detection.rs

@@ -427,8 +427,10 @@ impl<'w> Ticks<'w> {
         this_run: Tick,
     ) -> Self {
         Self {
-            added: cells.added.deref(),
-            changed: cells.changed.deref(),
+            // SAFETY: Caller ensures there is no mutable access to the cell.
+            added: unsafe { cells.added.deref() },
+            // SAFETY: Caller ensures there is no mutable access to the cell.
+            changed: unsafe { cells.changed.deref() },
             last_run,
             this_run,
         }
@@ -452,8 +454,10 @@ impl<'w> TicksMut<'w> {
         this_run: Tick,
     ) -> Self {
         Self {
-            added: cells.added.deref_mut(),
-            changed: cells.changed.deref_mut(),
+            // SAFETY: Caller ensures there is no alias to the cell.
+            added: unsafe { cells.added.deref_mut() },
+            // SAFETY: Caller ensures there is no alias to the cell.
+            changed: unsafe { cells.changed.deref_mut() },
             last_run,
             this_run,
         }
@@ -879,7 +883,8 @@ impl<'w> MutUntyped<'w> {
     /// - `T` must be the erased pointee type for this [`MutUntyped`].
     pub unsafe fn with_type<T>(self) -> Mut<'w, T> {
         Mut {
-            value: self.value.deref_mut(),
+            // SAFETY: `value` is `Aligned` and caller ensures the pointee type is `T`.
+            value: unsafe { self.value.deref_mut() },
             ticks: self.ticks,
         }
     }

crates/bevy_ecs/src/component.rs

@@ -357,9 +357,14 @@ impl std::fmt::Debug for ComponentDescriptor {
 }
 
 impl ComponentDescriptor {
-    // SAFETY: The pointer points to a valid value of type `T` and it is safe to drop this value.
+    /// # SAFETY
+    ///
+    /// `x` must points to a valid value of type `T`.
     unsafe fn drop_ptr<T>(x: OwningPtr<'_>) {
-        x.drop_as::<T>();
+        // SAFETY: Contract is required to be upheld by the caller.
+        unsafe {
+            x.drop_as::<T>();
+        }
     }
 
     /// Create a new `ComponentDescriptor` for the type `T`.
@@ -542,7 +547,8 @@ impl Components {
     #[inline]
     pub unsafe fn get_info_unchecked(&self, id: ComponentId) -> &ComponentInfo {
         debug_assert!(id.index() < self.components.len());
-        self.components.get_unchecked(id.0)
+        // SAFETY: The caller ensures `id` is valid.
+        unsafe { self.components.get_unchecked(id.0) }
     }
 
     /// Type-erased equivalent of [`Components::component_id()`].
@@ -763,8 +769,10 @@ impl<'a> TickCells<'a> {
     #[inline]
     pub(crate) unsafe fn read(&self) -> ComponentTicks {
         ComponentTicks {
-            added: self.added.read(),
-            changed: self.changed.read(),
+            // SAFETY: The callers uphold the invariants for `read`.
+            added: unsafe { self.added.read() },
+            // SAFETY: The callers uphold the invariants for `read`.
+            changed: unsafe { self.changed.read() },
         }
     }
 }

crates/bevy_ecs/src/entity/mod.rs

@@ -741,7 +741,8 @@ impl Entities {
     #[inline]
     pub(crate) unsafe fn set(&mut self, index: u32, location: EntityLocation) {
         // SAFETY: Caller guarantees that `index` a valid entity index
-        self.meta.get_unchecked_mut(index as usize).location = location;
+        let meta = unsafe { self.meta.get_unchecked_mut(index as usize) };
+        meta.location = location;
     }
 
     /// Increments the `generation` of a freed [`Entity`]. The next entity ID allocated with this
@@ -848,9 +849,14 @@ impl Entities {
         let free_cursor = self.free_cursor.get_mut();
         *free_cursor = 0;
         self.meta.reserve(count);
-        // the EntityMeta struct only contains integers, and it is valid to have all bytes set to u8::MAX
-        self.meta.as_mut_ptr().write_bytes(u8::MAX, count);
-        self.meta.set_len(count);
+        // SAFETY: The EntityMeta struct only contains integers, and it is valid to have all bytes set to u8::MAX
+        unsafe {
+            self.meta.as_mut_ptr().write_bytes(u8::MAX, count);
+        }
+        // SAFETY: We have reserved `count` elements above and we have initialized values from index 0 to `count`.
+        unsafe {
+            self.meta.set_len(count);
+        }
 
         self.len = count as u32;
     }