diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 3273faf9ab..5722872df1 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -144,7 +144,7 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream { let struct_name = &ast.ident; TokenStream::from(quote! { - /// SAFE: ComponentId is returned in field-definition-order. [from_components] and [get_components] use field-definition-order + /// SAFETY: ComponentId is returned in field-definition-order. [from_components] and [get_components] use field-definition-order unsafe impl #impl_generics #ecs_path::bundle::Bundle for #struct_name #ty_generics #where_clause { fn component_ids( components: &mut #ecs_path::component::Components, @@ -192,7 +192,7 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { let index = Index::from(i); param_fn_muts.push(quote! { pub fn #fn_name<'a>(&'a mut self) -> <#param::Fetch as SystemParamFetch<'a, 'a>>::Item { - // SAFE: systems run without conflicts with other systems. + // SAFETY: systems run without conflicts with other systems. // Conflicting params in ParamSet are not accessible at the same time // ParamSets are guaranteed to not conflict with other SystemParams unsafe { @@ -213,13 +213,13 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { type Fetch = ParamSetState<(#(#param::Fetch,)*)>; } - // SAFE: All parameters are constrained to ReadOnlyFetch, so World is only read + // SAFETY: All parameters are constrained to ReadOnlyFetch, so World is only read unsafe impl<#(#param_fetch: for<'w1, 's1> SystemParamFetch<'w1, 's1>,)*> ReadOnlySystemParamFetch for ParamSetState<(#(#param_fetch,)*)> where #(#param_fetch: ReadOnlySystemParamFetch,)* { } - // SAFE: Relevant parameter ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any ParamState conflicts + // SAFETY: Relevant parameter ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any ParamState conflicts // with any prior access, a panic will occur. unsafe impl<#(#param_fetch: for<'w1, 's1> SystemParamFetch<'w1, 's1>,)*> SystemParamState for ParamSetState<(#(#param_fetch,)*)> diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index 2c01f8c1bb..b5d6b30e3b 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -420,13 +420,13 @@ impl Archetypes { #[inline] pub fn empty(&self) -> &Archetype { - // SAFE: empty archetype always exists + // SAFETY: empty archetype always exists unsafe { self.archetypes.get_unchecked(ArchetypeId::EMPTY.index()) } } #[inline] pub(crate) fn empty_mut(&mut self) -> &mut Archetype { - // SAFE: empty archetype always exists + // SAFETY: empty archetype always exists unsafe { self.archetypes .get_unchecked_mut(ArchetypeId::EMPTY.index()) @@ -435,13 +435,13 @@ impl Archetypes { #[inline] pub fn resource(&self) -> &Archetype { - // SAFE: resource archetype always exists + // SAFETY: resource archetype always exists unsafe { self.archetypes.get_unchecked(ArchetypeId::RESOURCE.index()) } } #[inline] pub(crate) fn resource_mut(&mut self) -> &mut Archetype { - // SAFE: resource archetype always exists + // SAFETY: resource archetype always exists unsafe { self.archetypes .get_unchecked_mut(ArchetypeId::RESOURCE.index()) diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 6b81435fc8..8eb7730289 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -99,6 +99,10 @@ pub unsafe trait Bundle: Send + Sync + 'static { macro_rules! tuple_impl { ($($name: ident),*) => { + // SAFETY: + // - `Bundle::component_ids` returns the `ComponentId`s for each component type in the + // bundle, in the exact order that `Bundle::get_components` is called. + // - `Bundle::from_components` calls `func` exactly once for each `ComponentId` returned by `Bundle::component_ids`. unsafe impl<$($name: Component),*> Bundle for ($($name,)*) { #[allow(unused_variables)] fn component_ids(components: &mut Components, storages: &mut Storages) -> Vec { @@ -325,7 +329,7 @@ impl BundleInfo { bundle_status.push(ComponentStatus::Mutated); } else { bundle_status.push(ComponentStatus::Added); - // SAFE: component_id exists + // SAFETY: component_id exists let component_info = unsafe { components.get_info_unchecked(component_id) }; match component_info.storage_type() { StorageType::Table => new_table_components.push(component_id), @@ -354,7 +358,7 @@ impl BundleInfo { new_table_components.extend(current_archetype.table_components()); // sort to ignore order while hashing new_table_components.sort(); - // SAFE: all component ids in `new_table_components` exist + // SAFETY: all component ids in `new_table_components` exist table_id = unsafe { storages .tables @@ -492,7 +496,7 @@ impl<'a, 'b> BundleInserter<'a, 'b> { } else if new_archetype.id() == swapped_location.archetype_id { new_archetype } else { - // SAFE: the only two borrowed archetypes are above and we just did collision checks + // SAFETY: the only two borrowed archetypes are above and we just did collision checks &mut *self .archetypes_ptr .add(swapped_location.archetype_id.index()) @@ -567,7 +571,7 @@ impl<'a, 'b> BundleSpawner<'a, 'b> { #[inline] pub unsafe fn spawn(&mut self, bundle: T) -> Entity { let entity = self.entities.alloc(); - // SAFE: entity is allocated (but non-existent), `T` matches this BundleInfo's type + // SAFETY: entity is allocated (but non-existent), `T` matches this BundleInfo's type self.spawn_non_existent(entity, bundle); entity } @@ -599,14 +603,14 @@ impl Bundles { let id = self.bundle_ids.entry(TypeId::of::()).or_insert_with(|| { let component_ids = T::component_ids(components, storages); let id = BundleId(bundle_infos.len()); - // SAFE: T::component_id ensures info was created + // SAFETY: T::component_id ensures info was created let bundle_info = unsafe { initialize_bundle(std::any::type_name::(), component_ids, id, components) }; bundle_infos.push(bundle_info); id }); - // SAFE: index either exists, or was initialized + // SAFETY: index either exists, or was initialized unsafe { self.bundle_infos.get_unchecked(id.0) } } } @@ -623,7 +627,7 @@ unsafe fn initialize_bundle( let mut storage_types = Vec::new(); for &component_id in &component_ids { - // SAFE: component_id exists and is therefore valid + // SAFETY: component_id exists and is therefore valid let component_info = components.get_info_unchecked(component_id); storage_types.push(component_info.storage_type()); } diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index f4e2b61d14..eb379934ef 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -474,7 +474,7 @@ impl Components { #[inline] pub fn init_resource(&mut self) -> ComponentId { - // SAFE: The [`ComponentDescriptor`] matches the [`TypeId`] + // SAFETY: The [`ComponentDescriptor`] matches the [`TypeId`] unsafe { self.get_or_insert_resource_with(TypeId::of::(), || { ComponentDescriptor::new_resource::() @@ -484,7 +484,7 @@ impl Components { #[inline] pub fn init_non_send(&mut self) -> ComponentId { - // SAFE: The [`ComponentDescriptor`] matches the [`TypeId`] + // SAFETY: The [`ComponentDescriptor`] matches the [`TypeId`] unsafe { self.get_or_insert_resource_with(TypeId::of::(), || { ComponentDescriptor::new_non_send::(StorageType::default()) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index dcad2664f6..2c10a87eb3 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -591,6 +591,8 @@ impl Entities { // Flushes all reserved entities to an "invalid" state. Attempting to retrieve them will return None // unless they are later populated with a valid archetype. pub fn flush_as_invalid(&mut self) { + // SAFETY: as per `flush` safety docs, the archetype id can be set to [`ArchetypeId::INVALID`] if + // the [`Entity`] has not been assigned to an [`Archetype`][crate::archetype::Archetype], which is the case here unsafe { self.flush(|_entity, location| { location.archetype_id = ArchetypeId::INVALID; @@ -658,6 +660,7 @@ mod tests { fn reserve_entity_len() { let mut e = Entities::default(); e.reserve_entity(); + // SAFETY: entity_location is left invalid unsafe { e.flush(|_, _| {}) }; assert_eq!(e.len(), 1); } @@ -669,6 +672,7 @@ mod tests { assert!(entities.contains(e)); assert!(entities.get(e).is_none()); + // SAFETY: entity_location is left invalid unsafe { entities.flush(|_entity, _location| { // do nothing ... leaving entity location invalid diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 30ec5aa8e7..cafacc2a2d 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1,3 +1,4 @@ +#![warn(clippy::undocumented_unsafe_blocks)] #![doc = include_str!("../README.md")] #[cfg(target_pointer_width = "16")] diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index ad79241b10..919b7dd1ec 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -480,7 +480,7 @@ macro_rules! impl_query_filter_tuple { } } - // SAFE: filters are read only + // SAFETY: filters are read only unsafe impl<$($filter: ReadOnlyWorldQuery),*> ReadOnlyWorldQuery for Or<($($filter,)*)> {} }; } diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 6755026f3a..65881a6a93 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -52,6 +52,9 @@ where #[inline(always)] fn next(&mut self) -> Option { + // SAFETY: + // `tables` and `archetypes` belong to the same world that the cursor was initialized for. + // `query_state` is the state that was passed to `QueryIterationCursor::init`. unsafe { self.cursor .next(self.tables, self.archetypes, self.query_state) @@ -150,33 +153,42 @@ where #[inline(always)] fn next(&mut self) -> Option { - unsafe { - for entity in self.entity_iter.by_ref() { - let location = match self.entities.get(*entity.borrow()) { - Some(location) => location, - None => continue, - }; + for entity in self.entity_iter.by_ref() { + let location = match self.entities.get(*entity.borrow()) { + Some(location) => location, + None => continue, + }; - if !self - .query_state - .matched_archetypes - .contains(location.archetype_id.index()) - { - continue; - } + if !self + .query_state + .matched_archetypes + .contains(location.archetype_id.index()) + { + continue; + } - let archetype = &self.archetypes[location.archetype_id]; + let archetype = &self.archetypes[location.archetype_id]; + // 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 + unsafe { self.fetch .set_archetype(&self.query_state.fetch_state, archetype, self.tables); + } + // 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 + unsafe { self.filter .set_archetype(&self.query_state.filter_state, archetype, self.tables); - if self.filter.archetype_filter_fetch(location.index) { - return Some(self.fetch.archetype_fetch(location.index)); - } } - None + // SAFETY: set_archetype was called prior. + // `location.index` is an archetype index row in range of the current archetype, because if it was not, the match above would have `continue`d + if unsafe { self.filter.archetype_filter_fetch(location.index) } { + // SAFETY: set_archetype was called prior, `location.index` is an archetype index in range of the current archetype + return Some(unsafe { self.fetch.archetype_fetch(location.index) }); + } } + None } fn size_hint(&self) -> (usize, Option) { @@ -289,7 +301,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery, const K: usize> QueryCombinationIter< for<'a> QueryFetch<'a, Q>: Clone, for<'a> QueryFetch<'a, F>: Clone, { - // safety: we are limiting the returned reference to self, + // SAFETY: we are limiting the returned reference to self, // making sure this method cannot be called multiple times without getting rid // of any previously returned unique references first, thus preventing aliasing. unsafe { @@ -374,7 +386,9 @@ struct QueryIterationCursor<'w, 's, Q: WorldQuery, QF: Fetch<'w, State = Q::Stat archetype_id_iter: std::slice::Iter<'s, ArchetypeId>, fetch: QF, filter: QueryFetch<'w, F>, + // length of the table table or length of the archetype, depending on whether both `Q`'s and `F`'s fetches are dense current_len: usize, + // either table row or archetype index, depending on whether both `Q`'s and `F`'s fetches are dense current_index: usize, phantom: PhantomData<(&'w (), Q)>, } @@ -461,6 +475,10 @@ where // NOTE: If you are changing query iteration code, remember to update the following places, where relevant: // QueryIterationCursor, QueryState::for_each_unchecked_manual, QueryState::par_for_each_unchecked_manual + /// # Safety + /// `tables` and `archetypes` must belong to the same world that the [`QueryIterationCursor`] + /// was initialized for. + /// `query_state` must be the same [`QueryState`] that was passed to `init` or `init_empty`. #[inline(always)] unsafe fn next( &mut self, @@ -470,9 +488,12 @@ where ) -> Option { if Self::IS_DENSE { loop { + // we are on the beginning of the query, or finished processing a table, so skip to the next if self.current_index == self.current_len { let table_id = self.table_id_iter.next()?; let table = &tables[*table_id]; + // 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 self.fetch.set_table(&query_state.fetch_state, table); self.filter.set_table(&query_state.filter_state, table); self.current_len = table.len(); @@ -480,11 +501,15 @@ where continue; } + // SAFETY: set_table was called prior. + // `current_index` is a table row in range of the current table, because if it was not, then the if above would have been executed. if !self.filter.table_filter_fetch(self.current_index) { self.current_index += 1; continue; } + // SAFETY: set_table was called prior. + // `current_index` is a table row in range of the current table, because if it was not, then the if above would have been executed. let item = self.fetch.table_fetch(self.current_index); self.current_index += 1; @@ -495,6 +520,8 @@ where if self.current_index == self.current_len { let archetype_id = self.archetype_id_iter.next()?; let archetype = &archetypes[*archetype_id]; + // 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 self.fetch .set_archetype(&query_state.fetch_state, archetype, tables); self.filter @@ -504,11 +531,15 @@ where continue; } + // SAFETY: set_archetype was called prior. + // `current_index` is an archetype index row in range of the current archetype, because if it was not, then the if above would have been executed. if !self.filter.archetype_filter_fetch(self.current_index) { self.current_index += 1; continue; } + // SAFETY: set_archetype was called prior, `current_index` is an archetype index in range of the current archetype + // `current_index` 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 item = self.fetch.archetype_fetch(self.current_index); self.current_index += 1; return Some(item); diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index beb4776e1d..a1a271da81 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -81,7 +81,7 @@ impl QueryState { /// Checks if the query is empty for the given [`World`], where the last change and current tick are given. #[inline] pub fn is_empty(&self, world: &World, last_change_tick: u32, change_tick: u32) -> bool { - // SAFE: NopFetch does not access any members while &self ensures no one has exclusive access + // SAFETY: NopFetch does not access any members while &self ensures no one has exclusive access unsafe { self.iter_unchecked_manual::>(world, last_change_tick, change_tick) .next() @@ -211,7 +211,7 @@ impl QueryState { ) -> Result<[ROQueryItem<'w, Q>; N], QueryEntityError> { self.update_archetypes(world); - // SAFE: update_archetypes validates the `World` matches + // SAFETY: update_archetypes validates the `World` matches unsafe { self.get_many_read_only_manual( world, @@ -287,7 +287,7 @@ impl QueryState { ) -> Result<[QueryItem<'w, Q>; N], QueryEntityError> { self.update_archetypes(world); - // SAFE: method requires exclusive world access + // SAFETY: method requires exclusive world access // and world has been validated via update_archetypes unsafe { self.get_many_unchecked_manual( @@ -397,7 +397,7 @@ impl QueryState { last_change_tick: u32, change_tick: u32, ) -> Result<[ROQueryItem<'w, Q>; N], QueryEntityError> { - // SAFE: fetch is read-only + // SAFETY: fetch is read-only // and world must be validated let array_of_results = entities.map(|entity| { self.get_unchecked_manual::>( @@ -527,7 +527,7 @@ impl QueryState { &'s mut self, world: &'w World, ) -> QueryCombinationIter<'w, 's, Q, F, K> { - // SAFE: query is read only + // SAFETY: query is read only unsafe { self.update_archetypes(world); self.iter_combinations_unchecked_manual( @@ -550,7 +550,7 @@ impl QueryState { &'s mut self, world: &'w mut World, ) -> QueryCombinationIter<'w, 's, Q, F, K> { - // SAFE: query has unique world access + // SAFETY: query has unique world access unsafe { self.update_archetypes(world); self.iter_combinations_unchecked_manual( @@ -1272,6 +1272,8 @@ mod tests { // It's best to test get_many_unchecked_manual directly, // as it is shared and unsafe // We don't care about aliased mutabilty for the read-only equivalent + + // SAFETY: mutable access is not checked, but we own the world and don't use the query results assert!(unsafe { query_state .get_many_unchecked_manual::<10>( @@ -1284,6 +1286,7 @@ mod tests { }); assert_eq!( + // SAFETY: mutable access is not checked, but we own the world and don't use the query results unsafe { query_state .get_many_unchecked_manual( @@ -1298,6 +1301,7 @@ mod tests { ); assert_eq!( + // SAFETY: mutable access is not checked, but we own the world and don't use the query results unsafe { query_state .get_many_unchecked_manual( @@ -1312,6 +1316,7 @@ mod tests { ); assert_eq!( + // SAFETY: mutable access is not checked, but we own the world and don't use the query results unsafe { query_state .get_many_unchecked_manual( diff --git a/crates/bevy_ecs/src/reflect.rs b/crates/bevy_ecs/src/reflect.rs index 46871b59a1..ab4a81412e 100644 --- a/crates/bevy_ecs/src/reflect.rs +++ b/crates/bevy_ecs/src/reflect.rs @@ -69,7 +69,7 @@ impl ReflectComponent { world: &'a mut World, entity: Entity, ) -> Option> { - // SAFE: unique world access + // SAFETY: unique world access unsafe { (self.reflect_component_mut)(world, entity) } } @@ -137,14 +137,18 @@ impl FromType for ReflectComponent { .get::() .map(|c| c as &dyn Reflect) }, - reflect_component_mut: |world, entity| unsafe { - world - .get_entity(entity)? - .get_unchecked_mut::(world.last_change_tick(), world.read_change_tick()) - .map(|c| ReflectMut { - value: c.value as &mut dyn Reflect, - ticks: c.ticks, - }) + reflect_component_mut: |world, entity| { + // SAFETY: reflect_component_mut is an unsafe function pointer used by `reflect_component_unchecked_mut` which promises to never + // produce aliasing mutable references, and reflect_component_mut, which has mutable world access + unsafe { + world + .get_entity(entity)? + .get_unchecked_mut::(world.last_change_tick(), world.read_change_tick()) + .map(|c| ReflectMut { + value: c.value as &mut dyn Reflect, + ticks: c.ticks, + }) + } }, } } @@ -191,7 +195,7 @@ impl ReflectResource { /// Gets the value of this [`Resource`] type from the world as a mutable reflected reference. pub fn reflect_resource_mut<'a>(&self, world: &'a mut World) -> Option> { - // SAFE: unique world access + // SAFETY: unique world access unsafe { (self.reflect_resource_unchecked_mut)(world) } } @@ -205,6 +209,7 @@ impl ReflectResource { &self, world: &'a World, ) -> Option> { + // SAFETY: caller promises to uphold uniqueness guarantees (self.reflect_resource_unchecked_mut)(world) } @@ -234,13 +239,17 @@ impl FromType for ReflectResource { world.remove_resource::(); }, reflect_resource: |world| world.get_resource::().map(|res| res as &dyn Reflect), - reflect_resource_unchecked_mut: |world| unsafe { - world - .get_resource_unchecked_mut::() - .map(|res| ReflectMut { - value: res.value as &mut dyn Reflect, - ticks: res.ticks, - }) + reflect_resource_unchecked_mut: |world| { + // SAFETY: all usages of `reflect_resource_unchecked_mut` guarantee that there is either a single mutable + // reference or multiple immutable ones alive at any given point + unsafe { + world + .get_resource_unchecked_mut::() + .map(|res| ReflectMut { + value: res.value as &mut dyn Reflect, + ticks: res.ticks, + }) + } }, copy_resource: |source_world, destination_world| { let source_resource = source_world.resource::(); diff --git a/crates/bevy_ecs/src/schedule/executor_parallel.rs b/crates/bevy_ecs/src/schedule/executor_parallel.rs index c82924b0e2..5b9326ce98 100644 --- a/crates/bevy_ecs/src/schedule/executor_parallel.rs +++ b/crates/bevy_ecs/src/schedule/executor_parallel.rs @@ -190,6 +190,7 @@ impl ParallelExecutor { .unwrap_or_else(|error| unreachable!("{}", error)); #[cfg(feature = "trace")] let system_guard = system_span.enter(); + // SAFETY: the executor prevents two systems with conflicting access from running simultaneously. unsafe { system.run_unsafe((), world) }; #[cfg(feature = "trace")] drop(system_guard); diff --git a/crates/bevy_ecs/src/storage/blob_vec.rs b/crates/bevy_ecs/src/storage/blob_vec.rs index 1a4404d9b2..384c26a07d 100644 --- a/crates/bevy_ecs/src/storage/blob_vec.rs +++ b/crates/bevy_ecs/src/storage/blob_vec.rs @@ -1,6 +1,7 @@ use std::{ alloc::{handle_alloc_error, Layout}, cell::UnsafeCell, + num::NonZeroUsize, ptr::NonNull, }; @@ -14,7 +15,9 @@ pub(super) struct BlobVec { capacity: usize, /// Number of elements, not bytes len: usize, + // the `data` ptr's layout is always `array_layout(item_layout, capacity)` data: NonNull, + // the `swap_scratch` ptr's layout is always `item_layout` swap_scratch: NonNull, // None if the underlying type doesn't need to be dropped drop: Option)>, @@ -93,33 +96,45 @@ impl BlobVec { pub fn reserve_exact(&mut self, additional: usize) { let available_space = self.capacity - self.len; - if available_space < additional { - self.grow_exact(additional - available_space); + if available_space < additional && self.item_layout.size() > 0 { + // SAFETY: `available_space < additional`, so `additional - available_space > 0` + let increment = unsafe { NonZeroUsize::new_unchecked(additional - available_space) }; + // SAFETY: not called for ZSTs + unsafe { self.grow_exact(increment) }; } } - // FIXME: this should probably be an unsafe fn as it shouldn't be called if the layout - // is for a ZST - fn grow_exact(&mut self, increment: usize) { + // SAFETY: must not be called for a ZST item layout + #[warn(unsafe_op_in_unsafe_fn)] // to allow unsafe blocks in unsafe fn + unsafe fn grow_exact(&mut self, increment: NonZeroUsize) { debug_assert!(self.item_layout.size() != 0); - let new_capacity = self.capacity + increment; + let new_capacity = self.capacity + increment.get(); let new_layout = array_layout(&self.item_layout, new_capacity).expect("array layout should be valid"); - unsafe { - let new_data = if self.capacity == 0 { - std::alloc::alloc(new_layout) - } else { + let new_data = if self.capacity == 0 { + // SAFETY: + // - layout has non-zero size as per safety requirement + unsafe { std::alloc::alloc(new_layout) } + } else { + // SAFETY: + // - ptr was be allocated via this allocator + // - the layout of the ptr was `array_layout(self.item_layout, self.capacity)` + // - `item_layout.size() > 0` and `new_capacity > 0`, so the layout size is non-zero + // - "new_size, when rounded up to the nearest multiple of layout.align(), must not overflow (i.e., the rounded value must be less than usize::MAX)", + // since the item size is always a multiple of its align, the rounding cannot happen + // here and the overflow is handled in `array_layout` + unsafe { std::alloc::realloc( self.get_ptr_mut().as_ptr(), array_layout(&self.item_layout, self.capacity) .expect("array layout should be valid"), new_layout.size(), ) - }; + } + }; - self.data = NonNull::new(new_data).unwrap_or_else(|| handle_alloc_error(new_layout)); - } + self.data = NonNull::new(new_data).unwrap_or_else(|| handle_alloc_error(new_layout)); self.capacity = new_capacity; } @@ -274,14 +289,14 @@ impl BlobVec { /// Gets a [`Ptr`] to the start of the vec #[inline] pub fn get_ptr(&self) -> Ptr<'_> { - // SAFE: the inner data will remain valid for as long as 'self. + // SAFETY: the inner data will remain valid for as long as 'self. unsafe { Ptr::new(self.data) } } /// Gets a [`PtrMut`] to the start of the vec #[inline] pub fn get_ptr_mut(&mut self) -> PtrMut<'_> { - // SAFE: the inner data will remain valid for as long as 'self. + // SAFETY: the inner data will remain valid for as long as 'self. unsafe { PtrMut::new(self.data) } } @@ -290,7 +305,7 @@ impl BlobVec { /// # Safety /// The type `T` must be the type of the items in this [`BlobVec`]. pub unsafe fn get_slice(&self) -> &[UnsafeCell] { - // SAFE: the inner data will remain valid for as long as 'self. + // 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) } @@ -302,6 +317,7 @@ impl BlobVec { if let Some(drop) = self.drop { let layout_size = self.item_layout.size(); for i in 0..len { + // SAFETY: `i * layout_size` is inbounds for the allocation, and the item is left unreachable so it can be safely promoted to an `OwningPtr` unsafe { // NOTE: this doesn't use self.get_unchecked(i) because the debug_assert on index // will panic here due to self.len being set to 0 @@ -317,6 +333,7 @@ impl Drop for BlobVec { fn drop(&mut self) { self.clear(); if self.item_layout.size() > 0 { + // SAFETY: the `swap_scratch` pointer is always allocated using `self.item_layout` unsafe { std::alloc::dealloc(self.swap_scratch.as_ptr(), self.item_layout); } @@ -324,6 +341,7 @@ impl Drop for BlobVec { let array_layout = array_layout(&self.item_layout, self.capacity).expect("array layout should be valid"); if array_layout.size() > 0 { + // SAFETY: data ptr layout is correct, swap_scratch ptr layout is correct unsafe { std::alloc::dealloc(self.get_ptr_mut().as_ptr(), array_layout); } @@ -427,8 +445,9 @@ mod tests { #[test] fn resize_test() { let item_layout = Layout::new::(); - // usize doesn't need dropping + // SAFETY: `drop` fn is `None`, usize doesn't need dropping let mut blob_vec = unsafe { BlobVec::new(item_layout, None, 64) }; + // SAFETY: `i` is a usize, i.e. the type corresponding to `item_layout` unsafe { for i in 0..1_000 { push(&mut blob_vec, i as usize); @@ -458,8 +477,12 @@ mod tests { { let item_layout = Layout::new::(); let drop = drop_ptr::; + // SAFETY: drop is able to drop a value of its `item_layout` let mut blob_vec = unsafe { BlobVec::new(item_layout, Some(drop), 2) }; assert_eq!(blob_vec.capacity(), 2); + // SAFETY: the following code only deals with values of type `Foo`, which satisfies the safety requirement of `push`, `get_mut` and `swap_remove` that the + // values have a layout compatible to the blob vec's `item_layout`. + // Every index is in range. unsafe { let foo1 = Foo { a: 42, @@ -518,6 +541,7 @@ mod tests { fn blob_vec_drop_empty_capacity() { let item_layout = Layout::new::(); let drop = drop_ptr::; + // SAFETY: drop is able to drop a value of its `item_layout` let _ = unsafe { BlobVec::new(item_layout, Some(drop), 0) }; } } diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs index 1d5172b64a..3ca81979dc 100644 --- a/crates/bevy_ecs/src/storage/sparse_set.rs +++ b/crates/bevy_ecs/src/storage/sparse_set.rs @@ -159,7 +159,7 @@ impl ComponentSparseSet { let dense_index = *dense_index as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); - // SAFE: if the sparse index points to something in the dense vec, it exists + // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { self.dense.get_data_unchecked(dense_index) } }) } @@ -169,7 +169,7 @@ impl ComponentSparseSet { let dense_index = *self.sparse.get(entity.id())? as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); - // SAFE: if the sparse index points to something in the dense vec, it exists + // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { Some(( self.dense.get_data_unchecked(dense_index), @@ -183,7 +183,7 @@ impl ComponentSparseSet { let dense_index = *self.sparse.get(entity.id())? as usize; #[cfg(debug_assertions)] assert_eq!(entity, self.entities[dense_index]); - // SAFE: if the sparse index points to something in the dense vec, it exists + // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { Some(self.dense.get_ticks_unchecked(dense_index)) } } @@ -197,7 +197,7 @@ impl ComponentSparseSet { assert_eq!(entity, self.entities[dense_index]); self.entities.swap_remove(dense_index); let is_last = dense_index == self.dense.len() - 1; - // SAFE: dense_index was just removed from `sparse`, which ensures that it is valid + // SAFETY: dense_index was just removed from `sparse`, which ensures that it is valid let (value, _) = unsafe { self.dense.swap_remove_and_forget_unchecked(dense_index) }; if !is_last { let swapped_entity = self.entities[dense_index]; @@ -218,7 +218,7 @@ impl ComponentSparseSet { assert_eq!(entity, self.entities[dense_index]); self.entities.swap_remove(dense_index); let is_last = dense_index == self.dense.len() - 1; - // SAFE: if the sparse index points to something in the dense vec, it exists + // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { self.dense.swap_remove_unchecked(dense_index) } if !is_last { let swapped_entity = self.entities[dense_index]; @@ -280,7 +280,7 @@ impl SparseSet { pub fn insert(&mut self, index: I, value: V) { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { - // SAFE: dense indices stored in self.sparse always exist + // SAFETY: dense indices stored in self.sparse always exist unsafe { *self.dense.get_unchecked_mut(dense_index) = value; } @@ -293,7 +293,7 @@ impl SparseSet { pub fn get_or_insert_with(&mut self, index: I, func: impl FnOnce() -> V) -> &mut V { if let Some(dense_index) = self.sparse.get(index.clone()).cloned() { - // SAFE: dense indices stored in self.sparse always exist + // SAFETY: dense indices stored in self.sparse always exist unsafe { self.dense.get_unchecked_mut(dense_index) } } else { let value = func(); @@ -301,7 +301,7 @@ impl SparseSet { self.sparse.insert(index.clone(), dense_index); self.indices.push(index); self.dense.push(value); - // SAFE: dense index was just populated above + // SAFETY: dense index was just populated above unsafe { self.dense.get_unchecked_mut(dense_index) } } } @@ -323,7 +323,7 @@ impl SparseSet { pub fn get(&self, index: I) -> Option<&V> { self.sparse.get(index).map(|dense_index| { - // SAFE: if the sparse index points to something in the dense vec, it exists + // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { self.dense.get_unchecked(*dense_index) } }) } @@ -331,7 +331,7 @@ impl SparseSet { pub fn get_mut(&mut self, index: I) -> Option<&mut V> { let dense = &mut self.dense; self.sparse.get(index).map(move |dense_index| { - // SAFE: if the sparse index points to something in the dense vec, it exists + // SAFETY: if the sparse index points to something in the dense vec, it exists unsafe { dense.get_unchecked_mut(*dense_index) } }) } diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs index 4b716c6814..245f14be02 100644 --- a/crates/bevy_ecs/src/storage/table.rs +++ b/crates/bevy_ecs/src/storage/table.rs @@ -42,7 +42,7 @@ impl Column { #[inline] pub(crate) fn with_capacity(component_info: &ComponentInfo, capacity: usize) -> Self { Column { - // SAFE: component_info.drop() is valid for the types that will be inserted. + // SAFETY: component_info.drop() is valid for the types that will be inserted. data: unsafe { BlobVec::new(component_info.layout(), component_info.drop(), capacity) }, ticks: Vec::with_capacity(capacity), } @@ -559,7 +559,7 @@ mod tests { table.add_column(components.get_info(component_id).unwrap()); let entities = (0..200).map(Entity::from_raw).collect::>(); for entity in &entities { - // SAFE: we allocate and immediately set data afterwards + // SAFETY: we allocate and immediately set data afterwards unsafe { let row = table.allocate(*entity); let value: W = W(row); diff --git a/crates/bevy_ecs/src/system/commands/command_queue.rs b/crates/bevy_ecs/src/system/commands/command_queue.rs index d73b529253..fa21351fb2 100644 --- a/crates/bevy_ecs/src/system/commands/command_queue.rs +++ b/crates/bevy_ecs/src/system/commands/command_queue.rs @@ -21,10 +21,10 @@ pub struct CommandQueue { metas: Vec, } -// SAFE: All commands [`Command`] implement [`Send`] +// SAFETY: All commands [`Command`] implement [`Send`] unsafe impl Send for CommandQueue {} -// SAFE: `&CommandQueue` never gives access to the inner commands. +// SAFETY: `&CommandQueue` never gives access to the inner commands. unsafe impl Sync for CommandQueue {} impl CommandQueue { @@ -34,7 +34,7 @@ impl CommandQueue { where C: Command, { - /// SAFE: This function is only every called when the `command` bytes is the associated + /// SAFETY: This function is only every called when the `command` bytes is the associated /// [`Commands`] `T` type. Also this only reads the data via `read_unaligned` so unaligned /// accesses are safe. unsafe fn write_command(command: *mut MaybeUninit, world: &mut World) { @@ -57,7 +57,7 @@ impl CommandQueue { if size > 0 { self.bytes.reserve(size); - // SAFE: The internal `bytes` vector has enough storage for the + // SAFETY: The internal `bytes` vector has enough storage for the // command (see the call the `reserve` above), the vector has // its length set appropriately and can contain any kind of bytes. // In case we're writing a ZST and the `Vec` hasn't allocated yet @@ -83,13 +83,13 @@ impl CommandQueue { // flush the previously queued entities world.flush(); - // SAFE: In the iteration below, `meta.func` will safely consume and drop each pushed command. + // SAFETY: In the iteration below, `meta.func` will safely consume and drop each pushed command. // This operation is so that we can reuse the bytes `Vec`'s internal storage and prevent // unnecessary allocations. unsafe { self.bytes.set_len(0) }; for meta in self.metas.drain(..) { - // SAFE: The implementation of `write_command` is safe for the according Command type. + // SAFETY: The implementation of `write_command` is safe for the according Command type. // It's ok to read from `bytes.as_mut_ptr()` because we just wrote to it in `push`. // The bytes are safely cast to their original type, safely read, and then dropped. unsafe { diff --git a/crates/bevy_ecs/src/system/commands/parallel_scope.rs b/crates/bevy_ecs/src/system/commands/parallel_scope.rs index 41dc9b7289..02878041c4 100644 --- a/crates/bevy_ecs/src/system/commands/parallel_scope.rs +++ b/crates/bevy_ecs/src/system/commands/parallel_scope.rs @@ -68,7 +68,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for ParallelCommandsState { } } -// SAFE: no component or resource access to report +// SAFETY: no component or resource access to report unsafe impl SystemParamState for ParallelCommandsState { fn init(_: &mut World, _: &mut crate::system::SystemMeta) -> Self { Self::default() diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 9dec1b5f84..356c891dc3 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -166,7 +166,7 @@ impl SystemState { Param::Fetch: ReadOnlySystemParamFetch, { self.validate_world_and_update_archetypes(world); - // SAFE: Param is read-only and doesn't allow mutable access to World. It also matches the World this SystemState was created with. + // SAFETY: Param is read-only and doesn't allow mutable access to World. It also matches the World this SystemState was created with. unsafe { self.get_unchecked_manual(world) } } @@ -177,7 +177,7 @@ impl SystemState { world: &'w mut World, ) -> >::Item { self.validate_world_and_update_archetypes(world); - // SAFE: World is uniquely borrowed and matches the World this SystemState was created with. + // SAFETY: World is uniquely borrowed and matches the World this SystemState was created with. unsafe { self.get_unchecked_manual(world) } } diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index fd4ecb408e..024926fa9b 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -293,7 +293,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn iter(&self) -> QueryIter<'_, 's, Q, ROQueryFetch<'_, Q>, F> { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state @@ -323,7 +323,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn iter_mut(&mut self) -> QueryIter<'_, '_, Q, QueryFetch<'_, Q>, F> { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state @@ -340,7 +340,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// - if K > N: empty set (no K-sized combinations exist) #[inline] pub fn iter_combinations(&self) -> QueryCombinationIter<'_, '_, Q, F, K> { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state.iter_combinations_unchecked_manual( @@ -377,7 +377,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { pub fn iter_combinations_mut( &mut self, ) -> QueryCombinationIter<'_, '_, Q, F, K> { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state.iter_combinations_unchecked_manual( @@ -446,7 +446,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// this call does not result in multiple mutable references to the same component #[inline] pub unsafe fn iter_unsafe(&'s self) -> QueryIter<'w, 's, Q, QueryFetch<'w, Q>, F> { - // SEMI-SAFE: system runs without conflicts with other systems. + // SEMI-SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict self.state .iter_unchecked_manual(self.world, self.last_change_tick, self.change_tick) @@ -462,7 +462,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { pub unsafe fn iter_combinations_unsafe( &self, ) -> QueryCombinationIter<'_, '_, Q, F, K> { - // SEMI-SAFE: system runs without conflicts with other systems. + // SEMI-SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict self.state.iter_combinations_unchecked_manual( self.world, @@ -519,7 +519,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn for_each<'this>(&'this self, f: impl FnMut(ROQueryItem<'this, Q>)) { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state.for_each_unchecked_manual::, _>( @@ -554,7 +554,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn for_each_mut<'a, FN: FnMut(QueryItem<'a, Q>)>(&'a mut self, f: FN) { - // SAFE: system runs without conflicts with other systems. same-system queries have runtime + // SAFETY: system runs without conflicts with other systems. same-system queries have runtime // borrow checks when they conflict unsafe { self.state.for_each_unchecked_manual::, FN>( @@ -597,7 +597,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { batch_size: usize, f: impl Fn(ROQueryItem<'this, Q>) + Send + Sync + Clone, ) { - // SAFE: system runs without conflicts with other systems. same-system queries have runtime + // SAFETY: system runs without conflicts with other systems. same-system queries have runtime // borrow checks when they conflict unsafe { self.state @@ -625,7 +625,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { batch_size: usize, f: FN, ) { - // SAFE: system runs without conflicts with other systems. same-system queries have runtime + // SAFETY: system runs without conflicts with other systems. same-system queries have runtime // borrow checks when they conflict unsafe { self.state @@ -675,7 +675,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { ) where EntityList::Item: Borrow, { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state.many_for_each_unchecked_manual( @@ -721,7 +721,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn get(&self, entity: Entity) -> Result, QueryEntityError> { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state.get_unchecked_manual::>( @@ -746,7 +746,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { &self, entities: [Entity; N], ) -> Result<[ROQueryItem<'_, Q>; N], QueryEntityError> { - // SAFE: it is the scheduler's responsibility to ensure that `Query` is never handed out on the wrong `World`. + // SAFETY: it is the scheduler's responsibility to ensure that `Query` is never handed out on the wrong `World`. unsafe { self.state.get_many_read_only_manual( self.world, @@ -823,7 +823,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn get_mut(&mut self, entity: Entity) -> Result, QueryEntityError> { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state.get_unchecked_manual::>( @@ -846,7 +846,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { &mut self, entities: [Entity; N], ) -> Result<[QueryItem<'_, Q>; N], QueryEntityError> { - // SAFE: scheduler ensures safe Query world access + // SAFETY: scheduler ensures safe Query world access unsafe { self.state.get_many_unchecked_manual( self.world, @@ -917,7 +917,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { &'s self, entity: Entity, ) -> Result, QueryEntityError> { - // SEMI-SAFE: system runs without conflicts with other systems. + // 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, @@ -1011,7 +1011,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { &mut self, entity: Entity, ) -> Result, QueryComponentError> { - // SAFE: unique access to query (preventing aliased access) + // SAFETY: unique access to query (preventing aliased access) unsafe { self.get_component_unchecked_mut(entity) } } @@ -1117,6 +1117,9 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn get_single(&self) -> Result, QuerySingleError> { + // SAFETY: + // the query ensures that the components it accesses are not mutably accessible somewhere else + // and the query is read only. unsafe { self.state.get_single_unchecked_manual::>( self.world, @@ -1179,6 +1182,9 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn get_single_mut(&mut self) -> Result, QuerySingleError> { + // SAFETY: + // the query ensures mutable access to the components it accesses, and the query + // is uniquely borrowed unsafe { self.state.get_single_unchecked_manual::>( self.world, @@ -1238,7 +1244,7 @@ impl<'w, 's, Q: WorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn contains(&self, entity: Entity) -> bool { - // SAFE: NopFetch does not access any members while &self ensures no one has exclusive access + // SAFETY: NopFetch does not access any members while &self ensures no one has exclusive access unsafe { self.state .get_unchecked_manual::>( @@ -1340,7 +1346,7 @@ impl<'w, 's, Q: ReadOnlyWorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn get_inner(&'s self, entity: Entity) -> Result, QueryEntityError> { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state.get_unchecked_manual::>( @@ -1377,7 +1383,7 @@ impl<'w, 's, Q: ReadOnlyWorldQuery, F: WorldQuery> Query<'w, 's, Q, F> { /// ``` #[inline] pub fn iter_inner(&'s self) -> QueryIter<'w, 's, Q, ROQueryFetch<'w, Q>, F> { - // SAFE: system runs without conflicts with other systems. + // SAFETY: system runs without conflicts with other systems. // same-system queries have runtime borrow checks when they conflict unsafe { self.state diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index 44b719e526..2fbeba2382 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -46,7 +46,7 @@ pub trait System: Send + Sync + 'static { /// Runs the system with the given input in the world. fn run(&mut self, input: Self::In, world: &mut World) -> Self::Out { self.update_archetype_component_access(world); - // SAFE: world and resources are exclusively borrowed + // SAFETY: world and resources are exclusively borrowed unsafe { self.run_unsafe(input, world) } } fn apply_buffers(&mut self, world: &mut World); diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 96a574dd56..c3a419f0a8 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -134,10 +134,10 @@ impl<'w, 's, Q: WorldQuery + 'static, F: WorldQuery + 'static> SystemParam for Q type Fetch = QueryState; } -// SAFE: QueryState is constrained to read-only fetches, so it only reads World. +// SAFETY: QueryState is constrained to read-only fetches, so it only reads World. unsafe impl ReadOnlySystemParamFetch for QueryState {} -// SAFE: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If +// SAFETY: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If // this QueryState conflicts with any prior access, a panic will occur. unsafe impl SystemParamState for QueryState @@ -239,7 +239,7 @@ pub struct Res<'w, T: Resource> { change_tick: u32, } -// SAFE: Res only reads a single World resource +// SAFETY: Res only reads a single World resource unsafe impl ReadOnlySystemParamFetch for ResState {} impl<'w, T: Resource> Debug for Res<'w, T> @@ -305,7 +305,7 @@ impl<'a, T: Resource> SystemParam for Res<'a, T> { type Fetch = ResState; } -// SAFE: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res +// SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res // conflicts with any prior access, a panic will occur. unsafe impl SystemParamState for ResState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { @@ -370,9 +370,11 @@ impl<'a, T: Resource> SystemParam for Option> { type Fetch = OptionResState; } -// SAFE: Only reads a single World resource +// SAFETY: Only reads a single World resource unsafe impl ReadOnlySystemParamFetch for OptionResState {} +// SAFETY: this impl defers to `ResState`, which initializes +// and validates the correct world access unsafe impl SystemParamState for OptionResState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { Self(ResState::init(world, system_meta)) @@ -411,7 +413,7 @@ impl<'a, T: Resource> SystemParam for ResMut<'a, T> { type Fetch = ResMutState; } -// SAFE: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res +// SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res // conflicts with any prior access, a panic will occur. unsafe impl SystemParamState for ResMutState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { @@ -481,6 +483,8 @@ impl<'a, T: Resource> SystemParam for Option> { type Fetch = OptionResMutState; } +// SAFETY: this impl defers to `ResMutState`, which initializes +// and validates the correct world access unsafe impl SystemParamState for OptionResMutState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { Self(ResMutState::init(world, system_meta)) @@ -514,10 +518,10 @@ impl<'w, 's> SystemParam for Commands<'w, 's> { type Fetch = CommandQueue; } -// SAFE: Commands only accesses internal state +// SAFETY: Commands only accesses internal state unsafe impl ReadOnlySystemParamFetch for CommandQueue {} -// SAFE: only local state is accessed +// SAFETY: only local state is accessed unsafe impl SystemParamState for CommandQueue { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { Default::default() @@ -542,7 +546,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for CommandQueue { } } -/// SAFE: only reads world +/// SAFETY: only reads world unsafe impl ReadOnlySystemParamFetch for WorldState {} /// The [`SystemParamState`] of [`&World`](crate::world::World). @@ -553,6 +557,7 @@ impl<'w> SystemParam for &'w World { type Fetch = WorldState; } +// SAFETY: `read_all` access is set and conflicts result in a panic unsafe impl SystemParamState for WorldState { fn init(_world: &mut World, system_meta: &mut SystemMeta) -> Self { let mut access = Access::default(); @@ -637,7 +642,7 @@ impl<'w, 's> SystemParamFetch<'w, 's> for WorldState { /// ``` pub struct Local<'a, T: Resource>(&'a mut T); -// SAFE: Local only accesses internal state +// SAFETY: Local only accesses internal state unsafe impl ReadOnlySystemParamFetch for LocalState {} impl<'a, T: Resource> Debug for Local<'a, T> @@ -673,7 +678,7 @@ impl<'a, T: Resource + FromWorld> SystemParam for Local<'a, T> { type Fetch = LocalState; } -// SAFE: only local state is accessed +// SAFETY: only local state is accessed unsafe impl SystemParamState for LocalState { fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self { Self(T::from_world(world)) @@ -741,7 +746,7 @@ impl<'a, T: Component> RemovedComponents<'a, T> { } } -// SAFE: Only reads World components +// SAFETY: Only reads World components unsafe impl ReadOnlySystemParamFetch for RemovedComponentsState {} /// The [`SystemParamState`] of [`RemovedComponents`]. @@ -755,7 +760,7 @@ impl<'a, T: Component> SystemParam for RemovedComponents<'a, T> { type Fetch = RemovedComponentsState; } -// SAFE: no component access. removed component entity collections can be read in parallel and are +// SAFETY: no component access. removed component entity collections can be read in parallel and are // never mutably borrowed during system execution unsafe impl SystemParamState for RemovedComponentsState { fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self { @@ -803,7 +808,7 @@ pub struct NonSend<'w, T: 'static> { change_tick: u32, } -// SAFE: Only reads a single World non-send resource +// SAFETY: Only reads a single World non-send resource unsafe impl ReadOnlySystemParamFetch for NonSendState {} impl<'w, T> Debug for NonSend<'w, T> @@ -857,7 +862,7 @@ impl<'a, T: 'static> SystemParam for NonSend<'a, T> { type Fetch = NonSendState; } -// SAFE: NonSendComponentId and ArchetypeComponentId access is applied to SystemMeta. If this +// SAFETY: NonSendComponentId and ArchetypeComponentId access is applied to SystemMeta. If this // NonSend conflicts with any prior access, a panic will occur. unsafe impl SystemParamState for NonSendState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { @@ -926,9 +931,11 @@ impl<'w, T: 'static> SystemParam for Option> { type Fetch = OptionNonSendState; } -// SAFE: Only reads a single non-send resource +// SAFETY: Only reads a single non-send resource unsafe impl ReadOnlySystemParamFetch for OptionNonSendState {} +// SAFETY: this impl defers to `NonSendState`, which initializes +// and validates the correct world access unsafe impl SystemParamState for OptionNonSendState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { Self(NonSendState::init(world, system_meta)) @@ -968,7 +975,7 @@ impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { type Fetch = NonSendMutState; } -// SAFE: NonSendMut ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this +// SAFETY: NonSendMut ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this // NonSendMut conflicts with any prior access, a panic will occur. unsafe impl SystemParamState for NonSendMutState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { @@ -1041,6 +1048,8 @@ impl<'a, T: 'static> SystemParam for Option> { type Fetch = OptionNonSendMutState; } +// SAFETY: this impl defers to `NonSendMutState`, which initializes +// and validates the correct world access unsafe impl SystemParamState for OptionNonSendMutState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { Self(NonSendMutState::init(world, system_meta)) @@ -1075,14 +1084,14 @@ impl<'a> SystemParam for &'a Archetypes { type Fetch = ArchetypesState; } -// SAFE: Only reads World archetypes +// SAFETY: Only reads World archetypes unsafe impl ReadOnlySystemParamFetch for ArchetypesState {} /// The [`SystemParamState`] of [`Archetypes`]. #[doc(hidden)] pub struct ArchetypesState; -// SAFE: no component value access +// SAFETY: no component value access unsafe impl SystemParamState for ArchetypesState { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { Self @@ -1107,14 +1116,14 @@ impl<'a> SystemParam for &'a Components { type Fetch = ComponentsState; } -// SAFE: Only reads World components +// SAFETY: Only reads World components unsafe impl ReadOnlySystemParamFetch for ComponentsState {} /// The [`SystemParamState`] of [`Components`]. #[doc(hidden)] pub struct ComponentsState; -// SAFE: no component value access +// SAFETY: no component value access unsafe impl SystemParamState for ComponentsState { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { Self @@ -1139,14 +1148,14 @@ impl<'a> SystemParam for &'a Entities { type Fetch = EntitiesState; } -// SAFE: Only reads World entities +// SAFETY: Only reads World entities unsafe impl ReadOnlySystemParamFetch for EntitiesState {} /// The [`SystemParamState`] of [`Entities`]. #[doc(hidden)] pub struct EntitiesState; -// SAFE: no component value access +// SAFETY: no component value access unsafe impl SystemParamState for EntitiesState { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { Self @@ -1171,14 +1180,14 @@ impl<'a> SystemParam for &'a Bundles { type Fetch = BundlesState; } -// SAFE: Only reads World bundles +// SAFETY: Only reads World bundles unsafe impl ReadOnlySystemParamFetch for BundlesState {} /// The [`SystemParamState`] of [`Bundles`]. #[doc(hidden)] pub struct BundlesState; -// SAFE: no component value access +// SAFETY: no component value access unsafe impl SystemParamState for BundlesState { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { Self @@ -1228,7 +1237,7 @@ impl SystemChangeTick { } } -// SAFE: Only reads internal system state +// SAFETY: Only reads internal system state unsafe impl ReadOnlySystemParamFetch for SystemChangeTickState {} impl SystemParam for SystemChangeTick { @@ -1239,6 +1248,7 @@ impl SystemParam for SystemChangeTick { #[doc(hidden)] pub struct SystemChangeTickState {} +// SAFETY: `SystemParamTickState` doesn't require any world access unsafe impl SystemParamState for SystemChangeTickState { fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { Self {} @@ -1267,7 +1277,7 @@ macro_rules! impl_system_param_tuple { type Fetch = ($($param::Fetch,)*); } - // SAFE: tuple consists only of ReadOnlySystemParamFetches + // SAFETY: tuple consists only of ReadOnlySystemParamFetches unsafe impl<$($param: ReadOnlySystemParamFetch),*> ReadOnlySystemParamFetch for ($($param,)*) {} #[allow(unused_variables)] @@ -1289,7 +1299,8 @@ macro_rules! impl_system_param_tuple { } } - /// SAFE: implementors of each `SystemParamState` in the tuple have validated their impls + // SAFETY: implementors of each `SystemParamState` in the tuple have validated their impls + #[allow(clippy::undocumented_unsafe_blocks)] // false positive by clippy #[allow(non_snake_case)] unsafe impl<$($param: SystemParamState),*> SystemParamState for ($($param,)*) { #[inline] @@ -1403,7 +1414,7 @@ impl<'w, 's, P: SystemParam> StaticSystemParam<'w, 's, P> { #[doc(hidden)] pub struct StaticSystemParamState(S, PhantomData P>); -// Safe: This doesn't add any more reads, and the delegated fetch confirms it +// SAFETY: This doesn't add any more reads, and the delegated fetch confirms it unsafe impl ReadOnlySystemParamFetch for StaticSystemParamState { @@ -1428,11 +1439,12 @@ where world: &'world World, change_tick: u32, ) -> Self::Item { - // Safe: We properly delegate SystemParamState + // SAFETY: We properly delegate SystemParamState StaticSystemParam(S::get_param(&mut state.0, system_meta, world, change_tick)) } } +// SAFETY: all methods are just delegated to `S`'s `SystemParamState` implementation unsafe impl SystemParamState for StaticSystemParamState { diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index bf27017141..7d1051bba1 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -65,7 +65,7 @@ impl<'w> EntityRef<'w> { #[inline] pub fn get(&self) -> Option<&'w T> { - // SAFE: entity location is valid and returned component is of type T + // SAFETY: entity location is valid and returned component is of type T unsafe { get_component_with_type(self.world, TypeId::of::(), self.entity, self.location) .map(|value| value.deref::()) @@ -76,7 +76,7 @@ impl<'w> EntityRef<'w> { /// detection in custom runtimes. #[inline] pub fn get_change_ticks(&self) -> Option<&'w ComponentTicks> { - // SAFE: entity location is valid + // SAFETY: entity location is valid unsafe { get_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) .map(|ticks| ticks.deref()) @@ -123,7 +123,7 @@ impl<'w> EntityRef<'w> { #[inline] pub fn get_by_id(&self, component_id: ComponentId) -> Option> { self.world.components().get_info(component_id)?; - // SAFE: entity_location is valid, component_id is valid as checked by the line above + // SAFETY: entity_location is valid, component_id is valid as checked by the line above unsafe { get_component(self.world, component_id, self.entity, self.location) } } } @@ -184,7 +184,7 @@ impl<'w> EntityMut<'w> { #[inline] pub fn get(&self) -> Option<&'_ T> { - // SAFE: lifetimes enforce correct usage of returned borrow + // SAFETY: lifetimes enforce correct usage of returned borrow unsafe { get_component_with_type(self.world, TypeId::of::(), self.entity, self.location) .map(|value| value.deref::()) @@ -193,7 +193,7 @@ impl<'w> EntityMut<'w> { #[inline] pub fn get_mut(&mut self) -> Option> { - // SAFE: world access is unique, and lifetimes enforce correct usage of returned borrow + // SAFETY: world access is unique, and lifetimes enforce correct usage of returned borrow unsafe { self.get_unchecked_mut::() } } @@ -201,7 +201,7 @@ impl<'w> EntityMut<'w> { /// detection in custom runtimes. #[inline] pub fn get_change_ticks(&self) -> Option<&ComponentTicks> { - // SAFE: entity location is valid + // SAFETY: entity location is valid unsafe { get_ticks_with_type(self.world, TypeId::of::(), self.entity, self.location) .map(|ticks| ticks.deref()) @@ -245,7 +245,7 @@ impl<'w> EntityMut<'w> { self.location.archetype_id, change_tick, ); - // SAFE: location matches current entity. `T` matches `bundle_info` + // SAFETY: location matches current entity. `T` matches `bundle_info` unsafe { self.location = bundle_inserter.insert(self.entity, self.location.index, bundle); } @@ -263,6 +263,8 @@ impl<'w> EntityMut<'w> { let bundle_info = self.world.bundles.init_info::(components, storages); let old_location = self.location; + // SAFETY: `archetype_id` exists because it is referenced in the old `EntityLocation` which is valid, + // components exist in `bundle_info` because `Bundles::init_info` initializes a `BundleInfo` containing all components of the bundle type `T` let new_archetype_id = unsafe { remove_bundle_from_archetype( archetypes, @@ -281,12 +283,12 @@ impl<'w> EntityMut<'w> { let old_archetype = &mut archetypes[old_location.archetype_id]; let mut bundle_components = bundle_info.component_ids.iter().cloned(); let entity = self.entity; - // SAFE: bundle components are iterated in order, which guarantees that the component type + // SAFETY: bundle components are iterated in order, which guarantees that the component type // matches let result = unsafe { T::from_components(storages, |storages| { let component_id = bundle_components.next().unwrap(); - // SAFE: entity location is valid and table row is removed below + // SAFETY: entity location is valid and table row is removed below take_component( components, storages, @@ -299,6 +301,7 @@ impl<'w> EntityMut<'w> { }) }; + #[allow(clippy::undocumented_unsafe_blocks)] // TODO: document why this is safe unsafe { Self::move_entity_from_remove::( entity, @@ -350,14 +353,14 @@ impl<'w> EntityMut<'w> { .tables .get_2_mut(old_table_id, new_archetype.table_id()); - // SAFE: old_table_row exists + // SAFETY: old_table_row exists let move_result = if DROP { 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) }; - // SAFE: move_result.new_row is a valid position in new_archetype's 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); // if an entity was moved into this entity's table spot, update its table row @@ -385,6 +388,9 @@ impl<'w> EntityMut<'w> { let bundle_info = self.world.bundles.init_info::(components, storages); let old_location = self.location; + + // SAFETY: `archetype_id` exists because it is referenced in the old `EntityLocation` which is valid, + // components exist in `bundle_info` because `Bundles::init_info` initializes a `BundleInfo` containing all components of the bundle type `T` let new_archetype_id = unsafe { remove_bundle_from_archetype( archetypes, @@ -421,6 +427,7 @@ impl<'w> EntityMut<'w> { } } + #[allow(clippy::undocumented_unsafe_blocks)] // TODO: document why this is safe unsafe { Self::move_entity_from_remove::( entity, @@ -470,7 +477,7 @@ impl<'w> EntityMut<'w> { let sparse_set = world.storages.sparse_sets.get_mut(*component_id).unwrap(); sparse_set.remove(self.entity); } - // SAFE: table rows stored in archetypes always exist + // SAFETY: table rows stored in archetypes always exist moved_entity = unsafe { world.storages.tables[archetype.table_id()].swap_remove_unchecked(table_row) }; @@ -517,7 +524,7 @@ impl<'w> EntityMut<'w> { #[inline] pub fn get_by_id(&self, component_id: ComponentId) -> Option> { self.world.components().get_info(component_id)?; - // SAFE: entity_location is valid, component_id is valid as checked by the line above + // SAFETY: entity_location is valid, component_id is valid as checked by the line above unsafe { get_component(self.world, component_id, self.entity, self.location) } } @@ -532,7 +539,7 @@ impl<'w> EntityMut<'w> { #[inline] pub fn get_mut_by_id(&mut self, component_id: ComponentId) -> Option> { self.world.components().get_info(component_id)?; - // SAFE: entity_location is valid, component_id is valid as checked by the line above + // SAFETY: entity_location is valid, component_id is valid as checked by the line above unsafe { get_mut_by_id(self.world, self.entity, self.location, component_id) } } } @@ -552,14 +559,14 @@ pub(crate) unsafe fn get_component( location: EntityLocation, ) -> Option> { let archetype = &world.archetypes[location.archetype_id]; - // SAFE: component_id exists and is therefore valid + // SAFETY: component_id exists and is therefore valid let component_info = world.components.get_info_unchecked(component_id); match component_info.storage_type() { StorageType::Table => { let table = &world.storages.tables[archetype.table_id()]; let components = table.get_column(component_id)?; let table_row = archetype.entity_table_row(location.index); - // SAFE: archetypes only store valid table_rows and the stored component type is T + // SAFETY: archetypes only store valid table_rows and the stored component type is T Some(components.get_data_unchecked(table_row)) } StorageType::SparseSet => world @@ -589,7 +596,7 @@ unsafe fn get_component_and_ticks( let table = &world.storages.tables[archetype.table_id()]; let components = table.get_column(component_id)?; let table_row = archetype.entity_table_row(location.index); - // SAFE: archetypes only store valid table_rows and the stored component type is T + // SAFETY: archetypes only store valid table_rows and the stored component type is T Some(( components.get_data_unchecked(table_row), components.get_ticks_unchecked(table_row), @@ -617,7 +624,7 @@ unsafe fn get_ticks( let table = &world.storages.tables[archetype.table_id()]; let components = table.get_column(component_id)?; let table_row = archetype.entity_table_row(location.index); - // SAFE: archetypes only store valid table_rows and the stored component type is T + // SAFETY: archetypes only store valid table_rows and the stored component type is T Some(components.get_ticks_unchecked(table_row)) } StorageType::SparseSet => world @@ -655,10 +662,10 @@ unsafe fn take_component<'a>( match component_info.storage_type() { StorageType::Table => { let table = &mut storages.tables[archetype.table_id()]; - // SAFE: archetypes will always point to valid columns + // SAFETY: archetypes will always point to valid columns let components = table.get_column_mut(component_id).unwrap(); let table_row = archetype.entity_table_row(location.index); - // SAFE: archetypes only store valid table_rows and the stored component type is T + // SAFETY: archetypes only store valid table_rows and the stored component type is T components.get_data_unchecked_mut(table_row).promote() } StorageType::SparseSet => storages @@ -768,7 +775,7 @@ unsafe fn remove_bundle_from_archetype( let mut removed_sparse_set_components = Vec::new(); for component_id in bundle_info.component_ids.iter().cloned() { if current_archetype.contains(component_id) { - // SAFE: bundle components were already initialized by bundles.get_info + // SAFETY: bundle components were already initialized by bundles.get_info let component_info = components.get_info_unchecked(component_id); match component_info.storage_type() { StorageType::Table => removed_table_components.push(component_id), @@ -800,7 +807,7 @@ unsafe fn remove_bundle_from_archetype( next_table_id = if removed_table_components.is_empty() { current_archetype.table_id() } else { - // SAFE: all components in next_table_components exist + // SAFETY: all components in next_table_components exist storages .tables .get_id_or_insert(&next_table_components, components) @@ -850,7 +857,7 @@ pub(crate) unsafe fn get_mut( entity: Entity, location: EntityLocation, ) -> Option> { - // SAFE: world access is unique, entity location is valid, and returned component is of type + // SAFETY: world access is unique, entity location is valid, and returned component is of type // T let change_tick = world.change_tick(); let last_change_tick = world.last_change_tick(); @@ -874,7 +881,7 @@ pub(crate) unsafe fn get_mut_by_id( location: EntityLocation, component_id: ComponentId, ) -> Option { - // SAFE: world access is unique, entity location and component_id required to be valid + // SAFETY: world access is unique, entity location and component_id required to be valid get_component_and_ticks(world, component_id, entity, location).map(|(value, ticks)| { MutUntyped { value: value.assert_unique(), @@ -928,7 +935,7 @@ mod tests { let entity = world.entity(entity); let test_component = entity.get_by_id(component_id).unwrap(); - // SAFE: points to a valid `TestComponent` + // SAFETY: points to a valid `TestComponent` let test_component = unsafe { test_component.deref::() }; assert_eq!(test_component.0, 42); @@ -947,14 +954,15 @@ mod tests { let mut test_component = entity_mut.get_mut_by_id(component_id).unwrap(); { test_component.set_changed(); - // SAFE: `test_component` has unique access of the `EntityMut` and is not used afterwards let test_component = + // SAFETY: `test_component` has unique access of the `EntityMut` and is not used afterwards unsafe { test_component.into_inner().deref_mut::() }; test_component.0 = 43; } let entity = world.entity(entity); let test_component = entity.get_by_id(component_id).unwrap(); + // SAFETY: `TestComponent` is the correct component type let test_component = unsafe { test_component.deref::() }; assert_eq!(test_component.0, 43); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d220495a82..97bc75ebbb 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -317,11 +317,11 @@ impl World { self.flush(); match self.entities.alloc_at_without_replacement(entity) { AllocAtWithoutReplacement::Exists(location) => { - // SAFE: `entity` exists and `location` is that entity's location + // SAFETY: `entity` exists and `location` is that entity's location Some(unsafe { EntityMut::new(self, entity, location) }) } AllocAtWithoutReplacement::DidNotExist => { - // SAFE: entity was just allocated + // SAFETY: entity was just allocated Some(unsafe { self.spawn_at_internal(entity) }) } AllocAtWithoutReplacement::ExistsWithWrongGeneration => None, @@ -381,7 +381,7 @@ impl World { #[inline] pub fn get_entity_mut(&mut self, entity: Entity) -> Option { let location = self.entities.get(entity)?; - // SAFE: `entity` exists and `location` is that entity's location + // SAFETY: `entity` exists and `location` is that entity's location Some(unsafe { EntityMut::new(self, entity, location) }) } @@ -413,7 +413,7 @@ impl World { pub fn spawn(&mut self) -> EntityMut { self.flush(); let entity = self.entities.alloc(); - // SAFE: entity was just allocated + // SAFETY: entity was just allocated unsafe { self.spawn_at_internal(entity) } } @@ -423,10 +423,10 @@ impl World { let archetype = self.archetypes.empty_mut(); // PERF: consider avoiding allocating entities in the empty archetype unless needed let table_row = self.storages.tables[archetype.table_id()].allocate(entity); - // SAFE: no components are allocated by archetype.allocate() because the archetype is + // SAFETY: no components are allocated by archetype.allocate() because the archetype is // empty let location = archetype.allocate(entity, table_row); - // SAFE: entity index was just allocated + // SAFETY: entity index was just allocated self.entities .meta .get_unchecked_mut(entity.id() as usize) @@ -507,7 +507,7 @@ impl World { /// ``` #[inline] pub fn get_mut(&mut self, entity: Entity) -> Option> { - // SAFE: lifetimes enforce correct usage of returned borrow + // SAFETY: lifetimes enforce correct usage of returned borrow unsafe { get_mut(self, entity, self.get_entity(entity)?.location()) } } @@ -688,7 +688,7 @@ impl World { #[inline] pub fn insert_resource(&mut self, value: R) { let component_id = self.components.init_resource::(); - // SAFE: component_id just initialized and corresponds to resource of type T + // SAFETY: component_id just initialized and corresponds to resource of type T unsafe { self.insert_resource_with_id(component_id, value) }; } @@ -716,21 +716,21 @@ impl World { pub fn insert_non_send_resource(&mut self, value: R) { self.validate_non_send_access::(); let component_id = self.components.init_non_send::(); - // SAFE: component_id just initialized and corresponds to resource of type R + // SAFETY: component_id just initialized and corresponds to resource of type R unsafe { self.insert_resource_with_id(component_id, value) }; } /// Removes the resource of a given type and returns it, if it exists. Otherwise returns [None]. #[inline] pub fn remove_resource(&mut self) -> Option { - // SAFE: R is Send + Sync + // SAFETY: R is Send + Sync unsafe { self.remove_resource_unchecked() } } #[inline] pub fn remove_non_send_resource(&mut self) -> Option { self.validate_non_send_access::(); - // SAFE: we are on main thread + // SAFETY: we are on main thread unsafe { self.remove_resource_unchecked() } } @@ -747,10 +747,10 @@ impl World { if column.is_empty() { return None; } - // SAFE: if a resource column exists, row 0 exists as well. caller takes ownership of the + // SAFETY: if a resource column exists, row 0 exists as well. caller takes ownership of the // ptr value / drop is called when R is dropped let (ptr, _) = unsafe { column.swap_remove_and_forget_unchecked(0) }; - // SAFE: column is of type R + // SAFETY: column is of type R Some(unsafe { ptr.read::() }) } @@ -778,7 +778,7 @@ impl World { } else { return false; }; - // SAFE: resources table always have row 0 + // SAFETY: resources table always have row 0 let ticks = unsafe { column.get_ticks_unchecked(0).deref() }; ticks.is_added(self.last_change_tick(), self.read_change_tick()) } @@ -795,7 +795,7 @@ impl World { } else { return false; }; - // SAFE: resources table always have row 0 + // SAFETY: resources table always have row 0 let ticks = unsafe { column.get_ticks_unchecked(0).deref() }; ticks.is_changed(self.last_change_tick(), self.read_change_tick()) } @@ -852,14 +852,14 @@ impl World { #[inline] pub fn get_resource(&self) -> Option<&R> { let component_id = self.components.get_resource_id(TypeId::of::())?; - // SAFE: unique world access + // SAFETY: unique world access unsafe { self.get_resource_with_id(component_id) } } /// Gets a mutable reference to the resource of the given type if it exists #[inline] pub fn get_resource_mut(&mut self) -> Option> { - // SAFE: unique world access + // SAFETY: unique world access unsafe { self.get_resource_unchecked_mut() } } @@ -882,7 +882,7 @@ impl World { /// /// # Safety /// This will allow aliased mutable access to the given resource type. The caller must ensure - /// that only one mutable access exists at a time. + /// that there is either only one mutable access or multiple immutable accesses at a time. #[inline] pub unsafe fn get_resource_unchecked_mut(&self) -> Option> { let component_id = self.components.get_resource_id(TypeId::of::())?; @@ -934,7 +934,7 @@ impl World { #[inline] pub fn get_non_send_resource(&self) -> Option<&R> { let component_id = self.components.get_resource_id(TypeId::of::())?; - // SAFE: component id matches type T + // SAFETY: component id matches type T unsafe { self.get_non_send_with_id(component_id) } } @@ -942,7 +942,7 @@ impl World { /// Otherwise returns [None] #[inline] pub fn get_non_send_resource_mut(&mut self) -> Option> { - // SAFE: unique world access + // SAFETY: unique world access unsafe { self.get_non_send_resource_unchecked_mut() } } @@ -951,7 +951,7 @@ impl World { /// /// # Safety /// This will allow aliased mutable access to the given non-send resource type. The caller must - /// ensure that only one mutable access exists at a time. + /// ensure that there is either only one mutable access or multiple immutable accesses at a time. #[inline] pub unsafe fn get_non_send_resource_unchecked_mut(&self) -> Option> { let component_id = self.components.get_resource_id(TypeId::of::())?; @@ -1033,7 +1033,7 @@ impl World { SpawnOrInsert::Insert(ref mut inserter, archetype) if location.archetype_id == archetype => { - // SAFE: `entity` is valid, `location` matches entity, bundle matches inserter + // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter unsafe { inserter.insert(entity, location.index, bundle) }; } _ => { @@ -1045,7 +1045,7 @@ impl World { location.archetype_id, change_tick, ); - // SAFE: `entity` is valid, `location` matches entity, bundle matches inserter + // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter unsafe { inserter.insert(entity, location.index, bundle) }; spawn_or_insert = SpawnOrInsert::Insert(inserter, location.archetype_id); @@ -1054,7 +1054,7 @@ impl World { } AllocAtWithoutReplacement::DidNotExist => { if let SpawnOrInsert::Spawn(ref mut spawner) = spawn_or_insert { - // SAFE: `entity` is allocated (but non existent), bundle matches inserter + // SAFETY: `entity` is allocated (but non existent), bundle matches inserter unsafe { spawner.spawn_non_existent(entity, bundle) }; } else { let mut spawner = bundle_info.get_bundle_spawner( @@ -1064,7 +1064,7 @@ impl World { &mut self.storages, change_tick, ); - // SAFE: `entity` is valid, `location` matches entity, bundle matches inserter + // SAFETY: `entity` is valid, `location` matches entity, bundle matches inserter unsafe { spawner.spawn_non_existent(entity, bundle) }; spawn_or_insert = SpawnOrInsert::Spawn(spawner); } @@ -1123,11 +1123,11 @@ impl World { "resource does not exist: {}", std::any::type_name::() ); - // SAFE: if a resource column exists, row 0 exists as well. caller takes ownership of + // SAFETY: if a resource column exists, row 0 exists as well. caller takes ownership of // the ptr value / drop is called when R is dropped unsafe { column.swap_remove_and_forget_unchecked(0) } }; - // SAFE: pointer is of type R + // SAFETY: pointer is of type R // Read the value onto the stack to avoid potential mut aliasing. let mut value = unsafe { ptr.read::() }; let value_mut = Mut { @@ -1148,8 +1148,8 @@ impl World { .unwrap_or_else(|| panic!("resource does not exist: {}", std::any::type_name::())); OwningPtr::make(value, |ptr| { + // SAFETY: pointer is of type R unsafe { - // SAFE: pointer is of type R column.push(ptr, ticks); } }); @@ -1216,12 +1216,12 @@ impl World { let change_tick = self.change_tick(); let column = self.initialize_resource_internal(component_id); if column.is_empty() { - // SAFE: column is of type R and has been allocated above + // SAFETY: column is of type R and has been allocated above OwningPtr::make(value, |ptr| { column.push(ptr, ComponentTicks::new(change_tick)); }); } else { - // SAFE: column is of type R and has already been allocated + // SAFETY: column is of type R and has already been allocated *column.get_data_unchecked_mut(0).deref_mut::() = value; column.get_ticks_unchecked_mut(0).set_changed(change_tick); } @@ -1246,10 +1246,10 @@ impl World { "insert_resource_by_id called with component id which doesn't exist in this world" ) }); - // SAFE: component_id is valid, checked by the lines above + // SAFETY: component_id is valid, checked by the lines above let column = self.initialize_resource_internal(component_id); if column.is_empty() { - // SAFE: column is of type R and has been allocated above + // SAFETY: column is of type R and has been allocated above column.push(value, ComponentTicks::new(change_tick)); } else { let ptr = column.get_data_unchecked_mut(0); @@ -1266,7 +1266,7 @@ impl World { /// `component_id` must be valid for this world #[inline] unsafe fn initialize_resource_internal(&mut self, component_id: ComponentId) -> &mut Column { - // SAFE: resource archetype always exists + // SAFETY: resource archetype always exists let resource_archetype = self .archetypes .archetypes @@ -1294,14 +1294,14 @@ impl World { pub(crate) fn initialize_resource(&mut self) -> ComponentId { let component_id = self.components.init_resource::(); - // SAFE: resource initialized above + // SAFETY: resource initialized above unsafe { self.initialize_resource_internal(component_id) }; component_id } pub(crate) fn initialize_non_send_resource(&mut self) -> ComponentId { let component_id = self.components.init_non_send::(); - // SAFE: resource initialized above + // SAFETY: resource initialized above unsafe { self.initialize_resource_internal(component_id) }; component_id } @@ -1343,12 +1343,12 @@ impl World { /// such as inserting a [Component]. pub(crate) fn flush(&mut self) { let empty_archetype = self.archetypes.empty_mut(); + let table = &mut self.storages.tables[empty_archetype.table_id()]; + // PERF: consider pre-allocating space for flushed entities + // SAFETY: entity is set to a valid location unsafe { - let table = &mut self.storages.tables[empty_archetype.table_id()]; - // PERF: consider pre-allocating space for flushed entities - // SAFE: entity is set to a valid location self.entities.flush(|entity, location| { - // SAFE: no components are allocated by archetype.allocate() because the archetype + // SAFETY: no components are allocated by archetype.allocate() because the archetype // is empty *location = empty_archetype.allocate(entity, table.allocate(entity)); }); @@ -1428,9 +1428,10 @@ impl World { let column = self.get_populated_resource_column(component_id)?; - // SAFE: get_data_ptr requires that the mutability rules are not violated, and the caller promises + // SAFETY: get_data_ptr requires that the mutability rules are not violated, and the caller promises // to only modify the resource while the mutable borrow of the world is valid let ticks = Ticks { + // SAFETY: // - index is in-bounds because the column is initialized and non-empty // - no other reference to the ticks of the same row can exist at the same time component_ticks: unsafe { &mut *column.get_ticks_unchecked(0).get() }, @@ -1439,6 +1440,7 @@ impl World { }; Some(MutUntyped { + // SAFETY: world access is unique, so no other reference can exist at the same time value: unsafe { column.get_data_ptr().assert_unique() }, ticks, }) @@ -1460,7 +1462,7 @@ impl World { if column.is_empty() { return None; } - // SAFE: if a resource column exists, row 0 exists as well + // SAFETY: if a resource column exists, row 0 exists as well unsafe { column.swap_remove_unchecked(0) }; Some(()) @@ -1474,7 +1476,7 @@ impl World { #[inline] pub fn get_by_id(&self, entity: Entity, component_id: ComponentId) -> Option> { self.components().get_info(component_id)?; - // SAFE: entity_location is valid, component_id is valid as checked by the line above + // SAFETY: entity_location is valid, component_id is valid as checked by the line above unsafe { get_component( self, @@ -1497,7 +1499,7 @@ impl World { component_id: ComponentId, ) -> Option> { self.components().get_info(component_id)?; - // SAFE: entity_location is valid, component_id is valid as checked by the line above + // SAFETY: entity_location is valid, component_id is valid as checked by the line above unsafe { get_mut_by_id( self, @@ -1526,7 +1528,9 @@ impl fmt::Debug for World { // TODO: remove allow on lint - https://github.com/bevyengine/bevy/issues/3666 #[allow(clippy::non_send_fields_in_send_ty)] +// SAFETY: all methods on the world ensure that non-send resources are only accessible on the main thread unsafe impl Send for World {} +// SAFETY: all methods on the world ensure that non-send resources are only accessible on the main thread unsafe impl Sync for World {} /// Creates an instance of the type this trait is implemented for @@ -1711,6 +1715,7 @@ mod tests { .unwrap(); let resource = world.get_resource_by_id(component_id).unwrap(); + // SAFETY: `TestResource` is the correct resource type let resource = unsafe { resource.deref::() }; assert_eq!(resource.0, 42); @@ -1728,11 +1733,13 @@ mod tests { { let mut resource = world.get_resource_mut_by_id(component_id).unwrap(); resource.set_changed(); + // SAFETY: `TestResource` is the correct resource type let resource = unsafe { resource.into_inner().deref_mut::() }; resource.0 = 43; } let resource = world.get_resource_by_id(component_id).unwrap(); + // SAFETY: `TestResource` is the correct resource type let resource = unsafe { resource.deref::() }; assert_eq!(resource.0, 43); @@ -1744,7 +1751,7 @@ mod tests { let mut world = World::new(); - // SAFE: the drop function is valid for the layout and the data will be safe to access from any thread + // SAFETY: the drop function is valid for the layout and the data will be safe to access from any thread let descriptor = unsafe { ComponentDescriptor::new_with_layout( "Custom Test Component".to_string(), @@ -1761,11 +1768,14 @@ mod tests { let component_id = world.init_component_with_descriptor(descriptor); let value: [u8; 8] = [0, 1, 2, 3, 4, 5, 6, 7]; - OwningPtr::make(value, |ptr| unsafe { - // SAFE: value is valid for the component layout - world.insert_resource_by_id(component_id, ptr); + OwningPtr::make(value, |ptr| { + // SAFETY: value is valid for the component layout + unsafe { + world.insert_resource_by_id(component_id, ptr); + } }); + // SAFETY: [u8; 8] is the correct type for the resource let data = unsafe { world .get_resource_by_id(component_id) @@ -1785,9 +1795,11 @@ mod tests { let invalid_component_id = ComponentId::new(usize::MAX); let mut world = World::new(); - OwningPtr::make((), |ptr| unsafe { - // SAFE: ptr must be valid for the component_id `invalid_component_id` which is invalid, but checked by `insert_resource_by_id` - world.insert_resource_by_id(invalid_component_id, ptr); + OwningPtr::make((), |ptr| { + // SAFETY: ptr must be valid for the component_id `invalid_component_id` which is invalid, but checked by `insert_resource_by_id` + unsafe { + world.insert_resource_by_id(invalid_component_id, ptr); + } }); } diff --git a/crates/bevy_ecs/src/world/spawn_batch.rs b/crates/bevy_ecs/src/world/spawn_batch.rs index 002e30a2b3..f5e1bd2792 100644 --- a/crates/bevy_ecs/src/world/spawn_batch.rs +++ b/crates/bevy_ecs/src/world/spawn_batch.rs @@ -67,7 +67,7 @@ where fn next(&mut self) -> Option { let bundle = self.inner.next()?; - // SAFE: bundle matches spawner type + // SAFETY: bundle matches spawner type unsafe { Some(self.spawner.spawn(bundle)) } } diff --git a/crates/bevy_ecs/src/world/world_cell.rs b/crates/bevy_ecs/src/world/world_cell.rs index c40a852a02..08571bb6c4 100644 --- a/crates/bevy_ecs/src/world/world_cell.rs +++ b/crates/bevy_ecs/src/world/world_cell.rs @@ -186,7 +186,7 @@ impl<'w> WorldCell<'w> { let resource_archetype = self.world.archetypes.resource(); let archetype_component_id = resource_archetype.get_archetype_component_id(component_id)?; Some(WorldBorrow::new( - // SAFE: ComponentId matches TypeId + // SAFETY: ComponentId matches TypeId unsafe { self.world.get_resource_with_id(component_id)? }, archetype_component_id, self.access.clone(), @@ -218,7 +218,7 @@ impl<'w> WorldCell<'w> { let resource_archetype = self.world.archetypes.resource(); let archetype_component_id = resource_archetype.get_archetype_component_id(component_id)?; Some(WorldBorrowMut::new( - // SAFE: ComponentId matches TypeId and access is checked by WorldBorrowMut + // SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut unsafe { self.world .get_resource_unchecked_mut_with_id(component_id)? @@ -253,7 +253,7 @@ impl<'w> WorldCell<'w> { let resource_archetype = self.world.archetypes.resource(); let archetype_component_id = resource_archetype.get_archetype_component_id(component_id)?; Some(WorldBorrow::new( - // SAFE: ComponentId matches TypeId + // SAFETY: ComponentId matches TypeId unsafe { self.world.get_non_send_with_id(component_id)? }, archetype_component_id, self.access.clone(), @@ -285,7 +285,7 @@ impl<'w> WorldCell<'w> { let resource_archetype = self.world.archetypes.resource(); let archetype_component_id = resource_archetype.get_archetype_component_id(component_id)?; Some(WorldBorrowMut::new( - // SAFE: ComponentId matches TypeId and access is checked by WorldBorrowMut + // SAFETY: ComponentId matches TypeId and access is checked by WorldBorrowMut unsafe { self.world .get_non_send_unchecked_mut_with_id(component_id)? diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 54d65646cf..4d5687c66a 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -345,7 +345,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { let mut builder = WorldChildBuilder { current_entity: None, parent_entities: vec![entity], - // SAFE: self.update_location() is called below. It is impossible to make EntityMut + // SAFETY: self.update_location() is called below. It is impossible to make EntityMut // function calls on `self` within the scope defined here world: unsafe { self.world_mut() }, }; @@ -359,7 +359,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { fn push_children(&mut self, children: &[Entity]) -> &mut Self { let parent = self.id(); { - // SAFE: parent entity is not modified and its location is updated manually + // SAFETY: parent entity is not modified and its location is updated manually let world = unsafe { self.world_mut() }; for child in children.iter() { world @@ -381,7 +381,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self { let parent = self.id(); { - // SAFE: parent entity is not modified and its location is updated manually + // SAFETY: parent entity is not modified and its location is updated manually let world = unsafe { self.world_mut() }; for child in children.iter() { world @@ -403,7 +403,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> { fn remove_children(&mut self, children: &[Entity]) -> &mut Self { let parent = self.id(); - // SAFE: This doesn't change the parent's location + // SAFETY: This doesn't change the parent's location let world = unsafe { self.world_mut() }; for child in children.iter() { let mut child = world.entity_mut(*child); diff --git a/crates/bevy_hierarchy/src/hierarchy.rs b/crates/bevy_hierarchy/src/hierarchy.rs index 4d6753e719..cb4a60597c 100644 --- a/crates/bevy_hierarchy/src/hierarchy.rs +++ b/crates/bevy_hierarchy/src/hierarchy.rs @@ -114,7 +114,7 @@ impl<'w> DespawnRecursiveExt for EntityMut<'w> { ) .entered(); - // SAFE: EntityMut is consumed so even though the location is no longer + // SAFETY: EntityMut is consumed so even though the location is no longer // valid, it cannot be accessed again with the invalid location. unsafe { despawn_with_children_recursive(self.world_mut(), entity); @@ -131,7 +131,7 @@ impl<'w> DespawnRecursiveExt for EntityMut<'w> { ) .entered(); - // SAFE: The location is updated. + // SAFETY: The location is updated. unsafe { despawn_children(self.world_mut(), entity); self.update_location(); diff --git a/crates/bevy_ptr/src/lib.rs b/crates/bevy_ptr/src/lib.rs index e8173f674c..3269221896 100644 --- a/crates/bevy_ptr/src/lib.rs +++ b/crates/bevy_ptr/src/lib.rs @@ -161,6 +161,7 @@ impl<'a> OwningPtr<'a> { #[inline] pub fn make) -> R, R>(val: T, f: F) -> R { let mut temp = MaybeUninit::new(val); + // SAFETY: `temp.as_mut_ptr()` is a reference to a local value on the stack, so it cannot be null let ptr = unsafe { NonNull::new_unchecked(temp.as_mut_ptr().cast::()) }; f(Self(ptr, PhantomData)) } @@ -233,6 +234,7 @@ impl<'a, T> From<&'a [T]> for ThinSlicePtr<'a, T> { #[inline] fn from(slice: &'a [T]) -> Self { Self { + // SAFETY: a reference can never be null ptr: unsafe { NonNull::new_unchecked(slice.as_ptr() as *mut T) }, #[cfg(debug_assertions)] len: slice.len(), diff --git a/crates/bevy_time/src/fixed_timestep.rs b/crates/bevy_time/src/fixed_timestep.rs index d79f4e6ccf..e37ed7de1b 100644 --- a/crates/bevy_time/src/fixed_timestep.rs +++ b/crates/bevy_time/src/fixed_timestep.rs @@ -190,7 +190,7 @@ impl System for FixedTimestep { } unsafe fn run_unsafe(&mut self, _input: (), world: &World) -> ShouldRun { - // SAFE: this system inherits the internal system's component access and archetype component + // SAFETY: this system inherits the internal system's component access and archetype component // access, which means the caller has ensured running the internal system is safe self.internal_system.run_unsafe((), world) } diff --git a/crates/bevy_ui/src/flex/mod.rs b/crates/bevy_ui/src/flex/mod.rs index 8bc4a2fe10..149ecae13c 100644 --- a/crates/bevy_ui/src/flex/mod.rs +++ b/crates/bevy_ui/src/flex/mod.rs @@ -22,7 +22,7 @@ pub struct FlexSurface { taffy: Taffy, } -// SAFE: as long as MeasureFunc is Send + Sync. https://github.com/DioxusLabs/taffy/issues/146 +// SAFETY: as long as MeasureFunc is Send + Sync. https://github.com/DioxusLabs/taffy/issues/146 // TODO: remove allow on lint - https://github.com/bevyengine/bevy/issues/3666 #[allow(clippy::non_send_fields_in_send_ty)] unsafe impl Send for FlexSurface {} diff --git a/crates/bevy_utils/src/futures.rs b/crates/bevy_utils/src/futures.rs index dc0f68d5d1..1b752a33c7 100644 --- a/crates/bevy_utils/src/futures.rs +++ b/crates/bevy_utils/src/futures.rs @@ -29,5 +29,7 @@ fn noop_raw_waker() -> RawWaker { } fn noop_waker() -> Waker { + // SAFETY: the `RawWakerVTable` is just a big noop and doesn't violate any of the rules in `RawWakerVTable`s documentation + // (which talks about retaining and releasing any "resources", of which there are none in this case) unsafe { Waker::from_raw(noop_raw_waker()) } } diff --git a/crates/bevy_window/src/raw_window_handle.rs b/crates/bevy_window/src/raw_window_handle.rs index 5bc122558d..967b3c3301 100644 --- a/crates/bevy_window/src/raw_window_handle.rs +++ b/crates/bevy_window/src/raw_window_handle.rs @@ -24,7 +24,7 @@ impl RawWindowHandleWrapper { } } -// SAFE: RawWindowHandle is just a normal "raw pointer", which doesn't impl Send/Sync. However the pointer is only +// SAFETY: RawWindowHandle is just a normal "raw pointer", which doesn't impl Send/Sync. However the pointer is only // exposed via an unsafe method that forces the user to make a call for a given platform. (ex: some platforms don't // support doing window operations off of the main thread). // A recommendation for this pattern (and more context) is available here: @@ -41,7 +41,7 @@ unsafe impl Sync for RawWindowHandleWrapper {} /// In many cases, this should only be constructed on the main thread. pub struct ThreadLockedRawWindowHandleWrapper(RawWindowHandle); -// SAFE: the caller has validated that this is a valid context to get RawWindowHandle +// SAFETY: the caller has validated that this is a valid context to get RawWindowHandle // as otherwise an instance of this type could not have been constructed // NOTE: we cannot simply impl HasRawWindowHandle for RawWindowHandleWrapper, // as the `raw_window_handle` method is safe. We cannot guarantee that all calls