Skip to content

Remove Send + Sync bounds from PartialReflect and friends (and move the constraint to usages thereof) #16993

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 3 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
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ keywords = ["game", "engine", "gamedev", "graphics", "bevy"]
license = "MIT OR Apache-2.0"
repository = "https://github.com/bevyengine/bevy"
documentation = "https://docs.rs/bevy"
rust-version = "1.85.0"
rust-version = "1.86.0"

[workspace]
resolver = "2"
Expand Down
2 changes: 1 addition & 1 deletion benches/benches/bevy_reflect/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fn list_apply<M, LBase, LPatch, F1, F2, F3>(
) where
M: Measurement,
LBase: List,
LPatch: List,
LPatch: List + Send + Sync,
F1: Fn(usize) -> F2,
F2: Fn() -> LBase,
F3: Fn(usize) -> LPatch,
Expand Down
2 changes: 1 addition & 1 deletion benches/benches/bevy_reflect/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ fn map_apply<M, MBase, MPatch, F1, F2, F3>(
) where
M: Measurement,
MBase: Map,
MPatch: Map,
MPatch: Map + Send + Sync,
F1: Fn(usize) -> F2,
F2: Fn() -> MBase,
F3: Fn(usize) -> MPatch,
Expand Down
14 changes: 7 additions & 7 deletions benches/benches/bevy_reflect/struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ fn concrete_struct_apply(criterion: &mut Criterion) {

// Use functions that produce trait objects of varying concrete types as the
// input to the benchmark.
let inputs: &[fn() -> (Box<dyn Struct>, Box<dyn PartialReflect>)] = &[
let inputs: &[fn() -> (Box<dyn Struct>, Box<dyn PartialReflect + Send + Sync>)] = &[
|| (Box::new(Struct16::default()), Box::new(Struct16::default())),
|| (Box::new(Struct32::default()), Box::new(Struct32::default())),
|| (Box::new(Struct64::default()), Box::new(Struct64::default())),
Expand Down Expand Up @@ -243,7 +243,7 @@ fn dynamic_struct_to_dynamic_struct(criterion: &mut Criterion) {
fn dynamic_struct_apply(criterion: &mut Criterion) {
let mut group = create_group(criterion, bench!("dynamic_struct_apply"));

let patches: &[(fn() -> Box<dyn PartialReflect>, usize)] = &[
let patches: &[(fn() -> Box<dyn PartialReflect + Send + Sync>, usize)] = &[
(|| Box::new(Struct16::default()), 16),
(|| Box::new(Struct32::default()), 32),
(|| Box::new(Struct64::default()), 64),
Expand Down Expand Up @@ -612,12 +612,12 @@ struct Struct128 {
}

#[derive(Clone, Default, Reflect)]
struct GenericStruct1<T: Reflect + Default> {
struct GenericStruct1<T: Reflect + Send + Sync + Default> {
field_0: T,
}

#[derive(Clone, Default, Reflect)]
struct GenericStruct16<T: Reflect + Default> {
struct GenericStruct16<T: Reflect + Send + Sync + Default> {
field_0: T,
field_1: T,
field_2: T,
Expand All @@ -637,7 +637,7 @@ struct GenericStruct16<T: Reflect + Default> {
}

#[derive(Clone, Default, Reflect)]
struct GenericStruct32<T: Reflect + Default> {
struct GenericStruct32<T: Reflect + Send + Sync + Default> {
field_0: T,
field_1: T,
field_2: T,
Expand Down Expand Up @@ -673,7 +673,7 @@ struct GenericStruct32<T: Reflect + Default> {
}

#[derive(Clone, Default, Reflect)]
struct GenericStruct64<T: Reflect + Default> {
struct GenericStruct64<T: Reflect + Send + Sync + Default> {
field_0: T,
field_1: T,
field_2: T,
Expand Down Expand Up @@ -741,7 +741,7 @@ struct GenericStruct64<T: Reflect + Default> {
}

#[derive(Clone, Default, Reflect)]
struct GenericStruct128<T: Reflect + Default> {
struct GenericStruct128<T: Reflect + Send + Sync + Default> {
field_0: T,
field_1: T,
field_2: T,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_animation/src/animation_curves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ where
impl<P: Send + Sync + 'static, C> AnimationCurve for AnimatableCurve<P, C>
where
P: AnimatableProperty + Clone,
C: AnimationCompatibleCurve<P::Property> + Clone,
C: AnimationCompatibleCurve<P::Property> + Clone + Send + Sync,
{
fn clone_value(&self) -> Box<dyn AnimationCurve> {
Box::new(self.clone())
Expand Down Expand Up @@ -469,7 +469,7 @@ struct WeightsCurveEvaluator {

impl<C> AnimationCurve for WeightsCurve<C>
where
C: IterableCurve<f32> + Debug + Clone + Reflectable,
C: IterableCurve<f32> + Debug + Clone + Reflectable + Send + Sync,
{
fn clone_value(&self) -> Box<dyn AnimationCurve> {
Box::new(self.clone())
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ impl App {
/// See [`bevy_reflect::TypeRegistry::register_type_data`].
#[cfg(feature = "bevy_reflect")]
pub fn register_type_data<
T: bevy_reflect::Reflect + bevy_reflect::TypePath,
T: bevy_reflect::Reflect + Send + Sync + bevy_reflect::TypePath,
D: bevy_reflect::TypeData + bevy_reflect::FromType<T>,
>(
&mut self,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_app/src/sub_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl SubApp {
/// See [`App::register_type_data`].
#[cfg(feature = "bevy_reflect")]
pub fn register_type_data<
T: bevy_reflect::Reflect + bevy_reflect::TypePath,
T: bevy_reflect::Reflect + Send + Sync + bevy_reflect::TypePath,
D: bevy_reflect::TypeData + bevy_reflect::FromType<T>,
>(
&mut self,
Expand Down
7 changes: 4 additions & 3 deletions crates/bevy_asset/src/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,16 +691,17 @@ mod tests {
"Inserting the asset should result in a strong count of 1"
);

let reflected: &dyn Reflect = &handle;
let _cloned_handle: Box<dyn Reflect> = reflected.reflect_clone().unwrap();
let reflected: &(dyn Reflect + Send + Sync) = &handle;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the bounds necessary here?

let _cloned_handle: Box<dyn Reflect + Send + Sync> =
reflected.reflect_clone().unwrap();

assert_eq!(
Arc::strong_count(strong),
2,
"Cloning the handle with reflect should increase the strong count to 2"
);

let dynamic_handle: Box<dyn PartialReflect> = reflected.to_dynamic();
let dynamic_handle: Box<dyn PartialReflect + Send + Sync> = reflected.to_dynamic();

assert_eq!(
Arc::strong_count(strong),
Expand Down
56 changes: 37 additions & 19 deletions crates/bevy_asset/src/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@ pub struct ReflectAsset {
handle_type_id: TypeId,
assets_resource_type_id: TypeId,

get: fn(&World, UntypedHandle) -> Option<&dyn Reflect>,
get: fn(&World, UntypedHandle) -> Option<&(dyn Reflect + Send + Sync)>,
// SAFETY:
// - may only be called with an [`UnsafeWorldCell`] which can be used to access the corresponding `Assets<T>` resource mutably
// - may only be used to access **at most one** access at once
get_unchecked_mut: unsafe fn(UnsafeWorldCell<'_>, UntypedHandle) -> Option<&mut dyn Reflect>,
add: fn(&mut World, &dyn PartialReflect) -> UntypedHandle,
insert: fn(&mut World, UntypedHandle, &dyn PartialReflect),
get_unchecked_mut:
unsafe fn(UnsafeWorldCell<'_>, UntypedHandle) -> Option<&mut (dyn Reflect + Send + Sync)>,
add: fn(&mut World, &(dyn PartialReflect + Send + Sync)) -> UntypedHandle,
insert: fn(&mut World, UntypedHandle, &(dyn PartialReflect + Send + Sync)),
len: fn(&World) -> usize,
ids: for<'w> fn(&'w World) -> Box<dyn Iterator<Item = UntypedAssetId> + 'w>,
remove: fn(&mut World, UntypedHandle) -> Option<Box<dyn Reflect>>,
remove: fn(&mut World, UntypedHandle) -> Option<Box<dyn Reflect + Send + Sync>>,
}

impl ReflectAsset {
Expand All @@ -42,7 +43,11 @@ impl ReflectAsset {
}

/// Equivalent of [`Assets::get`]
pub fn get<'w>(&self, world: &'w World, handle: UntypedHandle) -> Option<&'w dyn Reflect> {
pub fn get<'w>(
&self,
world: &'w World,
handle: UntypedHandle,
) -> Option<&'w (dyn Reflect + Send + Sync)> {
(self.get)(world, handle)
}

Expand All @@ -51,7 +56,7 @@ impl ReflectAsset {
&self,
world: &'w mut World,
handle: UntypedHandle,
) -> Option<&'w mut dyn Reflect> {
) -> Option<&'w mut (dyn Reflect + Send + Sync)> {
// SAFETY: unique world access
#[expect(
unsafe_code,
Expand Down Expand Up @@ -97,22 +102,35 @@ impl ReflectAsset {
&self,
world: UnsafeWorldCell<'w>,
handle: UntypedHandle,
) -> Option<&'w mut dyn Reflect> {
) -> Option<&'w mut (dyn Reflect + Send + Sync)> {
// SAFETY: requirements are deferred to the caller
unsafe { (self.get_unchecked_mut)(world, handle) }
}

/// Equivalent of [`Assets::add`]
pub fn add(&self, world: &mut World, value: &dyn PartialReflect) -> UntypedHandle {
pub fn add(
&self,
world: &mut World,
value: &(dyn PartialReflect + Send + Sync),
) -> UntypedHandle {
(self.add)(world, value)
}
/// Equivalent of [`Assets::insert`]
pub fn insert(&self, world: &mut World, handle: UntypedHandle, value: &dyn PartialReflect) {
pub fn insert(
&self,
world: &mut World,
handle: UntypedHandle,
value: &(dyn PartialReflect + Send + Sync),
) {
(self.insert)(world, handle, value);
}

/// Equivalent of [`Assets::remove`]
pub fn remove(&self, world: &mut World, handle: UntypedHandle) -> Option<Box<dyn Reflect>> {
pub fn remove(
&self,
world: &mut World,
handle: UntypedHandle,
) -> Option<Box<dyn Reflect + Send + Sync>> {
(self.remove)(world, handle)
}

Expand Down Expand Up @@ -140,15 +158,15 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
get: |world, handle| {
let assets = world.resource::<Assets<A>>();
let asset = assets.get(&handle.typed_debug_checked());
asset.map(|asset| asset as &dyn Reflect)
asset.map(|asset| asset as &(dyn Reflect + Send + Sync))
},
get_unchecked_mut: |world, handle| {
// SAFETY: `get_unchecked_mut` must be called with `UnsafeWorldCell` having access to `Assets<A>`,
// and must ensure to only have at most one reference to it live at all times.
#[expect(unsafe_code, reason = "Uses `UnsafeWorldCell::get_resource_mut()`.")]
let assets = unsafe { world.get_resource_mut::<Assets<A>>().unwrap().into_inner() };
let asset = assets.get_mut(&handle.typed_debug_checked());
asset.map(|asset| asset as &mut dyn Reflect)
asset.map(|asset| asset as &mut (dyn Reflect + Send + Sync))
},
add: |world, value| {
let mut assets = world.resource_mut::<Assets<A>>();
Expand All @@ -173,15 +191,15 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
remove: |world, handle| {
let mut assets = world.resource_mut::<Assets<A>>();
let value = assets.remove(&handle.typed_debug_checked());
value.map(|value| Box::new(value) as Box<dyn Reflect>)
value.map(|value| Box::new(value) as Box<dyn Reflect + Send + Sync>)
},
}
}
}

/// Reflect type data struct relating a [`Handle<T>`] back to the `T` asset type.
///
/// Say you want to look up the asset values of a list of handles when you have access to their `&dyn Reflect` form.
/// Say you want to look up the asset values of a list of handles when you have access to their `&(dyn Reflect + Send + Sync)` form.
/// Assets can be looked up in the world using [`ReflectAsset`], but how do you determine which [`ReflectAsset`] to use when
/// only looking at the handle? [`ReflectHandle`] is stored in the type registry on each `Handle<T>` type, so you can use [`ReflectHandle::asset_type_id`] to look up
/// the [`ReflectAsset`] type data on the corresponding `T` asset type:
Expand All @@ -194,7 +212,7 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
///
/// # let world: &World = unimplemented!();
/// # let type_registry: TypeRegistry = unimplemented!();
/// let handles: Vec<&dyn Reflect> = unimplemented!();
/// let handles: Vec<&(dyn Reflect + Send + Sync)> = unimplemented!();
/// for handle in handles {
/// let reflect_handle = type_registry.get_type_data::<ReflectHandle>(handle.type_id()).unwrap();
/// let reflect_asset = type_registry.get_type_data::<ReflectAsset>(reflect_handle.asset_type_id()).unwrap();
Expand All @@ -208,7 +226,7 @@ impl<A: Asset + FromReflect> FromType<A> for ReflectAsset {
pub struct ReflectHandle {
asset_type_id: TypeId,
downcast_handle_untyped: fn(&dyn Any) -> Option<UntypedHandle>,
typed: fn(UntypedHandle) -> Box<dyn Reflect>,
typed: fn(UntypedHandle) -> Box<dyn Reflect + Send + Sync>,
}
impl ReflectHandle {
/// The [`TypeId`] of the asset
Expand All @@ -221,9 +239,9 @@ impl ReflectHandle {
(self.downcast_handle_untyped)(handle)
}

/// A way to go from a [`UntypedHandle`] to a [`Handle<T>`] in a `Box<dyn Reflect>`.
/// A way to go from a [`UntypedHandle`] to a [`Handle<T>`] in a `Box<dyn Reflect + Send + Sync>`.
/// Equivalent of [`UntypedHandle::typed`].
pub fn typed(&self, handle: UntypedHandle) -> Box<dyn Reflect> {
pub fn typed(&self, handle: UntypedHandle) -> Box<dyn Reflect + Send + Sync> {
(self.typed)(handle)
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_color/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ homepage = "https://bevyengine.org"
repository = "https://github.com/bevyengine/bevy"
license = "MIT OR Apache-2.0"
keywords = ["bevy", "color"]
rust-version = "1.85.0"
rust-version = "1.86.0"

[dependencies]
bevy_math = { path = "../bevy_math", version = "0.16.0-dev", default-features = false, features = [
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_dev_tools/src/picking_debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ pub fn log_event_debug<E: Event + Debug>(mut events: EventReader<pointer::Pointe
}

/// Listens for pointer events of type `E` and logs them at "debug" level
pub fn log_pointer_event_debug<E: Debug + Clone + Reflect>(
pub fn log_pointer_event_debug<E: Debug + Clone + Reflect + Send + Sync>(
mut pointer_events: EventReader<Pointer<E>>,
) {
for event in pointer_events.read() {
Expand All @@ -138,7 +138,7 @@ pub fn log_pointer_event_debug<E: Debug + Clone + Reflect>(
}

/// Listens for pointer events of type `E` and logs them at "trace" level
pub fn log_pointer_event_trace<E: Debug + Clone + Reflect>(
pub fn log_pointer_event_trace<E: Debug + Clone + Reflect + Send + Sync>(
mut pointer_events: EventReader<Pointer<E>>,
) {
for event in pointer_events.read() {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ repository = "https://github.com/bevyengine/bevy"
license = "MIT OR Apache-2.0"
keywords = ["ecs", "game", "bevy"]
categories = ["game-engines", "data-structures"]
rust-version = "1.85.0"
rust-version = "1.86.0"

[features]
default = ["std", "bevy_reflect", "async_executor", "backtrace"]
Expand Down
9 changes: 6 additions & 3 deletions crates/bevy_ecs/src/entity/clone_entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ impl<'a> SourceComponent<'a> {
self.ptr
}

/// Returns a reference to the component on the source entity as [`&dyn Reflect`](bevy_reflect::Reflect).
/// Returns a reference to the component on the source entity as [`&(dyn Reflect + Send + Sync)`](bevy_reflect::Reflect).
///
/// Will return `None` if:
/// - World does not have [`AppTypeRegistry`](`crate::reflect::AppTypeRegistry`).
Expand All @@ -55,7 +55,7 @@ impl<'a> SourceComponent<'a> {
pub fn read_reflect(
&self,
registry: &bevy_reflect::TypeRegistry,
) -> Option<&dyn bevy_reflect::Reflect> {
) -> Option<&(dyn bevy_reflect::Reflect + Send + Sync)> {
let type_id = self.info.type_id()?;
let reflect_from_ptr = registry.get_type_data::<bevy_reflect::ReflectFromPtr>(type_id)?;
if reflect_from_ptr.type_id() != type_id {
Expand Down Expand Up @@ -219,7 +219,10 @@ impl<'a, 'b> ComponentCloneCtx<'a, 'b> {
/// - Passed component's [`TypeId`] does not match source component [`TypeId`].
/// - Component has already been written once.
#[cfg(feature = "bevy_reflect")]
pub fn write_target_component_reflect(&mut self, component: Box<dyn bevy_reflect::Reflect>) {
pub fn write_target_component_reflect(
&mut self,
component: Box<dyn bevy_reflect::Reflect + Send + Sync>,
) {
if self.target_component_written {
panic!("Trying to write component multiple times")
}
Expand Down
Loading