diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index d6a67d8fffd4b..b333b09378434 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/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) } } } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index f976e533ac3b3..f0266f6a676cf 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -192,8 +192,9 @@ unsafe impl 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(&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 } } diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index 6a2fb3a9921af..7fda0a13f9ea6 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/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(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, } } diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 57a4e6dce1ab7..61349f4c86e0f 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/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(x: OwningPtr<'_>) { - x.drop_as::(); + // SAFETY: Contract is required to be upheld by the caller. + unsafe { + x.drop_as::(); + } } /// 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() }, } } } diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index efc0a22458195..a1715fee58d49 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/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; } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 27f329c7b7103..4fb9f8a111367 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -389,9 +389,9 @@ unsafe impl<'a> WorldQuery for EntityRef<'a> { _table_row: TableRow, ) -> Self::Item<'w> { // SAFETY: `fetch` must be called with an entity that exists in the world - let cell = world.get_entity(entity).debug_checked_unwrap(); + let cell = unsafe { world.get_entity(entity).debug_checked_unwrap() }; // SAFETY: Read-only access to every component has been registered. - EntityRef::new(cell) + unsafe { EntityRef::new(cell) } } fn update_component_access(_state: &Self::State, access: &mut FilteredAccess) { @@ -465,9 +465,9 @@ unsafe impl<'a> WorldQuery for EntityMut<'a> { _table_row: TableRow, ) -> Self::Item<'w> { // SAFETY: `fetch` must be called with an entity that exists in the world - let cell = world.get_entity(entity).debug_checked_unwrap(); + let cell = unsafe { world.get_entity(entity).debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - EntityMut::new(cell) + unsafe { EntityMut::new(cell) } } fn update_component_access(_state: &Self::State, access: &mut FilteredAccess) { @@ -560,9 +560,9 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { _table_row: TableRow, ) -> Self::Item<'w> { // SAFETY: `fetch` must be called with an entity that exists in the world - let cell = world.get_entity(entity).debug_checked_unwrap(); + let cell = unsafe { world.get_entity(entity).debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - FilteredEntityRef::new(cell, access.clone()) + unsafe { FilteredEntityRef::new(cell, access.clone()) } } fn update_component_access( @@ -672,9 +672,9 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { _table_row: TableRow, ) -> Self::Item<'w> { // SAFETY: `fetch` must be called with an entity that exists in the world - let cell = world.get_entity(entity).debug_checked_unwrap(); + let cell = unsafe { world.get_entity(entity).debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - FilteredEntityMut::new(cell, access.clone()) + unsafe { FilteredEntityMut::new(cell, access.clone()) } } fn update_component_access( @@ -748,15 +748,17 @@ unsafe impl WorldQuery for &T { ReadFetch { table_components: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { - world - // SAFETY: The underlying type associated with `component_id` is `T`, - // which we are allowed to access since we registered it in `update_archetype_component_access`. - // Note that we do not actually access any components in this function, we just get a shared - // reference to the sparse set, which is used to access the components in `Self::fetch`. - .storages() - .sparse_sets - .get(component_id) - .debug_checked_unwrap() + // SAFETY: The underlying type associated with `component_id` is `T`, + // which we are allowed to access since we registered it in `update_archetype_component_access`. + // Note that we do not actually access any components in this function, we just get a shared + // reference to the sparse set, which is used to access the components in `Self::fetch`. + unsafe { + world + .storages() + .sparse_sets + .get(component_id) + .debug_checked_unwrap() + } }), } } @@ -776,7 +778,10 @@ unsafe impl WorldQuery for &T { table: &'w Table, ) { if Self::IS_DENSE { - Self::set_table(fetch, component_id, table); + // SAFETY: `set_archetype`'s safety rules are a super set of the `set_table`'s ones. + unsafe { + Self::set_table(fetch, component_id, table); + } } } @@ -802,17 +807,20 @@ unsafe impl WorldQuery for &T { table_row: TableRow, ) -> Self::Item<'w> { match T::Storage::STORAGE_TYPE { - StorageType::Table => fetch - .table_components - .debug_checked_unwrap() - .get(table_row.as_usize()) - .deref(), - StorageType::SparseSet => fetch - .sparse_set - .debug_checked_unwrap() - .get(entity) - .debug_checked_unwrap() - .deref(), + StorageType::Table => { + // SAFETY: STORAGE_TYPE = Table + let table = unsafe { fetch.table_components.debug_checked_unwrap() }; + // SAFETY: Caller ensures `table_row` is in range. + let item = unsafe { table.get(table_row.as_usize()) }; + item.deref() + } + StorageType::SparseSet => { + // SAFETY: STORAGE_TYPE = SparseSet + let sparse_set = unsafe { fetch.sparse_set.debug_checked_unwrap() }; + // SAFETY: Caller ensures `entity` is in range. + let item = unsafe { sparse_set.get(entity).debug_checked_unwrap() }; + item.deref() + } } } @@ -898,12 +906,17 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { RefFetch { table_data: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { - world - // SAFETY: See &T::init_fetch. - .storages() - .sparse_sets - .get(component_id) - .debug_checked_unwrap() + // SAFETY: The underlying type associated with `component_id` is `T`, + // which we are allowed to access since we registered it in `update_archetype_component_access`. + // Note that we do not actually access any components in this function, we just get a shared + // reference to the sparse set, which is used to access the components in `Self::fetch`. + unsafe { + world + .storages() + .sparse_sets + .get(component_id) + .debug_checked_unwrap() + } }), last_run, this_run, @@ -925,7 +938,10 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { table: &'w Table, ) { if Self::IS_DENSE { - Self::set_table(fetch, component_id, table); + // SAFETY: `set_archetype`'s safety rules are a super set of the `set_table`'s ones. + unsafe { + Self::set_table(fetch, component_id, table); + } } } @@ -951,24 +967,38 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { ) -> Self::Item<'w> { match T::Storage::STORAGE_TYPE { StorageType::Table => { + // SAFETY: STORAGE_TYPE = Table let (table_components, added_ticks, changed_ticks) = - fetch.table_data.debug_checked_unwrap(); + unsafe { fetch.table_data.debug_checked_unwrap() }; + + // SAFETY: The caller ensures `table_row` is in range. + let component = unsafe { table_components.get(table_row.as_usize()) }; + // SAFETY: The caller ensures `table_row` is in range. + let added = unsafe { added_ticks.get(table_row.as_usize()) }; + // SAFETY: The caller ensures `table_row` is in range. + let changed = unsafe { changed_ticks.get(table_row.as_usize()) }; + Ref { - value: table_components.get(table_row.as_usize()).deref(), + value: component.deref(), ticks: Ticks { - added: added_ticks.get(table_row.as_usize()).deref(), - changed: changed_ticks.get(table_row.as_usize()).deref(), + added: added.deref(), + changed: changed.deref(), this_run: fetch.this_run, last_run: fetch.last_run, }, } } StorageType::SparseSet => { - let (component, ticks) = fetch - .sparse_set - .debug_checked_unwrap() - .get_with_ticks(entity) - .debug_checked_unwrap(); + // SAFETY: STORAGE_TYPE = SparseSet + let component_sparse_set = unsafe { fetch.sparse_set.debug_checked_unwrap() }; + + // SAFETY: The caller ensures `entity` is in range. + let (component, ticks) = unsafe { + component_sparse_set + .get_with_ticks(entity) + .debug_checked_unwrap() + }; + Ref { value: component.deref(), ticks: Ticks::from_tick_cells(ticks, fetch.last_run, fetch.this_run), @@ -1059,12 +1089,17 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { WriteFetch { table_data: None, sparse_set: (T::Storage::STORAGE_TYPE == StorageType::SparseSet).then(|| { - world - // SAFETY: See &T::init_fetch. - .storages() - .sparse_sets - .get(component_id) - .debug_checked_unwrap() + // SAFETY: The underlying type associated with `component_id` is `T`, + // which we are allowed to access since we registered it in `update_archetype_component_access`. + // Note that we do not actually access any components in this function, we just get a shared + // reference to the sparse set, which is used to access the components in `Self::fetch`. + unsafe { + world + .storages() + .sparse_sets + .get(component_id) + .debug_checked_unwrap() + } }), last_run, this_run, @@ -1086,7 +1121,10 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { table: &'w Table, ) { if Self::IS_DENSE { - Self::set_table(fetch, component_id, table); + // SAFETY: `set_archetype`'s safety rules are a super set of the `set_table`'s ones. + unsafe { + Self::set_table(fetch, component_id, table); + } } } @@ -1112,24 +1150,38 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { ) -> Self::Item<'w> { match T::Storage::STORAGE_TYPE { StorageType::Table => { + // SAFETY: STORAGE_TYPE = Table let (table_components, added_ticks, changed_ticks) = - fetch.table_data.debug_checked_unwrap(); + unsafe { fetch.table_data.debug_checked_unwrap() }; + + // SAFETY: The caller ensures `table_row` is in range. + let component = unsafe { table_components.get(table_row.as_usize()) }; + // SAFETY: The caller ensures `table_row` is in range. + let added = unsafe { added_ticks.get(table_row.as_usize()) }; + // SAFETY: The caller ensures `table_row` is in range. + let changed = unsafe { changed_ticks.get(table_row.as_usize()) }; + Mut { - value: table_components.get(table_row.as_usize()).deref_mut(), + value: component.deref_mut(), ticks: TicksMut { - added: added_ticks.get(table_row.as_usize()).deref_mut(), - changed: changed_ticks.get(table_row.as_usize()).deref_mut(), + added: added.deref_mut(), + changed: changed.deref_mut(), this_run: fetch.this_run, last_run: fetch.last_run, }, } } StorageType::SparseSet => { - let (component, ticks) = fetch - .sparse_set - .debug_checked_unwrap() - .get_with_ticks(entity) - .debug_checked_unwrap(); + // SAFETY: STORAGE_TYPE = SparseSet + let component_sparse_set = unsafe { fetch.sparse_set.debug_checked_unwrap() }; + + // SAFETY: The caller ensures `entity` is in range. + let (component, ticks) = unsafe { + component_sparse_set + .get_with_ticks(entity) + .debug_checked_unwrap() + }; + Mut { value: component.assert_unique().deref_mut(), ticks: TicksMut::from_tick_cells(ticks, fetch.last_run, fetch.this_run), @@ -1207,7 +1259,8 @@ unsafe impl WorldQuery for Option { this_run: Tick, ) -> OptionFetch<'w, T> { OptionFetch { - fetch: T::init_fetch(world, state, last_run, this_run), + // SAFETY: The invariants are uphold by the caller. + fetch: unsafe { T::init_fetch(world, state, last_run, this_run) }, matches: false, } } @@ -1223,7 +1276,10 @@ unsafe impl WorldQuery for Option { ) { fetch.matches = T::matches_component_set(state, &|id| archetype.contains(id)); if fetch.matches { - T::set_archetype(&mut fetch.fetch, state, archetype, table); + // SAFETY: The invariants are uphold by the caller. + unsafe { + T::set_archetype(&mut fetch.fetch, state, archetype, table); + } } } @@ -1231,7 +1287,10 @@ unsafe impl WorldQuery for Option { unsafe fn set_table<'w>(fetch: &mut OptionFetch<'w, T>, state: &T::State, table: &'w Table) { fetch.matches = T::matches_component_set(state, &|id| table.has_column(id)); if fetch.matches { - T::set_table(&mut fetch.fetch, state, table); + // SAFETY: The invariants are uphold by the caller. + unsafe { + T::set_table(&mut fetch.fetch, state, table); + } } } @@ -1243,7 +1302,8 @@ unsafe impl WorldQuery for Option { ) -> Self::Item<'w> { fetch .matches - .then(|| T::fetch(&mut fetch.fetch, entity, table_row)) + // SAFETY: The invariants are uphold by the caller. + .then(|| unsafe { T::fetch(&mut fetch.fetch, entity, table_row) }) } fn update_component_access(state: &T::State, access: &mut FilteredAccess) { @@ -1484,7 +1544,8 @@ macro_rules! impl_anytuple_fetch { #[allow(clippy::unused_unit)] unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; - ($(($name::init_fetch(_world, $name, _last_run, _this_run), false),)*) + // SAFETY: The invariants are uphold by the caller. + ($(( unsafe { $name::init_fetch(_world, $name, _last_run, _this_run) }, false),)*) } const IS_DENSE: bool = true $(&& $name::IS_DENSE)*; @@ -1501,7 +1562,8 @@ macro_rules! impl_anytuple_fetch { $( $name.1 = $name::matches_component_set($state, &|id| _archetype.contains(id)); if $name.1 { - $name::set_archetype(&mut $name.0, $state, _archetype, _table); + // SAFETY: The invariants are uphold by the caller. + unsafe { $name::set_archetype(&mut $name.0, $state, _archetype, _table); } } )* } @@ -1513,7 +1575,8 @@ macro_rules! impl_anytuple_fetch { $( $name.1 = $name::matches_component_set($state, &|id| _table.has_column(id)); if $name.1 { - $name::set_table(&mut $name.0, $state, _table); + // SAFETY: The invariants are required to be upheld by the caller. + unsafe { $name::set_table(&mut $name.0, $state, _table); } } )* } @@ -1527,7 +1590,8 @@ macro_rules! impl_anytuple_fetch { ) -> Self::Item<'w> { let ($($name,)*) = _fetch; ($( - $name.1.then(|| $name::fetch(&mut $name.0, _entity, _table_row)), + // SAFETY: The invariants are required to be upheld by the caller. + $name.1.then(|| unsafe { $name::fetch(&mut $name.0, _entity, _table_row) }), )*) } diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 0216ad5a43df9..e88293e227120 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -382,7 +382,8 @@ macro_rules! impl_query_filter_tuple { unsafe fn init_fetch<'w>(world: UnsafeWorldCell<'w>, state: &Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> { let ($($filter,)*) = state; ($(OrFetch { - fetch: $filter::init_fetch(world, $filter, last_run, this_run), + // SAFETY: The invariants are uphold by the caller. + fetch: unsafe { $filter::init_fetch(world, $filter, last_run, this_run) }, matches: false, },)*) } @@ -394,7 +395,8 @@ macro_rules! impl_query_filter_tuple { $( $filter.matches = $filter::matches_component_set($state, &|id| table.has_column(id)); if $filter.matches { - $filter::set_table(&mut $filter.fetch, $state, table); + // SAFETY: The invariants are uphold by the caller. + unsafe { $filter::set_table(&mut $filter.fetch, $state, table); } } )* } @@ -411,7 +413,8 @@ macro_rules! impl_query_filter_tuple { $( $filter.matches = $filter::matches_component_set($state, &|id| archetype.contains(id)); if $filter.matches { - $filter::set_archetype(&mut $filter.fetch, $state, archetype, table); + // SAFETY: The invariants are uphold by the caller. + unsafe { $filter::set_archetype(&mut $filter.fetch, $state, archetype, table); } } )* } @@ -423,7 +426,8 @@ macro_rules! impl_query_filter_tuple { _table_row: TableRow ) -> Self::Item<'w> { let ($($filter,)*) = fetch; - false $(|| ($filter.matches && $filter::filter_fetch(&mut $filter.fetch, _entity, _table_row)))* + // SAFETY: The invariants are uphold by the caller. + false $(|| ($filter.matches && unsafe { $filter::filter_fetch(&mut $filter.fetch, _entity, _table_row) }))* } fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { @@ -470,7 +474,8 @@ macro_rules! impl_query_filter_tuple { entity: Entity, table_row: TableRow ) -> bool { - Self::fetch(fetch, entity, table_row) + // SAFETY: The invariants are uphold by the caller. + unsafe { Self::fetch(fetch, entity, table_row) } } } }; @@ -492,7 +497,8 @@ macro_rules! impl_tuple_query_filter { _table_row: TableRow ) -> bool { let ($($name,)*) = fetch; - true $(&& $name::filter_fetch($name, _entity, _table_row))* + // SAFETY: The invariants are uphold by the caller. + true $(&& unsafe { $name::filter_fetch($name, _entity, _table_row) })* } } @@ -620,7 +626,10 @@ unsafe impl WorldQuery for Added { table: &'w Table, ) { if Self::IS_DENSE { - Self::set_table(fetch, component_id, table); + // SAFETY: `set_archetype`'s safety rules are a super set of the `set_table`'s ones. + unsafe { + Self::set_table(fetch, component_id, table); + } } } @@ -643,18 +652,23 @@ unsafe impl WorldQuery for Added { table_row: TableRow, ) -> Self::Item<'w> { match T::Storage::STORAGE_TYPE { - StorageType::Table => fetch - .table_ticks - .debug_checked_unwrap() - .get(table_row.as_usize()) - .deref() - .is_newer_than(fetch.last_run, fetch.this_run), + StorageType::Table => { + // SAFETY: STORAGE_TYPE = Table + let table = unsafe { fetch.table_ticks.debug_checked_unwrap() }; + // SAFETY: The caller ensures `table_row` is in range. + let tick = unsafe { table.get(table_row.as_usize()) }; + + tick.deref().is_newer_than(fetch.last_run, fetch.this_run) + } StorageType::SparseSet => { - let sparse_set = &fetch.sparse_set.debug_checked_unwrap(); - ComponentSparseSet::get_added_tick(sparse_set, entity) - .debug_checked_unwrap() - .deref() - .is_newer_than(fetch.last_run, fetch.this_run) + // SAFETY: STORAGE_TYPE = SparseSet + let sparse_set = unsafe { &fetch.sparse_set.debug_checked_unwrap() }; + // SAFETY: The caller ensures `entity` is in range. + let tick = unsafe { + ComponentSparseSet::get_added_tick(sparse_set, entity).debug_checked_unwrap() + }; + + tick.deref().is_newer_than(fetch.last_run, fetch.this_run) } } } @@ -691,7 +705,8 @@ impl QueryFilter for Added { entity: Entity, table_row: TableRow, ) -> bool { - Self::fetch(fetch, entity, table_row) + // SAFETY: The invariants are uphold by the caller. + unsafe { Self::fetch(fetch, entity, table_row) } } } @@ -820,7 +835,10 @@ unsafe impl WorldQuery for Changed { table: &'w Table, ) { if Self::IS_DENSE { - Self::set_table(fetch, component_id, table); + // SAFETY: `set_archetype`'s safety rules are a super set of the `set_table`'s ones. + unsafe { + Self::set_table(fetch, component_id, table); + } } } @@ -843,18 +861,23 @@ unsafe impl WorldQuery for Changed { table_row: TableRow, ) -> Self::Item<'w> { match T::Storage::STORAGE_TYPE { - StorageType::Table => fetch - .table_ticks - .debug_checked_unwrap() - .get(table_row.as_usize()) - .deref() - .is_newer_than(fetch.last_run, fetch.this_run), + StorageType::Table => { + // SAFETY: STORAGE_TYPE = Table + let table = unsafe { fetch.table_ticks.debug_checked_unwrap() }; + // SAFETY: The caller ensures `table_row` is in range. + let tick = unsafe { table.get(table_row.as_usize()) }; + + tick.deref().is_newer_than(fetch.last_run, fetch.this_run) + } StorageType::SparseSet => { - let sparse_set = &fetch.sparse_set.debug_checked_unwrap(); - ComponentSparseSet::get_changed_tick(sparse_set, entity) - .debug_checked_unwrap() - .deref() - .is_newer_than(fetch.last_run, fetch.this_run) + // SAFETY: STORAGE_TYPE = SparseSet + let sparse_set = unsafe { &fetch.sparse_set.debug_checked_unwrap() }; + // SAFETY: The caller ensures `entity` is in range. + let tick = unsafe { + ComponentSparseSet::get_changed_tick(sparse_set, entity).debug_checked_unwrap() + }; + + tick.deref().is_newer_than(fetch.last_run, fetch.this_run) } } } @@ -892,7 +915,8 @@ impl QueryFilter for Changed { entity: Entity, table_row: TableRow, ) -> bool { - Self::fetch(fetch, entity, table_row) + // SAFETY: The invariants are uphold by the caller. + unsafe { Self::fetch(fetch, entity, table_row) } } } diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index e79371c0d7a6e..8d47073ccf070 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -34,9 +34,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { QueryIter { query_state, // SAFETY: We only access table data that has been registered in `query_state`. - tables: &world.storages().tables, + tables: unsafe { &world.storages().tables }, archetypes: world.archetypes(), - cursor: QueryIterationCursor::init(world, query_state, last_run, this_run), + // SAFETY: The invariants are uphold by the caller. + cursor: unsafe { QueryIterationCursor::init(world, query_state, last_run, this_run) }, } } @@ -121,11 +122,13 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { let entities = table.entities(); for row in rows { // SAFETY: Caller assures `row` in range of the current archetype. - let entity = entities.get_unchecked(row); + let entity = unsafe { entities.get_unchecked(row) }; let row = TableRow::from_usize(row); + // SAFETY: set_table was called prior. // Caller assures `row` in range of the current archetype. - if !F::filter_fetch(&mut self.cursor.filter, *entity, row) { + let fetched = unsafe { !F::filter_fetch(&mut self.cursor.filter, *entity, row) }; + if fetched { continue; } @@ -173,24 +176,30 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { let entities = archetype.entities(); for index in indices { // SAFETY: Caller assures `index` in range of the current archetype. - let archetype_entity = entities.get_unchecked(index); + let archetype_entity = unsafe { entities.get_unchecked(index) }; + // SAFETY: set_archetype was called prior. // Caller assures `index` in range of the current archetype. - if !F::filter_fetch( - &mut self.cursor.filter, - archetype_entity.id(), - archetype_entity.table_row(), - ) { + let fetched = unsafe { + !F::filter_fetch( + &mut self.cursor.filter, + archetype_entity.id(), + archetype_entity.table_row(), + ) + }; + if fetched { continue; } // SAFETY: set_archetype was called prior, `index` is an archetype index in range of the current archetype // Caller assures `index` in range of the current archetype. - let item = D::fetch( - &mut self.cursor.fetch, - archetype_entity.id(), - archetype_entity.table_row(), - ); + let item = unsafe { + D::fetch( + &mut self.cursor.fetch, + archetype_entity.id(), + archetype_entity.table_row(), + ) + }; accum = func(accum, item); } @@ -339,28 +348,32 @@ where // SAFETY: `archetype` is from the world that `fetch/filter` were created for, // `fetch_state`/`filter_state` are the states that `fetch/filter` were initialized with - D::set_archetype( - &mut self.fetch, - &self.query_state.fetch_state, - archetype, - table, - ); + unsafe { + D::set_archetype( + &mut self.fetch, + &self.query_state.fetch_state, + archetype, + table, + ); + } // SAFETY: `table` is from the world that `fetch/filter` were created for, // `fetch_state`/`filter_state` are the states that `fetch/filter` were initialized with - F::set_archetype( - &mut self.filter, - &self.query_state.filter_state, - archetype, - table, - ); + unsafe { + F::set_archetype( + &mut self.filter, + &self.query_state.filter_state, + archetype, + table, + ); + } // SAFETY: set_archetype was called prior. // `location.archetype_row` is an archetype index row in range of the current archetype, because if it was not, the match above would have `continue`d - if F::filter_fetch(&mut self.filter, entity, location.table_row) { + if unsafe { F::filter_fetch(&mut self.filter, entity, location.table_row) } { // SAFETY: // - set_archetype was called prior, `location.archetype_row` is an archetype index in range of the current archetype // - fetch is only called once for each entity. - return Some(D::fetch(&mut self.fetch, entity, location.table_row)); + return Some(unsafe { D::fetch(&mut self.fetch, entity, location.table_row) }); } } None @@ -510,7 +523,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, const K: usize> QueryCombinationIter< QueryCombinationIter { query_state, // SAFETY: We only access table data that has been registered in `query_state`. - tables: &world.storages().tables, + tables: unsafe { &world.storages().tables }, archetypes: world.archetypes(), cursors: array.assume_init(), } @@ -764,8 +777,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let table = tables.get(*table_id).debug_checked_unwrap(); // SAFETY: `table` is from the world that `fetch/filter` were created for, // `fetch_state`/`filter_state` are the states that `fetch/filter` were initialized with - D::set_table(&mut self.fetch, &query_state.fetch_state, table); - F::set_table(&mut self.filter, &query_state.filter_state, table); + unsafe { + D::set_table(&mut self.fetch, &query_state.fetch_state, table); + F::set_table(&mut self.filter, &query_state.filter_state, table); + } self.table_entities = table.entities(); self.current_len = table.entity_count(); self.current_row = 0; @@ -774,7 +789,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // SAFETY: set_table was called prior. // `current_row` is a table row in range of the current table, because if it was not, then the if above would have been executed. - let entity = self.table_entities.get_unchecked(self.current_row); + let entity = unsafe { self.table_entities.get_unchecked(self.current_row) }; let row = TableRow::from_usize(self.current_row); if !F::filter_fetch(&mut self.filter, *entity, row) { self.current_row += 1; @@ -786,7 +801,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - `current_row` must be a table row in range of the current table, // because if it was not, then the if above would have been executed. // - fetch is only called once for each `entity`. - let item = D::fetch(&mut self.fetch, *entity, row); + let item = unsafe { D::fetch(&mut self.fetch, *entity, row) }; self.current_row += 1; return Some(item); @@ -799,13 +814,20 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let table = tables.get(archetype.table_id()).debug_checked_unwrap(); // SAFETY: `archetype` and `tables` are from the world that `fetch/filter` were created for, // `fetch_state`/`filter_state` are the states that `fetch/filter` were initialized with - D::set_archetype(&mut self.fetch, &query_state.fetch_state, archetype, table); - F::set_archetype( - &mut self.filter, - &query_state.filter_state, - archetype, - table, - ); + unsafe { + D::set_archetype( + &mut self.fetch, + &query_state.fetch_state, + archetype, + table, + ); + F::set_archetype( + &mut self.filter, + &query_state.filter_state, + archetype, + table, + ); + } self.archetype_entities = archetype.entities(); self.current_len = archetype.len(); self.current_row = 0; @@ -814,7 +836,8 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // SAFETY: set_archetype was called prior. // `current_row` is an archetype index row in range of the current archetype, because if it was not, then the if above would have been executed. - let archetype_entity = self.archetype_entities.get_unchecked(self.current_row); + let archetype_entity = + unsafe { self.archetype_entities.get_unchecked(self.current_row) }; if !F::filter_fetch( &mut self.filter, archetype_entity.id(), @@ -829,11 +852,13 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - `current_row` must be an archetype index row in range of the current archetype, // because if it was not, then the if above would have been executed. // - fetch is only called once for each `archetype_entity`. - let item = D::fetch( - &mut self.fetch, - archetype_entity.id(), - archetype_entity.table_row(), - ); + let item = unsafe { + D::fetch( + &mut self.fetch, + archetype_entity.id(), + archetype_entity.table_row(), + ) + }; self.current_row += 1; return Some(item); } diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index f32ba11b7686f..e120d124961fb 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -664,16 +664,16 @@ impl QueryState { let mut values = [(); N].map(|_| MaybeUninit::uninit()); for (value, entity) in std::iter::zip(&mut values, entities) { - // SAFETY: fetch is read-only - // and world must be validated - let item = self - .as_readonly() - .get_unchecked_manual(world, entity, last_run, this_run)?; + // SAFETY: fetch is read-only and world must be validated + let item = unsafe { + self.as_readonly() + .get_unchecked_manual(world, entity, last_run, this_run)? + }; *value = MaybeUninit::new(item); } // SAFETY: Each value has been fully initialized. - Ok(values.map(|x| x.assume_init())) + Ok(values.map(|x| unsafe { x.assume_init() })) } /// Gets the query results for the given [`World`] and array of [`Entity`], where the last change and @@ -1134,7 +1134,7 @@ impl QueryState { bevy_tasks::ComputeTaskPool::get().scope(|scope| { if D::IS_DENSE && F::IS_DENSE { // SAFETY: We only access table data that has been registered in `self.archetype_component_access`. - let tables = &world.storages().tables; + let tables = unsafe { &world.storages().tables }; for table_id in &self.matched_table_ids { let table = &tables[*table_id]; if table.is_empty() { diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 18d52abc8cca9..9d0d0d829ef4b 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -164,7 +164,8 @@ macro_rules! impl_tuple_world_query { #[allow(clippy::unused_unit)] unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; - ($($name::init_fetch(_world, $name, _last_run, _this_run),)*) + // SAFETY: The invariants are uphold by the caller. + ($(unsafe { $name::init_fetch(_world, $name, _last_run, _this_run) },)*) } const IS_DENSE: bool = true $(&& $name::IS_DENSE)*; @@ -178,14 +179,16 @@ macro_rules! impl_tuple_world_query { ) { let ($($name,)*) = _fetch; let ($($state,)*) = _state; - $($name::set_archetype($name, $state, _archetype, _table);)* + // SAFETY: The invariants are uphold by the caller. + $(unsafe { $name::set_archetype($name, $state, _archetype, _table); })* } #[inline] unsafe fn set_table<'w>(_fetch: &mut Self::Fetch<'w>, _state: &Self::State, _table: &'w Table) { let ($($name,)*) = _fetch; let ($($state,)*) = _state; - $($name::set_table($name, $state, _table);)* + // SAFETY: The invariants are uphold by the caller. + $(unsafe { $name::set_table($name, $state, _table); })* } #[inline(always)] @@ -196,7 +199,8 @@ macro_rules! impl_tuple_world_query { _table_row: TableRow ) -> Self::Item<'w> { let ($($name,)*) = _fetch; - ($($name::fetch($name, _entity, _table_row),)*) + // SAFETY: The invariants are uphold by the caller. + ($(unsafe { $name::fetch($name, _entity, _table_row) },)*) } fn update_component_access(state: &Self::State, _access: &mut FilteredAccess) { diff --git a/crates/bevy_ecs/src/reflect/component.rs b/crates/bevy_ecs/src/reflect/component.rs index 2a17c8aff0634..9788123a4a8fe 100644 --- a/crates/bevy_ecs/src/reflect/component.rs +++ b/crates/bevy_ecs/src/reflect/component.rs @@ -194,7 +194,7 @@ impl ReflectComponent { entity: UnsafeEntityCell<'a>, ) -> Option> { // SAFETY: safety requirements deferred to caller - (self.0.reflect_unchecked_mut)(entity) + unsafe { (self.0.reflect_unchecked_mut)(entity) } } /// Gets the value of this [`Component`] type from entity from `source_world` and [applies](Self::apply()) it to the value of this [`Component`] type in entity in `destination_world`. diff --git a/crates/bevy_ecs/src/reflect/resource.rs b/crates/bevy_ecs/src/reflect/resource.rs index 924eb54b09c8e..a8804fbd2c98d 100644 --- a/crates/bevy_ecs/src/reflect/resource.rs +++ b/crates/bevy_ecs/src/reflect/resource.rs @@ -116,7 +116,7 @@ impl ReflectResource { world: UnsafeWorldCell<'w>, ) -> Option> { // SAFETY: caller promises to uphold uniqueness guarantees - (self.0.reflect_unchecked_mut)(world) + unsafe { (self.0.reflect_unchecked_mut)(world) } } /// Gets the value of this [`Resource`] type from `source_world` and [applies](Self::apply()) it to the value of this [`Resource`] type in `destination_world`. diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index c17c1dbd73a06..1e00c3893053e 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -362,7 +362,7 @@ impl MultiThreadedExecutor { // SAFETY: `can_run` returned true, which means that: // - It must have called `update_archetype_component_access` for each run condition. // - There can be no systems running whose accesses would conflict with any conditions. - if !self.should_run(system_index, system, conditions, world_cell) { + if unsafe { !self.should_run(system_index, system, conditions, world_cell) } { self.skip_system_and_signal_dependents(system_index); continue; } @@ -479,8 +479,9 @@ impl MultiThreadedExecutor { // - The caller ensures that `world` has permission to read any data // required by the conditions. // - `update_archetype_component_access` has been called for each run condition. - let set_conditions_met = - evaluate_and_fold_conditions(&mut conditions.set_conditions[set_idx], world); + let set_conditions_met = unsafe { + evaluate_and_fold_conditions(&mut conditions.set_conditions[set_idx], world) + }; if !set_conditions_met { self.skipped_systems @@ -496,8 +497,9 @@ impl MultiThreadedExecutor { // - The caller ensures that `world` has permission to read any data // required by the conditions. // - `update_archetype_component_access` has been called for each run condition. - let system_conditions_met = - evaluate_and_fold_conditions(&mut conditions.system_conditions[system_index], world); + let system_conditions_met = unsafe { + evaluate_and_fold_conditions(&mut conditions.system_conditions[system_index], world) + }; if !system_conditions_met { self.skipped_systems.insert(system_index); diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 8636ac90383c0..7ff217d4adae2 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -208,7 +208,7 @@ impl BlobVec { // Pointer to the value in the vector that will get replaced. // SAFETY: The caller ensures that `index` fits in this vector. - let destination = NonNull::from(self.get_unchecked_mut(index)); + let destination = NonNull::from(unsafe { self.get_unchecked_mut(index) }); let source = value.as_ptr(); if let Some(drop) = self.drop { @@ -226,7 +226,7 @@ impl BlobVec { // that the element will not get observed or double dropped later. // - If a panic occurs, `self.len` will remain `0`, which ensures a double-drop // does not occur. Instead, all elements will be forgotten. - let old_value = OwningPtr::new(destination); + let old_value = unsafe { OwningPtr::new(destination) }; // This closure will run in case `drop()` panics, // which ensures that `value` does not get forgotten. @@ -249,7 +249,13 @@ impl BlobVec { // so it must still be initialized and it is safe to transfer ownership into the vector. // - `source` and `destination` were obtained from different memory locations, // both of which we have exclusive access to, so they are guaranteed not to overlap. - std::ptr::copy_nonoverlapping::(source, destination.as_ptr(), self.item_layout.size()); + unsafe { + std::ptr::copy_nonoverlapping::( + source, + destination.as_ptr(), + self.item_layout.size(), + ); + } } /// Appends an element to the back of the vector. @@ -304,7 +310,8 @@ impl BlobVec { // - `new_len` is less than the old len, so it must fit in this vector's allocation. // - `size` is a multiple of the erased type's alignment, // so adding a multiple of `size` will preserve alignment. - self.get_ptr_mut().byte_add(new_len * size).promote() + let p = unsafe { self.get_ptr_mut().byte_add(new_len * size) }; + p.promote() } /// Removes the value at `index` and copies the value stored into `ptr`. @@ -358,7 +365,7 @@ impl BlobVec { // so this operation will not overflow the original allocation. // - `size` is a multiple of the erased type's alignment, // so adding a multiple of `size` will preserve alignment. - self.get_ptr().byte_add(index * size) + unsafe { self.get_ptr().byte_add(index * size) } } /// Returns a mutable reference to the element at `index`, without doing bounds checking. @@ -374,7 +381,7 @@ impl BlobVec { // so this operation will not overflow the original allocation. // - `size` is a multiple of the erased type's alignment, // so adding a multiple of `size` will preserve alignment. - self.get_ptr_mut().byte_add(index * size) + unsafe { self.get_ptr_mut().byte_add(index * size) } } /// Gets a [`Ptr`] to the start of the vec @@ -397,7 +404,7 @@ impl BlobVec { /// The type `T` must be the type of the items in this [`BlobVec`]. pub unsafe fn get_slice(&self) -> &[UnsafeCell] { // SAFETY: the inner data will remain valid for as long as 'self. - std::slice::from_raw_parts(self.data.as_ptr() as *const UnsafeCell, self.len) + unsafe { std::slice::from_raw_parts(self.data.as_ptr() as *const UnsafeCell, self.len) } } /// Clears the vector, removing (and dropping) all values. @@ -502,9 +509,11 @@ mod tests { use super::BlobVec; use std::{alloc::Layout, cell::RefCell, mem, rc::Rc}; - // SAFETY: The pointer points to a valid value of type `T` and it is safe to drop this value. unsafe fn drop_ptr(x: OwningPtr<'_>) { - x.drop_as::(); + // SAFETY: The pointer points to a valid value of type `T` and it is safe to drop this value. + unsafe { + x.drop_as::(); + } } /// # Safety diff --git a/crates/bevy_ecs/src/storage/resource.rs b/crates/bevy_ecs/src/storage/resource.rs index 5eeebb1f1d04c..d83e96497a145 100644 --- a/crates/bevy_ecs/src/storage/resource.rs +++ b/crates/bevy_ecs/src/storage/resource.rs @@ -154,7 +154,9 @@ impl ResourceData { // SAFETY: The caller ensures that the provided value is valid for the underlying type and // is properly initialized. We've ensured that a value is already present and previously // initialized. - self.data.replace_unchecked(Self::ROW, value); + unsafe { + self.data.replace_unchecked(Self::ROW, value); + } } else { if !SEND { self.origin_thread_id = Some(std::thread::current().id()); @@ -185,7 +187,9 @@ impl ResourceData { // SAFETY: The caller ensures that the provided value is valid for the underlying type and // is properly initialized. We've ensured that a value is already present and previously // initialized. - self.data.replace_unchecked(Self::ROW, value); + unsafe { + self.data.replace_unchecked(Self::ROW, value); + } } else { if !SEND { self.origin_thread_id = Some(std::thread::current().id()); diff --git a/crates/bevy_ecs/src/system/adapter_system.rs b/crates/bevy_ecs/src/system/adapter_system.rs index 24b32b899996f..c44e19c24d709 100644 --- a/crates/bevy_ecs/src/system/adapter_system.rs +++ b/crates/bevy_ecs/src/system/adapter_system.rs @@ -107,8 +107,9 @@ where #[inline] unsafe fn run_unsafe(&mut self, input: Self::In, world: UnsafeWorldCell) -> Self::Out { // SAFETY: `system.run_unsafe` has the same invariants as `self.run_unsafe`. - self.func - .adapt(input, |input| self.system.run_unsafe(input, world)) + self.func.adapt(input, |input| unsafe { + self.system.run_unsafe(input, world) + }) } #[inline] diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index 4594eae39b639..8184cf76226fb 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -171,8 +171,9 @@ where // in parallel, so their world accesses will not conflict with each other. // Additionally, `update_archetype_component_access` has been called, // which forwards to the implementations for `self.a` and `self.b`. - |input| self.a.run_unsafe(input, world), - |input| self.b.run_unsafe(input, world), + |input| unsafe { self.a.run_unsafe(input, world) }, + // SAFETY: See the comment above. + |input| unsafe { self.b.run_unsafe(input, world) }, ) } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 903c512a4937b..1ad187904ac7b 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -343,7 +343,8 @@ impl SystemState { world: UnsafeWorldCell<'w>, ) -> SystemParamItem<'w, 's, Param> { let change_tick = world.increment_change_tick(); - self.fetch(world, change_tick) + // SAFETY: The invariants are uphold by the caller. + unsafe { self.fetch(world, change_tick) } } /// # Safety @@ -356,7 +357,9 @@ impl SystemState { world: UnsafeWorldCell<'w>, change_tick: Tick, ) -> SystemParamItem<'w, 's, Param> { - let param = Param::get_param(&mut self.param_state, &self.meta, world, change_tick); + // SAFETY: The invariants are uphold by the caller. + let param = + unsafe { Param::get_param(&mut self.param_state, &self.meta, world, change_tick) }; self.meta.last_run = change_tick; param } @@ -490,12 +493,14 @@ where // if the world does not match. // - All world accesses used by `F::Param` have been registered, so the caller // will ensure that there are no data access conflicts. - let params = F::Param::get_param( - self.param_state.as_mut().expect(Self::PARAM_MESSAGE), - &self.system_meta, - world, - change_tick, - ); + let params = unsafe { + F::Param::get_param( + self.param_state.as_mut().expect(Self::PARAM_MESSAGE), + &self.system_meta, + world, + change_tick, + ) + }; let out = self.func.run(input, params); self.system_meta.last_run = change_tick; out diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 7689c459c06b1..232ac3cd0feb0 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -661,8 +661,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { // SAFETY: // - `self.world` has permission to access the required components. // - The caller ensures that this operation will not result in any aliased mutable accesses. - self.state - .iter_unchecked_manual(self.world, self.last_run, self.this_run) + unsafe { + self.state + .iter_unchecked_manual(self.world, self.last_run, self.this_run) + } } /// Iterates over all possible combinations of `K` query items without repetition. @@ -682,8 +684,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { // SAFETY: // - `self.world` has permission to access the required components. // - The caller ensures that this operation will not result in any aliased mutable accesses. - self.state - .iter_combinations_unchecked_manual(self.world, self.last_run, self.this_run) + unsafe { + self.state + .iter_combinations_unchecked_manual(self.world, self.last_run, self.this_run) + } } /// Returns an [`Iterator`] over the query items generated from an [`Entity`] list. @@ -707,8 +711,14 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { // SAFETY: // - `self.world` has permission to access the required components. // - The caller ensures that this operation will not result in any aliased mutable accesses. - self.state - .iter_many_unchecked_manual(entities, self.world, self.last_run, self.this_run) + unsafe { + self.state.iter_many_unchecked_manual( + entities, + self.world, + self.last_run, + self.this_run, + ) + } } /// Returns a parallel iterator over the query results for the given [`World`]. @@ -1014,8 +1024,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { pub unsafe fn get_unchecked(&self, entity: Entity) -> Result, QueryEntityError> { // SEMI-SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict - self.state - .get_unchecked_manual(self.world, entity, self.last_run, self.this_run) + unsafe { + self.state + .get_unchecked_manual(self.world, entity, self.last_run, self.this_run) + } } /// Returns a single read-only query item when there is exactly one entity matching the query. diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 68415fd6eb08b..a058c37428fcd 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -228,7 +228,7 @@ unsafe impl SystemParam for Qu // SAFETY: We have registered all of the query's world accesses, // so the caller ensures that `world` has permission to access any // world data that the query needs. - Query::new(world, state, system_meta.last_run, change_tick) + unsafe { Query::new(world, state, system_meta.last_run, change_tick) } } } @@ -654,7 +654,7 @@ unsafe impl SystemParam for &'_ World { _change_tick: Tick, ) -> Self::Item<'w, 's> { // SAFETY: Read-only access to the entire world was registered in `init_state`. - world.world() + unsafe { world.world() } } } @@ -1500,7 +1500,7 @@ unsafe impl SystemParam for StaticSystemParam<'_, '_, change_tick: Tick, ) -> Self::Item<'world, 'state> { // SAFETY: Defer to the safety of P::SystemParam - StaticSystemParam(P::get_param(state, system_meta, world, change_tick)) + StaticSystemParam(unsafe { P::get_param(state, system_meta, world, change_tick) }) } } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index d0bb9c270d852..bc12b3343c93b 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -828,7 +828,9 @@ impl<'w> EntityWorldMut<'w> { Some(result) } - /// Safety: `new_archetype_id` must have the same or a subset of the components + /// # Safety + /// + /// `new_archetype_id` must have the same or a subset of the components /// in `old_archetype_id`. Probably more safety stuff too, audit a call to /// this fn as if the code here was written inline /// @@ -874,15 +876,16 @@ impl<'w> EntityWorldMut<'w> { .tables .get_2_mut(old_table_id, new_archetype.table_id()); - // SAFETY: old_table_row exists let move_result = if DROP { - old_table.move_to_and_drop_missing_unchecked(old_table_row, new_table) + // SAFETY: old_table_row exists + unsafe { old_table.move_to_and_drop_missing_unchecked(old_table_row, new_table) } } else { - old_table.move_to_and_forget_missing_unchecked(old_table_row, new_table) + // SAFETY: old_table_row exists + unsafe { old_table.move_to_and_forget_missing_unchecked(old_table_row, new_table) } }; // SAFETY: move_result.new_row is a valid position in new_archetype's table - let new_location = new_archetype.allocate(entity, move_result.new_row); + let new_location = unsafe { new_archetype.allocate(entity, move_result.new_row) }; // if an entity was moved into this entity's table row, update its table row if let Some(swapped_entity) = move_result.swapped_entity { @@ -906,7 +909,9 @@ impl<'w> EntityWorldMut<'w> { *self_location = new_location; // SAFETY: The entity is valid and has been moved to the new location already. - entities.set(entity.index(), new_location); + unsafe { + entities.set(entity.index(), new_location); + } } /// Remove the components of `bundle_info` from `entity`, where `self_location` and `old_location` @@ -927,14 +932,16 @@ impl<'w> EntityWorldMut<'w> { ) { // SAFETY: `archetype_id` exists because it is referenced in `old_location` which is valid // and components in `bundle_info` must exist due to this functions safety invariants. - let new_archetype_id = remove_bundle_from_archetype( - archetypes, - storages, - components, - old_location.archetype_id, - bundle_info, - true, - ) + let new_archetype_id = unsafe { + remove_bundle_from_archetype( + archetypes, + storages, + components, + old_location.archetype_id, + bundle_info, + true, + ) + } .expect("intersections should always return a result"); if new_archetype_id == old_location.archetype_id { @@ -960,16 +967,18 @@ impl<'w> EntityWorldMut<'w> { // SAFETY: `new_archetype_id` is a subset of the components in `old_location.archetype_id` // because it is created by removing a bundle from these components. - Self::move_entity_from_remove::( - entity, - self_location, - old_location.archetype_id, - old_location, - entities, - archetypes, - storages, - new_archetype_id, - ); + unsafe { + Self::move_entity_from_remove::( + entity, + self_location, + old_location.archetype_id, + old_location, + entities, + archetypes, + storages, + new_archetype_id, + ); + } } /// Removes any components in the [`Bundle`] from the entity. @@ -2135,7 +2144,7 @@ unsafe fn remove_bundle_from_archetype( for component_id in bundle_info.components().iter().cloned() { if current_archetype.contains(component_id) { // SAFETY: bundle components were already initialized by bundles.get_info - let component_info = components.get_info_unchecked(component_id); + let component_info = unsafe { components.get_info_unchecked(component_id) }; match component_info.storage_type() { StorageType::Table => removed_table_components.push(component_id), StorageType::SparseSet => removed_sparse_set_components.push(component_id), @@ -2167,9 +2176,11 @@ unsafe fn remove_bundle_from_archetype( current_archetype.table_id() } else { // SAFETY: all components in next_table_components exist - storages - .tables - .get_id_or_insert(&next_table_components, components) + unsafe { + storages + .tables + .get_id_or_insert(&next_table_components, components) + } }; } @@ -2230,7 +2241,7 @@ pub(crate) unsafe fn take_component<'a>( location: EntityLocation, ) -> OwningPtr<'a> { // SAFETY: caller promises component_id to be valid - let component_info = components.get_info_unchecked(component_id); + let component_info = unsafe { components.get_info_unchecked(component_id) }; removed_components.send(component_id, entity); match component_info.storage_type() { StorageType::Table => { @@ -2240,9 +2251,11 @@ pub(crate) unsafe fn take_component<'a>( // - archetypes only store valid table_rows // - index is in bounds as promised by caller // - promote is safe because the caller promises to remove the table row without dropping it immediately afterwards - components - .get_data_unchecked_mut(location.table_row) - .promote() + unsafe { + components + .get_data_unchecked_mut(location.table_row) + .promote() + } } StorageType::SparseSet => storages .sparse_sets diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index e4c45d2b69081..4aebf6bc508f8 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -773,9 +773,11 @@ impl World { let table_row = self.storages.tables[archetype.table_id()].allocate(entity); // SAFETY: no components are allocated by archetype.allocate() because the archetype is // empty - let location = archetype.allocate(entity, table_row); + let location = unsafe { archetype.allocate(entity, table_row) }; // SAFETY: entity index was just allocated - self.entities.set(entity.index(), location); + unsafe { + self.entities.set(entity.index(), location); + } EntityWorldMut::new(self, entity, location) } @@ -1743,9 +1745,11 @@ impl World { ) { let change_tick = self.change_tick(); - // SAFETY: value is valid for component_id, ensured by caller - self.initialize_resource_internal(component_id) - .insert(value, change_tick); + let resource = self.initialize_resource_internal(component_id); + // SAFETY: `value` is valid for `component_id`, ensured by caller + unsafe { + resource.insert(value, change_tick); + } } /// Inserts a new `!Send` resource with the given `value`. Will replace the value if it already @@ -1768,9 +1772,11 @@ impl World { ) { let change_tick = self.change_tick(); - // SAFETY: value is valid for component_id, ensured by caller - self.initialize_non_send_internal(component_id) - .insert(value, change_tick); + let resource = self.initialize_non_send_internal(component_id); + // SAFETY: `value` is valid for `component_id`, ensured by caller + unsafe { + resource.insert(value, change_tick); + } } /// # Panics