Add basic debug checks for read-only UnsafeWorldCell (#17393)

# Objective

The method `World::as_unsafe_world_cell_readonly` is used to create an
`UnsafeWorldCell` which is only allowed to access world data immutably.
This can be tricky to use, as the data that an `UnsafeWorldCell` is
allowed to access exists only in documentation (you could think of it as
a "doc-time abstraction" rather than a "compile-time" abstraction). It's
quite easy to forget where a particular instance came from and attempt
to use it for mutable access, leading to instant, silent undefined
behavior.

## Solution

Add a debug-mode only flag to `UnsafeWorldCell` which tracks whether or
not the instance can be used to access world data mutably. This should
catch basic improper usages of `as_unsafe_world_cell_readonly`.

## Future work

There are a few ways that you can bypass the runtime checks introduced
by this PR:

* Any world accesses done via `UnsafeWorldCell::storages` are completely
invisible to these runtime checks. Unfortunately, `storages` constitutes
most of the world accesses used in the engine itself, so this PR will
mostly benefit downstream users of bevy.
* It's possible to call `get_resource_by_id`, and then convert the
returned `Ptr` to a `PtrMut` by calling `assert_unique`. In the future
we'll probably want to add a debug-mode only flag to `Ptr` which tracks
whether or not it can be upgraded to a `PtrMut`. I didn't include this
change in this PR as those types are currently defined using macros
which makes it a bit tricky to modify their definitions.
* Any data accesses done through a mutable `UnsafeWorldCell` are
completely unchecked, meaning it's possible to unsoundly create multiple
mutable references to a single component, for example. In the future we
may want to store an `Access<>` set inside of the world's `Storages` to
add granular debug-mode runtime checks.

That said, I'd consider this PR to be a good first step towards adding
full runtime checks to `UnsafeWorldCell`.

## Testing

Added a few tests that basic invalid mutable world access result in a
panic.

crates/bevy_ecs/src/world/unsafe_world_cell.rs

@@ -84,7 +84,12 @@ use portable_atomic::Ordering;
 /// }
 /// ```
 #[derive(Copy, Clone)]
-pub struct UnsafeWorldCell<'w>(*mut World, PhantomData<(&'w World, &'w UnsafeCell<World>)>);
+pub struct UnsafeWorldCell<'w> {
+    ptr: *mut World,
+    #[cfg(debug_assertions)]
+    allows_mutable_access: bool,
+    _marker: PhantomData<(&'w World, &'w UnsafeCell<World>)>,
+}
 
 // SAFETY: `&World` and `&mut World` are both `Send`
 unsafe impl Send for UnsafeWorldCell<'_> {}
@@ -107,13 +112,32 @@ impl<'w> UnsafeWorldCell<'w> {
     /// Creates a [`UnsafeWorldCell`] that can be used to access everything immutably
     #[inline]
     pub(crate) fn new_readonly(world: &'w World) -> Self {
-        Self(ptr::from_ref(world).cast_mut(), PhantomData)
+        Self {
+            ptr: ptr::from_ref(world).cast_mut(),
+            #[cfg(debug_assertions)]
+            allows_mutable_access: false,
+            _marker: PhantomData,
+        }
     }
 
     /// Creates [`UnsafeWorldCell`] that can be used to access everything mutably
     #[inline]
     pub(crate) fn new_mutable(world: &'w mut World) -> Self {
-        Self(ptr::from_mut(world), PhantomData)
+        Self {
+            ptr: ptr::from_mut(world),
+            #[cfg(debug_assertions)]
+            allows_mutable_access: true,
+            _marker: PhantomData,
+        }
+    }
+
+    #[cfg_attr(debug_assertions, inline(never), track_caller)]
+    #[cfg_attr(not(debug_assertions), inline(always))]
+    pub(crate) fn assert_allows_mutable_access(self) {
+        debug_assert!(
+            self.allows_mutable_access,
+            "mutating world data via `World::as_unsafe_world_cell_readonly` is forbidden"
+        );
     }
 
     /// Gets a mutable reference to the [`World`] this [`UnsafeWorldCell`] belongs to.
@@ -161,9 +185,10 @@ impl<'w> UnsafeWorldCell<'w> {
     /// ```
     #[inline]
     pub unsafe fn world_mut(self) -> &'w mut World {
+        self.assert_allows_mutable_access();
         // SAFETY:
         // - caller ensures the created `&mut World` is the only borrow of world
-        unsafe { &mut *self.0 }
+        unsafe { &mut *self.ptr }
     }
 
     /// Gets a reference to the [`&World`](World) this [`UnsafeWorldCell`] belongs to.
@@ -212,7 +237,7 @@ impl<'w> UnsafeWorldCell<'w> {
         // SAFETY:
         // - caller ensures that the returned `&World` is not used in a way that would conflict
         //   with any existing mutable borrows of world data
-        unsafe { &*self.0 }
+        unsafe { &*self.ptr }
     }
 
     /// Retrieves this world's unique [ID](WorldId).
@@ -457,6 +482,7 @@ impl<'w> UnsafeWorldCell<'w> {
     /// - no other references to the resource exist at the same time
     #[inline]
     pub unsafe fn get_resource_mut<R: Resource>(self) -> Option<Mut<'w, R>> {
+        self.assert_allows_mutable_access();
         let component_id = self.components().get_resource_id(TypeId::of::<R>())?;
         // SAFETY:
         // - caller ensures `self` has permission to access the resource mutably
@@ -484,6 +510,7 @@ impl<'w> UnsafeWorldCell<'w> {
         self,
         component_id: ComponentId,
     ) -> Option<MutUntyped<'w>> {
+        self.assert_allows_mutable_access();
         // SAFETY: we only access data that the caller has ensured is unaliased and `self`
         //  has permission to access.
         let (ptr, ticks, _caller) = unsafe { self.storages() }
@@ -520,6 +547,7 @@ impl<'w> UnsafeWorldCell<'w> {
     /// - no other references to the resource exist at the same time
     #[inline]
     pub unsafe fn get_non_send_resource_mut<R: 'static>(self) -> Option<Mut<'w, R>> {
+        self.assert_allows_mutable_access();
         let component_id = self.components().get_resource_id(TypeId::of::<R>())?;
         // SAFETY:
         // - caller ensures that `self` has permission to access the resource
@@ -550,6 +578,7 @@ impl<'w> UnsafeWorldCell<'w> {
         self,
         component_id: ComponentId,
     ) -> Option<MutUntyped<'w>> {
+        self.assert_allows_mutable_access();
         let change_tick = self.change_tick();
         // SAFETY: we only access data that the caller has ensured is unaliased and `self`
         //  has permission to access.
@@ -623,18 +652,20 @@ impl<'w> UnsafeWorldCell<'w> {
     /// - the [`UnsafeWorldCell`] has permission to access the queue mutably
     /// - no mutable references to the queue exist at the same time
     pub(crate) unsafe fn get_raw_command_queue(self) -> RawCommandQueue {
+        self.assert_allows_mutable_access();
         // SAFETY:
         // - caller ensures there are no existing mutable references
         // - caller ensures that we have permission to access the queue
-        unsafe { (*self.0).command_queue.clone() }
+        unsafe { (*self.ptr).command_queue.clone() }
     }
 
     /// # Safety
     /// It is the callers responsibility to ensure that there are no outstanding
     /// references to `last_trigger_id`.
     pub(crate) unsafe fn increment_trigger_id(self) {
+        self.assert_allows_mutable_access();
         // SAFETY: Caller ensure there are no outstanding references
-        unsafe { (*self.0).last_trigger_id += 1 }
+        unsafe { (*self.ptr).last_trigger_id += 1 }
     }
 }
 
@@ -884,6 +915,8 @@ impl<'w> UnsafeEntityCell<'w> {
         last_change_tick: Tick,
         change_tick: Tick,
     ) -> Option<Mut<'w, T>> {
+        self.world.assert_allows_mutable_access();
+
         let component_id = self.world.components().get_id(TypeId::of::<T>())?;
 
         // SAFETY:
@@ -1000,6 +1033,8 @@ impl<'w> UnsafeEntityCell<'w> {
         self,
         component_id: ComponentId,
     ) -> Result<MutUntyped<'w>, GetEntityMutByIdError> {
+        self.world.assert_allows_mutable_access();
+
         let info = self
             .world
             .components()
@@ -1184,3 +1219,45 @@ impl EntityBorrow for UnsafeEntityCell<'_> {
         self.id()
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate as bevy_ecs;
+
+    #[test]
+    #[should_panic = "is forbidden"]
+    fn as_unsafe_world_cell_readonly_world_mut_forbidden() {
+        let world = World::new();
+        let world_cell = world.as_unsafe_world_cell_readonly();
+        // SAFETY: this invalid usage will be caught by a runtime panic.
+        let _ = unsafe { world_cell.world_mut() };
+    }
+
+    #[derive(Resource)]
+    struct R;
+
+    #[test]
+    #[should_panic = "is forbidden"]
+    fn as_unsafe_world_cell_readonly_resource_mut_forbidden() {
+        let mut world = World::new();
+        world.insert_resource(R);
+        let world_cell = world.as_unsafe_world_cell_readonly();
+        // SAFETY: this invalid usage will be caught by a runtime panic.
+        let _ = unsafe { world_cell.get_resource_mut::<R>() };
+    }
+
+    #[derive(Component)]
+    struct C;
+
+    #[test]
+    #[should_panic = "is forbidden"]
+    fn as_unsafe_world_cell_readonly_component_mut_forbidden() {
+        let mut world = World::new();
+        let entity = world.spawn(C).id();
+        let world_cell = world.as_unsafe_world_cell_readonly();
+        let entity_cell = world_cell.get_entity(entity).unwrap();
+        // SAFETY: this invalid usage will be caught by a runtime panic.
+        let _ = unsafe { entity_cell.get_mut::<C>() };
+    }
+}