diff --git a/clippy.toml b/clippy.toml index 46c61a895f..d1d234817a 100644 --- a/clippy.toml +++ b/clippy.toml @@ -11,6 +11,8 @@ doc-valid-idents = [ "..", ] +check-private-items = true + disallowed-methods = [ { path = "f32::powi", reason = "use bevy_math::ops::FloatPow::squared, bevy_math::ops::FloatPow::cubed, or bevy_math::ops::powf instead for libm determinism" }, { path = "f32::log", reason = "use bevy_math::ops::ln, bevy_math::ops::log2, or bevy_math::ops::log10 instead for libm determinism" }, diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 1f8cf10ba0..607e14145a 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -1279,7 +1279,7 @@ impl<'w> BundleSpawner<'w> { unsafe { &mut self.world.world_mut().entities } } - /// # Safety: + /// # Safety /// - `Self` must be dropped after running this function as it may invalidate internal pointers. #[inline] pub(crate) unsafe fn flush_commands(&mut self) { @@ -1344,11 +1344,13 @@ impl Bundles { } /// # Safety - /// A `BundleInfo` with the given `BundleId` must have been initialized for this instance of `Bundles`. + /// A [`BundleInfo`] with the given [`BundleId`] must have been initialized for this instance of `Bundles`. pub(crate) unsafe fn get_unchecked(&self, id: BundleId) -> &BundleInfo { self.bundle_infos.get_unchecked(id.0) } + /// # Safety + /// This [`BundleId`] must have been initialized with a single [`Component`] (via [`init_component_info`](Self::init_dynamic_info)) pub(crate) unsafe fn get_storage_unchecked(&self, id: BundleId) -> StorageType { *self .dynamic_component_storages @@ -1356,6 +1358,8 @@ impl Bundles { .debug_checked_unwrap() } + /// # Safety + /// This [`BundleId`] must have been initialized with multiple [`Component`]s (via [`init_dynamic_info`](Self::init_dynamic_info)) pub(crate) unsafe fn get_storages_unchecked(&mut self, id: BundleId) -> &mut Vec { self.dynamic_bundle_storages .get_mut(&id) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index b7d248b7ea..910bd9fc10 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -738,7 +738,7 @@ impl Debug for ComponentDescriptor { } impl ComponentDescriptor { - /// # SAFETY + /// # Safety /// /// `x` must point to a valid value of type `T`. unsafe fn drop_ptr(x: OwningPtr<'_>) { diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index c3faa4d50e..6522a1b237 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -1290,7 +1290,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator>> } } - /// Safety: + /// # Safety /// All arguments must stem from the same valid `QueryManyIter`. /// /// The lifetime here is not restrictive enough for Fetch with &mut access, @@ -1578,7 +1578,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, const K: usize> QueryCombinationIter< } } - /// Safety: + /// # Safety /// The lifetime here is not restrictive enough for Fetch with &mut access, /// as calling `fetch_next_aliased_unchecked` multiple times can produce multiple /// references to the same component, leading to unique reference aliasing. @@ -1733,6 +1733,9 @@ impl Clone for QueryIterationCursor<'_, '_, D, F> } impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { + /// # Safety + /// - `world` must have permission to access any of the components registered in `query_state`. + /// - `world` must be the same one used to initialize `query_state`. unsafe fn init_empty( world: UnsafeWorldCell<'w>, query_state: &'s QueryState, @@ -1781,25 +1784,41 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { } } - /// retrieve item returned from most recent `next` call again. + /// Retrieve item returned from most recent `next` call again. + /// + /// # Safety + /// The result of `next` and any previous calls to `peek_last` with this row must have been + /// dropped to prevent aliasing mutable references. #[inline] unsafe fn peek_last(&mut self) -> Option> { if self.current_row > 0 { let index = self.current_row - 1; if self.is_dense { - let entity = self.table_entities.get_unchecked(index); - Some(D::fetch( - &mut self.fetch, - *entity, - TableRow::from_usize(index), - )) + // SAFETY: This must have been called previously in `next` as `current_row > 0` + let entity = unsafe { self.table_entities.get_unchecked(index) }; + // SAFETY: + // - `set_table` must have been called previously either in `next` or before it. + // - `*entity` and `index` are in the current table. + unsafe { + Some(D::fetch( + &mut self.fetch, + *entity, + TableRow::from_usize(index), + )) + } } else { - let archetype_entity = self.archetype_entities.get_unchecked(index); - Some(D::fetch( - &mut self.fetch, - archetype_entity.id(), - archetype_entity.table_row(), - )) + // SAFETY: This must have been called previously in `next` as `current_row > 0` + let archetype_entity = unsafe { self.archetype_entities.get_unchecked(index) }; + // SAFETY: + // - `set_archetype` must have been called previously either in `next` or before it. + // - `archetype_entity.id()` and `archetype_entity.table_row()` are in the current archetype. + unsafe { + Some(D::fetch( + &mut self.fetch, + archetype_entity.id(), + archetype_entity.table_row(), + )) + } } } else { None diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index 61cd622d5a..6ac740255c 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -123,7 +123,7 @@ impl QueryState { /// /// Consider using `as_readonly` or `as_nop` instead which are safe functions. /// - /// # SAFETY + /// # Safety /// /// `NewD` must have a subset of the access that `D` does and match the exact same archetypes/tables /// `NewF` must have a subset of the access that `F` does and match the exact same archetypes/tables diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 04e1dc416b..9b695aa4b2 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -507,8 +507,12 @@ mod tests { use super::BlobVec; use std::{alloc::Layout, cell::RefCell, mem::align_of, rc::Rc}; + /// # Safety + /// + /// The pointer `x` must point to a valid value of type `T` and it must be safe to drop this value. unsafe fn drop_ptr(x: OwningPtr<'_>) { - // SAFETY: The pointer points to a valid value of type `T` and it is safe to drop this value. + // SAFETY: It is guaranteed by the caller that `x` points to a + // valid value of type `T` and it is safe to drop this value. unsafe { x.drop_as::(); } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index ac038dff1a..48b9910d2b 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1923,7 +1923,7 @@ pub struct DynSystemParam<'w, 's> { } impl<'w, 's> DynSystemParam<'w, 's> { - /// # SAFETY + /// # Safety /// - `state` must be a `ParamState` for some inner `T: SystemParam`. /// - The passed [`UnsafeWorldCell`] must have access to any world data registered /// in [`init_state`](SystemParam::init_state) for the inner system param. @@ -1998,7 +1998,7 @@ impl<'w, 's> DynSystemParam<'w, 's> { } } -/// # SAFETY +/// # Safety /// - `state` must be a `ParamState` for some inner `T: SystemParam`. /// - The passed [`UnsafeWorldCell`] must have access to any world data registered /// in [`init_state`](SystemParam::init_state) for the inner system param. diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index e8b2c6f563..91d021b1fa 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -1130,7 +1130,7 @@ impl<'w> EntityWorldMut<'w> { /// Remove the components of `bundle` from `entity`. /// - /// SAFETY: + /// # Safety /// - A `BundleInfo` with the corresponding `BundleId` must have been initialized. #[allow(clippy::too_many_arguments)] unsafe fn remove_bundle(&mut self, bundle: BundleId) -> EntityLocation { @@ -1519,7 +1519,8 @@ impl<'w> EntityWorldMut<'w> { } } -/// SAFETY: all components in the archetype must exist in world +/// # Safety +/// All components in the archetype must exist in world unsafe fn trigger_on_replace_and_on_remove_hooks_and_observers( deferred_world: &mut DeferredWorld, archetype: &Archetype, @@ -2387,6 +2388,8 @@ impl<'w, B> EntityRefExcept<'w, B> where B: Bundle, { + /// # Safety + /// Other users of `UnsafeEntityCell` must only have mutable access to the components in `B`. pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>) -> Self { Self { entity, @@ -2466,6 +2469,8 @@ impl<'w, B> EntityMutExcept<'w, B> where B: Bundle, { + /// # Safety + /// Other users of `UnsafeEntityCell` must not have access to any components not in `B`. pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>) -> Self { Self { entity, diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 56a4057d3b..f1af0c11c2 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -248,7 +248,7 @@ impl<'w> UnsafeWorldCell<'w> { } /// Retrieves this world's [`Observers`] collection. - pub(crate) unsafe fn observers(self) -> &'w Observers { + pub(crate) fn observers(self) -> &'w Observers { // SAFETY: // - we only access world metadata &unsafe { self.world_metadata() }.observers @@ -1002,7 +1002,7 @@ impl<'w> UnsafeEntityCell<'w> { impl<'w> UnsafeWorldCell<'w> { #[inline] - /// # Safety: + /// # Safety /// - the returned `Table` is only used in ways that this [`UnsafeWorldCell`] has permission for. /// - the returned `Table` is only used in ways that would not conflict with any existing borrows of world data. unsafe fn fetch_table(self, location: EntityLocation) -> Option<&'w Table> { @@ -1013,7 +1013,7 @@ impl<'w> UnsafeWorldCell<'w> { } #[inline] - /// # Safety: + /// # Safety /// - the returned `ComponentSparseSet` is only used in ways that this [`UnsafeWorldCell`] has permission for. /// - the returned `ComponentSparseSet` is only used in ways that would not conflict with any existing /// borrows of world data. diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 3393d0ef44..ef1039920c 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -447,7 +447,8 @@ fn extract(main_world: &mut World, render_world: &mut World) { main_world.insert_resource(ScratchMainWorld(scratch_world)); } -/// SAFETY: this function must be called from the main thread. +/// # Safety +/// This function must be called from the main thread. unsafe fn initialize_render_app(app: &mut App) { app.init_resource::(); diff --git a/crates/bevy_utils/src/futures.rs b/crates/bevy_utils/src/futures.rs index ab425cee3a..acf19797ea 100644 --- a/crates/bevy_utils/src/futures.rs +++ b/crates/bevy_utils/src/futures.rs @@ -36,10 +36,10 @@ pub fn check_ready(future: &mut F) -> Option { } } -unsafe fn noop_clone(_data: *const ()) -> RawWaker { +fn noop_clone(_data: *const ()) -> RawWaker { noop_raw_waker() } -unsafe fn noop(_data: *const ()) {} +fn noop(_data: *const ()) {} const NOOP_WAKER_VTABLE: RawWakerVTable = RawWakerVTable::new(noop_clone, noop, noop, noop);