Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable clippy::check-private-items so that missing_safety_doc will apply to private functions as well #15161

Merged
merged 9 commits into from
Sep 18, 2024
2 changes: 2 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand Down
8 changes: 6 additions & 2 deletions crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -1344,18 +1344,22 @@ 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
.get(&id)
.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<StorageType> {
self.dynamic_bundle_storages
.get_mut(&id)
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(x: OwningPtr<'_>) {
Expand Down
49 changes: 34 additions & 15 deletions crates/bevy_ecs/src/query/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1290,7 +1290,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator<Item: Borrow<Entity>>>
}
}

/// Safety:
/// # Safety
/// All arguments must stem from the same valid `QueryManyIter`.
///
/// The lifetime here is not restrictive enough for Fetch with &mut access,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -1733,6 +1733,9 @@ impl<D: QueryData, F: QueryFilter> 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<D, F>,
Expand Down Expand Up @@ -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<D::Item<'w>> {
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
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
///
/// 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
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_ecs/src/storage/blob_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(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::<T>();
}
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/system/system_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1923,7 +1923,7 @@ pub struct DynSystemParam<'w, 's> {
}

impl<'w, 's> DynSystemParam<'w, 's> {
/// # SAFETY
/// # Safety
/// - `state` must be a `ParamState<T>` 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.
Expand Down Expand Up @@ -1998,7 +1998,7 @@ impl<'w, 's> DynSystemParam<'w, 's> {
}
}

/// # SAFETY
/// # Safety
/// - `state` must be a `ParamState<T>` 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.
Expand Down
9 changes: 7 additions & 2 deletions crates/bevy_ecs/src/world/entity_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/world/unsafe_world_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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> {
Expand All @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<ScratchMainWorld>();

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_utils/src/futures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ pub fn check_ready<F: Future + Unpin>(future: &mut F) -> Option<F::Output> {
}
}

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);

Expand Down