Skip to content

Fix ComponentsRegistrator unsoundness #20187

New issue

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

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

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 32 additions & 19 deletions crates/bevy_ecs/src/component/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ use alloc::{boxed::Box, vec::Vec};
use bevy_platform::sync::PoisonError;
use bevy_utils::TypeIdMap;
use core::any::Any;
use core::ops::DerefMut;
use core::{any::TypeId, fmt::Debug, ops::Deref};

use crate::query::DebugCheckedUnwrap as _;
use crate::{
component::{
Component, ComponentDescriptor, ComponentId, Components, RequiredComponents, StorageType,
},
query::DebugCheckedUnwrap as _,
resource::Resource,
};

Expand Down Expand Up @@ -62,8 +61,8 @@ impl ComponentIds {

/// A [`Components`] wrapper that enables additional features, like registration.
pub struct ComponentsRegistrator<'w> {
components: &'w mut Components,
ids: &'w mut ComponentIds,
pub(super) components: &'w mut Components,
pub(super) ids: &'w mut ComponentIds,
}

impl Deref for ComponentsRegistrator<'_> {
Expand All @@ -74,12 +73,6 @@ impl Deref for ComponentsRegistrator<'_> {
}
}

impl DerefMut for ComponentsRegistrator<'_> {
fn deref_mut(&mut self) -> &mut Self::Target {
self.components
}
}

impl<'w> ComponentsRegistrator<'w> {
/// Constructs a new [`ComponentsRegistrator`].
///
Expand Down Expand Up @@ -223,10 +216,11 @@ impl<'w> ComponentsRegistrator<'w> {
) {
// SAFETY: ensured by caller.
unsafe {
self.register_component_inner(id, ComponentDescriptor::new::<T>());
self.components
.register_component_inner(id, ComponentDescriptor::new::<T>());
}
let type_id = TypeId::of::<T>();
let prev = self.indices.insert(type_id, id);
let prev = self.components.indices.insert(type_id, id);
debug_assert!(prev.is_none());

let mut required_components = RequiredComponents::default();
Expand Down Expand Up @@ -272,7 +266,7 @@ impl<'w> ComponentsRegistrator<'w> {
let id = self.ids.next_mut();
// SAFETY: The id is fresh.
unsafe {
self.register_component_inner(id, descriptor);
self.components.register_component_inner(id, descriptor);
}
id
}
Expand Down Expand Up @@ -339,7 +333,8 @@ impl<'w> ComponentsRegistrator<'w> {
let id = self.ids.next_mut();
// SAFETY: The resource is not currently registered, the id is fresh, and the [`ComponentDescriptor`] matches the [`TypeId`]
unsafe {
self.register_resource_unchecked(type_id, id, descriptor());
self.components
.register_resource_unchecked(type_id, id, descriptor());
}
id
}
Expand All @@ -363,10 +358,20 @@ impl<'w> ComponentsRegistrator<'w> {
let id = self.ids.next_mut();
// SAFETY: The id is fresh.
unsafe {
self.register_component_inner(id, descriptor);
self.components.register_component_inner(id, descriptor);
}
id
}

/// Equivalent of `Components::any_queued_mut`
pub fn any_queued_mut(&mut self) -> bool {
self.components.any_queued_mut()
}

/// Equivalent of `Components::any_queued_mut`
pub fn num_queued_mut(&mut self) -> usize {
self.components.num_queued_mut()
}
}

/// A queued component registration.
Expand Down Expand Up @@ -587,7 +592,9 @@ impl<'w> ComponentsQueuedRegistrator<'w> {
self.register_arbitrary_dynamic(descriptor, |registrator, id, descriptor| {
// SAFETY: Id uniqueness handled by caller.
unsafe {
registrator.register_component_inner(id, descriptor);
registrator
.components
.register_component_inner(id, descriptor);
}
})
}
Expand Down Expand Up @@ -616,7 +623,9 @@ impl<'w> ComponentsQueuedRegistrator<'w> {
// SAFETY: Id uniqueness handled by caller, and the type_id matches descriptor.
#[expect(unused_unsafe, reason = "More precise to specify.")]
unsafe {
registrator.register_resource_unchecked(type_id, id, descriptor);
registrator
.components
.register_resource_unchecked(type_id, id, descriptor);
}
},
)
Expand Down Expand Up @@ -648,7 +657,9 @@ impl<'w> ComponentsQueuedRegistrator<'w> {
// SAFETY: Id uniqueness handled by caller, and the type_id matches descriptor.
#[expect(unused_unsafe, reason = "More precise to specify.")]
unsafe {
registrator.register_resource_unchecked(type_id, id, descriptor);
registrator
.components
.register_resource_unchecked(type_id, id, descriptor);
}
},
)
Expand All @@ -672,7 +683,9 @@ impl<'w> ComponentsQueuedRegistrator<'w> {
self.register_arbitrary_dynamic(descriptor, |registrator, id, descriptor| {
// SAFETY: Id uniqueness handled by caller.
unsafe {
registrator.register_component_inner(id, descriptor);
registrator
.components
.register_component_inner(id, descriptor);
}
})
}
Expand Down
15 changes: 8 additions & 7 deletions crates/bevy_ecs/src/component/required.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,14 @@ impl<'w> ComponentsRegistrator<'w> {

// SAFETY: We just created the components.
unsafe {
self.register_required_components_manual_unchecked::<R>(
requiree,
required,
required_components,
constructor,
inheritance_depth,
);
self.components
.register_required_components_manual_unchecked::<R>(
requiree,
required,
required_components,
constructor,
inheritance_depth,
);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
title: ComponentsRegistrator no longer implements DerefMut
pull_requests: [14791, 15458, 15269]
---

`ComponentsRegistrator` no longer implements `DerefMut<Target = Components>`, meaning you won't be able to get a `&mut Components` from it. The only two methods on `Components` that took `&mut self` (`any_queued_mut` and `num_queued_mut`) have been reimplemented on `ComponentsRegistrator`, meaning you won't need to migrate them. Other usages of `&mut Components` were unsupported.
Loading