From f896fecd5ef62438825c81e55ca531bd8acaadcc Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 5 Dec 2022 12:44:43 -0800 Subject: [PATCH 1/3] Document options for !Sync types for Component and Resources --- crates/bevy_ecs/src/component.rs | 27 ++++++++++++++++++++++ crates/bevy_ecs/src/system/system_param.rs | 27 ++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 442fc60ae03a7..9e9b87869eabb 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -110,6 +110,33 @@ use std::{ /// /// [orphan rule]: https://doc.rust-lang.org/book/ch10-02-traits.html#implementing-a-trait-on-a-type /// [newtype pattern]: https://doc.rust-lang.org/book/ch19-03-advanced-traits.html#using-the-newtype-pattern-to-implement-external-traits-on-external-types +/// +/// # `!Sync` Components +/// A `!Sync` type cannot implement `Component`. However, it is possible to wrap a `Send` but not `Sync` +/// type in [`SyncCell`] or the currently unstable [`Exclusive`] to make it `Sync`. This forces only +/// having mutable access (`&mut T` only, never `&T`), but makes it safe to reference across multiple +/// threads. +/// +/// ```compile_fail +/// # use std::cell::RefCell; +/// # use bevy_ecs::component::Component; +/// use bevy_utils::synccell::SyncCell; +/// +/// // This will fail to compile since RefCell is !Sync. +/// #[derive(Component)] +/// struct NotSync { +/// counter: RefCell, +/// } +/// +/// // This will compile. +/// #[derive(Component)] +/// struct ActuallySync { +/// counter: SyncCell>, +/// } +/// ``` +/// +/// [`SyncCell`]: bevy_utils::synccell::SyncCell +/// [`Exclusive`]: https://doc.rust-lang.org/nightly/std/sync/struct.Exclusive.html pub trait Component: Send + Sync + 'static { type Storage: ComponentStorage; } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 4e98e422879cb..5303c5b97f501 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -93,6 +93,33 @@ use std::{ /// /// This will most commonly occur when working with `SystemParam`s generically, as the requirement /// has not been proven to the compiler. +/// +/// # `!Sync` Resources +/// A `!Sync` type cannot implement `Resource`. However, it is possible to wrap a `Send` but not `Sync` +/// type in [`SyncCell`] or the currently unstable [`Exclusive`] to make it `Sync`. This forces only +/// having mutable access (`&mut T` only, never `&T`), but makes it safe to reference across multiple +/// threads. +/// +/// ```compile_fail +/// # use std::cell::RefCell; +/// # use bevy_ecs::system::Resource; +/// use bevy_utils::synccell::SyncCell; +/// +/// // This will fail to compile since RefCell is !Sync. +/// #[derive(Resource)] +/// struct NotSync { +/// counter: RefCell, +/// } +/// +/// // This will compile. +/// #[derive(Resource)] +/// struct ActuallySync { +/// counter: SyncCell>, +/// } +/// ``` +/// +/// [`SyncCell`]: bevy_utils::synccell::SyncCell +/// [`Exclusive`]: https://doc.rust-lang.org/nightly/std/sync/struct.Exclusive.html pub trait SystemParam: Sized { type Fetch: for<'w, 's> SystemParamFetch<'w, 's>; } From 0b1105661051a42216c5b3a15019bceded8e2138 Mon Sep 17 00:00:00 2001 From: James Liu Date: Thu, 8 Dec 2022 18:19:57 -0800 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Atlas Dostal --- crates/bevy_ecs/src/component.rs | 5 +++++ crates/bevy_ecs/src/system/system_param.rs | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 9e9b87869eabb..1ddd444dee19b 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -127,7 +127,12 @@ use std::{ /// struct NotSync { /// counter: RefCell, /// } +/// ``` /// +/// ``` +/// # use std::cell::RefCell; +/// # use bevy_ecs::system::Resource; +/// # use bevy_utils::synccell::SyncCell; /// // This will compile. /// #[derive(Component)] /// struct ActuallySync { diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 5303c5b97f501..2541eb5858c77 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -110,7 +110,12 @@ use std::{ /// struct NotSync { /// counter: RefCell, /// } +/// ``` /// +/// ``` +/// # use std::cell::RefCell; +/// # use bevy_ecs::system::Resource; +/// # use bevy_utils::synccell::SyncCell; /// // This will compile. /// #[derive(Resource)] /// struct ActuallySync { From 1c649dc4d8a863d7213a43c5957311c12a5671fa Mon Sep 17 00:00:00 2001 From: james7132 Date: Thu, 8 Dec 2022 18:35:34 -0800 Subject: [PATCH 3/3] Fix CI --- crates/bevy_ecs/src/component.rs | 10 +++++----- crates/bevy_ecs/src/system/system_param.rs | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/component.rs b/crates/bevy_ecs/src/component.rs index 1ddd444dee19b..39f89b95ffaaf 100644 --- a/crates/bevy_ecs/src/component.rs +++ b/crates/bevy_ecs/src/component.rs @@ -117,22 +117,22 @@ use std::{ /// having mutable access (`&mut T` only, never `&T`), but makes it safe to reference across multiple /// threads. /// +/// This will fail to compile since `RefCell` is `!Sync`. /// ```compile_fail /// # use std::cell::RefCell; /// # use bevy_ecs::component::Component; -/// use bevy_utils::synccell::SyncCell; -/// -/// // This will fail to compile since RefCell is !Sync. /// #[derive(Component)] /// struct NotSync { /// counter: RefCell, /// } /// ``` /// +/// This will compile since the `RefCell` is wrapped with `SyncCell`. /// ``` /// # use std::cell::RefCell; -/// # use bevy_ecs::system::Resource; -/// # use bevy_utils::synccell::SyncCell; +/// # use bevy_ecs::component::Component; +/// use bevy_utils::synccell::SyncCell; +/// /// // This will compile. /// #[derive(Component)] /// struct ActuallySync { diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index b5644327e4e2d..d52961c4ad86a 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -96,23 +96,23 @@ use std::{ /// having mutable access (`&mut T` only, never `&T`), but makes it safe to reference across multiple /// threads. /// +/// This will fail to compile since `RefCell` is `!Sync`. /// ```compile_fail /// # use std::cell::RefCell; /// # use bevy_ecs::system::Resource; -/// use bevy_utils::synccell::SyncCell; /// -/// // This will fail to compile since RefCell is !Sync. /// #[derive(Resource)] /// struct NotSync { /// counter: RefCell, /// } /// ``` /// +/// This will compile since the `RefCell` is wrapped with `SyncCell`. /// ``` /// # use std::cell::RefCell; /// # use bevy_ecs::system::Resource; -/// # use bevy_utils::synccell::SyncCell; -/// // This will compile. +/// use bevy_utils::synccell::SyncCell; +/// /// #[derive(Resource)] /// struct ActuallySync { /// counter: SyncCell>,