Skip to content

Commit

Permalink
Make Reflect impls unsafe (Reflect::any must return self) (#1679)
Browse files Browse the repository at this point in the history
Fixes #1100 

Implementors must make sure that `Reflect::any` and `Reflect::any_mut` both return the `self` reference passed in (both for logical correctness and downcast safety).
  • Loading branch information
cart committed Mar 17, 2021
1 parent 107dd73 commit 5fedb60
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 14 deletions.
9 changes: 6 additions & 3 deletions crates/bevy_reflect/bevy_reflect_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ fn impl_struct(
}
}

impl #impl_generics #bevy_reflect_path::Reflect for #struct_name#ty_generics #where_clause {
// SAFE: any and any_mut both return self
unsafe impl #impl_generics #bevy_reflect_path::Reflect for #struct_name#ty_generics #where_clause {
#[inline]
fn type_name(&self) -> &str {
std::any::type_name::<Self>()
Expand Down Expand Up @@ -382,7 +383,8 @@ fn impl_tuple_struct(
}
}

impl #impl_generics #bevy_reflect_path::Reflect for #struct_name#ty_generics {
// SAFE: any and any_mut both return self
unsafe impl #impl_generics #bevy_reflect_path::Reflect for #struct_name#ty_generics {
#[inline]
fn type_name(&self) -> &str {
std::any::type_name::<Self>()
Expand Down Expand Up @@ -457,7 +459,8 @@ fn impl_value(
TokenStream::from(quote! {
#get_type_registration_impl

impl #impl_generics #bevy_reflect_path::Reflect for #type_name#ty_generics #where_clause {
// SAFE: any and any_mut both return self
unsafe impl #impl_generics #bevy_reflect_path::Reflect for #type_name#ty_generics #where_clause {
#[inline]
fn type_name(&self) -> &str {
std::any::type_name::<Self>()
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_reflect/src/impls/smallvec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ where
}
}

impl<T: Array + Send + Sync + 'static> Reflect for SmallVec<T>
// SAFE: any and any_mut both return self
unsafe impl<T: Array + Send + Sync + 'static> Reflect for SmallVec<T>
where
T::Item: Reflect + Clone,
{
Expand Down
9 changes: 6 additions & 3 deletions crates/bevy_reflect/src/impls/std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ impl<T: Reflect> List for Vec<T> {
}
}

impl<T: Reflect> Reflect for Vec<T> {
// SAFE: any and any_mut both return self
unsafe impl<T: Reflect> Reflect for Vec<T> {
fn type_name(&self) -> &str {
std::any::type_name::<Self>()
}
Expand Down Expand Up @@ -160,7 +161,8 @@ impl<K: Reflect + Clone + Eq + Hash, V: Reflect + Clone> Map for HashMap<K, V> {
}
}

impl<K: Reflect + Clone + Eq + Hash, V: Reflect + Clone> Reflect for HashMap<K, V> {
// SAFE: any and any_mut both return self
unsafe impl<K: Reflect + Clone + Eq + Hash, V: Reflect + Clone> Reflect for HashMap<K, V> {
fn type_name(&self) -> &str {
std::any::type_name::<Self>()
}
Expand Down Expand Up @@ -227,7 +229,8 @@ where
}
}

impl Reflect for Cow<'static, str> {
// SAFE: any and any_mut both return self
unsafe impl Reflect for Cow<'static, str> {
fn type_name(&self) -> &str {
std::any::type_name::<Self>()
}
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_reflect/src/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ impl List for DynamicList {
}
}

impl Reflect for DynamicList {
// SAFE: any and any_mut both return self
unsafe impl Reflect for DynamicList {
#[inline]
fn type_name(&self) -> &str {
self.name.as_str()
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_reflect/src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ impl Map for DynamicMap {
}
}

impl Reflect for DynamicMap {
// SAFE: any and any_mut both return self
unsafe impl Reflect for DynamicMap {
fn type_name(&self) -> &str {
&self.name
}
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_reflect/src/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ pub enum ReflectMut<'a> {
}

/// A reflected rust type.
pub trait Reflect: Any + Send + Sync {
///
/// # Safety
/// Implementors _must_ ensure that [Reflect::any] and [Reflect::any_mut] both return the `self` value passed in
/// If this is not done, [Reflect::downcast] will be UB (and also just logically broken).
pub unsafe trait Reflect: Any + Send + Sync {
fn type_name(&self) -> &str;
fn any(&self) -> &dyn Any;
fn any_mut(&mut self) -> &mut dyn Any;
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_reflect/src/struct_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ impl Struct for DynamicStruct {
}
}

impl Reflect for DynamicStruct {
// SAFE: any and any_mut both return self
unsafe impl Reflect for DynamicStruct {
#[inline]
fn type_name(&self) -> &str {
&self.name
Expand Down
6 changes: 4 additions & 2 deletions crates/bevy_reflect/src/tuple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ impl Tuple for DynamicTuple {
}
}

impl Reflect for DynamicTuple {
// SAFE: any and any_mut both return self
unsafe impl Reflect for DynamicTuple {
#[inline]
fn type_name(&self) -> &str {
self.name()
Expand Down Expand Up @@ -274,7 +275,8 @@ macro_rules! impl_reflect_tuple {
}
}

impl<$($name: Reflect),*> Reflect for ($($name,)*) {
// SAFE: any and any_mut both return self
unsafe impl<$($name: Reflect),*> Reflect for ($($name,)*) {
fn type_name(&self) -> &str {
std::any::type_name::<Self>()
}
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_reflect/src/tuple_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ impl TupleStruct for DynamicTupleStruct {
}
}

impl Reflect for DynamicTupleStruct {
// SAFE: any and any_mut both return self
unsafe impl Reflect for DynamicTupleStruct {
#[inline]
fn type_name(&self) -> &str {
self.name.as_str()
Expand Down

0 comments on commit 5fedb60

Please sign in to comment.