Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Merged by Bors] - Remove Sync bound from Local #5483

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 9 additions & 8 deletions crates/bevy_ecs/src/system/system_param.rs
Expand Up @@ -15,6 +15,7 @@ pub use bevy_ecs_macros::Resource;
pub use bevy_ecs_macros::SystemParam;
use bevy_ecs_macros::{all_tuples, impl_param_set};
use bevy_ptr::UnsafeCellDeref;
use bevy_utils::synccell::SyncCell;
use std::{
fmt::Debug,
marker::PhantomData,
Expand Down Expand Up @@ -673,10 +674,10 @@ impl<'w, 's> SystemParamFetch<'w, 's> for WorldState {
/// // .add_system(reset_to_system(my_config))
/// # assert_is_system(reset_to_system(Config(10)));
/// ```
pub struct Local<'a, T: FromWorld + Send + Sync + 'static>(&'a mut T);
pub struct Local<'a, T: FromWorld + Send + 'static>(&'a mut T);

// SAFETY: Local only accesses internal state
unsafe impl<T: Send + Sync + 'static> ReadOnlySystemParamFetch for LocalState<T> {}
unsafe impl<T: Send + 'static> ReadOnlySystemParamFetch for LocalState<T> {}

impl<'a, T: FromWorld + Send + Sync + 'static> Debug for Local<'a, T>
where
Expand Down Expand Up @@ -705,20 +706,20 @@ impl<'a, T: FromWorld + Send + Sync + 'static> DerefMut for Local<'a, T> {

/// The [`SystemParamState`] of [`Local<T>`].
#[doc(hidden)]
pub struct LocalState<T: Send + Sync + 'static>(T);
pub struct LocalState<T: Send + 'static>(SyncCell<T>);

impl<'a, T: Send + Sync + 'static + FromWorld> SystemParam for Local<'a, T> {
impl<'a, T: FromWorld + Send + 'static> SystemParam for Local<'a, T> {
type Fetch = LocalState<T>;
}

// SAFETY: only local state is accessed
unsafe impl<T: FromWorld + Send + Sync + 'static> SystemParamState for LocalState<T> {
unsafe impl<T: FromWorld + Send + 'static> SystemParamState for LocalState<T> {
fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self {
Self(T::from_world(world))
Self(SyncCell::new(T::from_world(world)))
}
}

impl<'w, 's, T: Send + Sync + 'static + FromWorld> SystemParamFetch<'w, 's> for LocalState<T> {
impl<'w, 's, T: FromWorld + Send + 'static> SystemParamFetch<'w, 's> for LocalState<T> {
type Item = Local<'s, T>;

#[inline]
Expand All @@ -728,7 +729,7 @@ impl<'w, 's, T: Send + Sync + 'static + FromWorld> SystemParamFetch<'w, 's> for
_world: &'w World,
_change_tick: u32,
) -> Self::Item {
Local(&mut state.0)
Local(state.0.get())
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/bevy_utils/src/lib.rs
Expand Up @@ -6,6 +6,7 @@ pub mod futures;
pub mod label;
mod short_names;
pub use short_names::get_short_name;
pub mod synccell;

mod default;
mod float_ord;
Expand Down
39 changes: 39 additions & 0 deletions crates/bevy_utils/src/synccell.rs
@@ -0,0 +1,39 @@
/// See [`Exclusive`](https://github.com/rust-lang/rust/issues/98407) for stdlib's upcoming implementation,
/// which should replace this one entirely.
///
/// Provides a wrapper that allows making any type unconditionally [`Sync`] by only providing mutable access.
#[repr(transparent)]
pub struct SyncCell<T: ?Sized> {
inner: T,
}

impl<T: Sized> SyncCell<T> {
/// Construct a new instance of a `SyncCell` from the given value.
pub fn new(inner: T) -> Self {
Self { inner }
}

/// Deconstruct this `SyncCell` into its inner value.
pub fn to_inner(Self { inner }: Self) -> T {
inner
}
}

impl<T: ?Sized> SyncCell<T> {
/// Get a reference to this `SyncCell`'s inner value.
pub fn get(&mut self) -> &mut T {
&mut self.inner
}

/// Build a mutable reference to a `SyncCell` from a mutable reference
/// to its inner value, to skip constructing with [`new()`](SyncCell::new()).
pub fn from_mut(r: &'_ mut T) -> &'_ mut SyncCell<T> {
// SAFETY: repr is transparent, so refs have the same layout; and `SyncCell` properties are `&mut`-agnostic
unsafe { &mut *(r as *mut T as *mut SyncCell<T>) }
}
}

PROMETHIA-27 marked this conversation as resolved.
Show resolved Hide resolved
// SAFETY: `Sync` only allows multithreaded access via immutable reference.
// As `SyncCell` requires an exclusive reference to access the wrapped value,
// marking this type as `Sync` does not actually allow threaded access to the inner value.
unsafe impl<T: ?Sized> Sync for SyncCell<T> {}