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

Update from_reflect to return a Result #7075

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
f895ce8
Update `from_reflect` to return a Result
elbertronnie Jan 2, 2023
0d73862
Added `ReflectKind` and Added more elaborate errors for `FromReflectE…
elbertronnie Jan 14, 2023
7888122
Resolve merge conflicts with 229d6c6
elbertronnie Jan 14, 2023
6dd0e45
Merge branch 'main' into return-result
elbertronnie Jan 14, 2023
2d46959
Fix compilation issue
elbertronnie Jan 14, 2023
78e7adb
Fix spelling error
elbertronnie Jan 14, 2023
861a5fd
Make `take_from_reflect` return `FromReflectError` also
elbertronnie Jan 18, 2023
271a18e
Remove panics in `FromReflect` impl for `Option<T>`
elbertronnie Jan 18, 2023
aa0b3c6
Remove `Dynamic` from `ReflectKind`
elbertronnie Jan 19, 2023
695796b
Add `PartialEq`, `Eq` and `Hash` to `ReflectKind`
elbertronnie Jan 19, 2023
5b7bc14
Update `display_from_kind` to return `&str` instead of `String`
elbertronnie Jan 19, 2023
4ba849e
Add period at the end in docs
elbertronnie Jan 19, 2023
8c46495
Change `variant`'s type to `Cow<'static, str>`
elbertronnie Jan 20, 2023
c7254ce
Added description of "kind" in `ReflectKind`
elbertronnie Jan 21, 2023
5f814aa
Update docs to be specific to it's value
elbertronnie Jan 21, 2023
b6f3c5d
Move `FromReflectError` to from_reflect.rs
elbertronnie Jan 21, 2023
2e528a7
Changed Error names to include NamedField and UnnamedField
elbertronnie Jan 21, 2023
8fc5732
Add docs to variants of `FromReflectError`
elbertronnie Jan 21, 2023
c5370d4
Add `IndexError` in `from_reflect` of `SmallVec`
elbertronnie Jan 21, 2023
7e1d30f
Rename `InvalidSize` to `InvalidLength` and cargo fmt
elbertronnie Jan 21, 2023
53ace23
Added tests for FromReflectError
elbertronnie Jan 22, 2023
7974319
Tuple should have fields instead of index
elbertronnie Jan 22, 2023
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
54 changes: 49 additions & 5 deletions crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::fq_std::FQDefault;
use crate::fq_std::{FQBox, FQDefault};
use crate::{
derive_data::{EnumVariantFields, ReflectEnum},
utility::ident_or_index,
Expand Down Expand Up @@ -53,18 +53,62 @@ pub(crate) fn get_variant_constructors(
);
quote!(.expect(#type_err_message))
} else {
quote!(?)
match &field.data.ident {
Some(ident) => {
let name = ident.to_string();
quote!(.map_err(|err| #bevy_reflect_path::FromReflectError::NamedFieldError {
from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
field: #name,
source: #FQBox::new(err),
})?)
},
None => quote!(.map_err(|err| #bevy_reflect_path::FromReflectError::UnnamedFieldError {
from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
index: #reflect_index,
source: #FQBox::new(err),
})?)
}
};
let field_accessor = match &field.data.ident {
Some(ident) => {
let name = ident.to_string();
quote!(.field(#name))
if can_panic {
quote!(.field(#name))
} else {
quote!(.field(#name)
.ok_or_else(|| #bevy_reflect_path::FromReflectError::MissingNamedField {
from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
field: #name,
})
)
}
}
None => if can_panic {
quote!(.field_at(#reflect_index))
} else {
quote!(.field_at(#reflect_index)
.ok_or_else(|| #bevy_reflect_path::FromReflectError::MissingUnnamedField {
from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
index: #reflect_index,
})
)
}
None => quote!(.field_at(#reflect_index)),
};
reflect_index += 1;
let missing_field_err_message = format!("the field {error_repr} was not declared");
let accessor = quote!(#field_accessor .expect(#missing_field_err_message));
let accessor = if can_panic {
quote!(#field_accessor .expect(#missing_field_err_message))
} else {
quote!(#field_accessor?)
};
quote! {
#bevy_reflect_path::FromReflect::from_reflect(#ref_value #accessor)
#unwrapper
Expand Down
7 changes: 7 additions & 0 deletions crates/bevy_reflect/bevy_reflect_derive/src/fq_std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use quote::{quote, ToTokens};
pub(crate) struct FQAny;
pub(crate) struct FQBox;
pub(crate) struct FQClone;
pub(crate) struct FQCow;
pub(crate) struct FQDefault;
pub(crate) struct FQOption;
pub(crate) struct FQResult;
Expand All @@ -58,6 +59,12 @@ impl ToTokens for FQClone {
}
}

impl ToTokens for FQCow {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::std::borrow::Cow).to_tokens(tokens);
}
}

impl ToTokens for FQDefault {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::core::default::Default).to_tokens(tokens);
Expand Down
110 changes: 88 additions & 22 deletions crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::container_attributes::REFLECT_DEFAULT;
use crate::derive_data::ReflectEnum;
use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors};
use crate::field_attributes::DefaultBehavior;
use crate::fq_std::{FQAny, FQClone, FQDefault, FQOption};
use crate::fq_std::{FQAny, FQBox, FQClone, FQCow, FQDefault, FQResult};
use crate::{ReflectMeta, ReflectStruct};
use proc_macro::TokenStream;
use proc_macro2::Span;
Expand All @@ -26,16 +26,25 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream {
let (impl_generics, ty_generics, where_clause) = meta.generics().split_for_impl();
TokenStream::from(quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_clause {
fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQOption<Self> {
#FQOption::Some(#FQClone::clone(<dyn #FQAny>::downcast_ref::<#type_name #ty_generics>(<dyn #bevy_reflect_path::Reflect>::as_any(reflect))?))
fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQResult<Self, #bevy_reflect_path::FromReflectError> {
#FQResult::Ok(#FQClone::clone(
<dyn #FQAny>::downcast_ref::<#type_name #ty_generics>(<dyn #bevy_reflect_path::Reflect>::as_any(reflect))
.ok_or_else(|| #bevy_reflect_path::FromReflectError::InvalidType {
from_type: #bevy_reflect_path::Reflect::get_type_info(reflect),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
})?
))
}
}
})
}

/// Implements `FromReflect` for the given enum type
pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
let fqoption = FQOption.into_token_stream();
let fqresult = FQResult.into_token_stream();
let fqbox = FQBox.into_token_stream();
let fqcow = FQCow.into_token_stream();

let type_name = reflect_enum.meta().type_name();
let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path();
Expand All @@ -50,14 +59,31 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
reflect_enum.meta().generics().split_for_impl();
TokenStream::from(quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_clause {
fn from_reflect(#ref_value: &dyn #bevy_reflect_path::Reflect) -> #FQOption<Self> {
fn from_reflect(#ref_value: &dyn #bevy_reflect_path::Reflect) -> #FQResult<Self, #bevy_reflect_path::FromReflectError> {
if let #bevy_reflect_path::ReflectRef::Enum(#ref_value) = #bevy_reflect_path::Reflect::reflect_ref(#ref_value) {
match #bevy_reflect_path::Enum::variant_name(#ref_value) {
#(#variant_names => #fqoption::Some(#variant_constructors),)*
name => panic!("variant with name `{}` does not exist on enum `{}`", name, ::core::any::type_name::<Self>()),
#(#variant_names => (|| #fqresult::Ok(#variant_constructors))().map_err(|err| {
#bevy_reflect_path::FromReflectError::VariantError {
from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
variant: #fqcow::Borrowed(#variant_names),
source: #fqbox::new(err),
}
}),)*
name => #FQResult::Err(#bevy_reflect_path::FromReflectError::MissingVariant {
from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
variant: #fqcow::Owned(name.to_string()),
}),
}
} else {
#FQOption::None
#FQResult::Err(#bevy_reflect_path::FromReflectError::InvalidType {
from_type: #bevy_reflect_path::Reflect::get_type_info(#ref_value),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(#ref_value),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
})
}
}
}
Expand All @@ -75,7 +101,7 @@ impl MemberValuePair {
}

fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> TokenStream {
let fqoption = FQOption.into_token_stream();
let fqresult = FQResult.into_token_stream();

let struct_name = reflect_struct.meta().type_name();
let generics = reflect_struct.meta().generics();
Expand All @@ -92,23 +118,29 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
let MemberValuePair(active_members, active_values) =
get_active_fields(reflect_struct, &ref_struct, &ref_struct_type, is_tuple);

let error = quote!(#bevy_reflect_path::FromReflectError::InvalidType {
from_type: #bevy_reflect_path::Reflect::get_type_info(reflect),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
});

let constructor = if reflect_struct.meta().traits().contains(REFLECT_DEFAULT) {
quote!(
let mut __this: Self = #FQDefault::default();
#(
if let #fqoption::Some(__field) = #active_values() {
if let #fqresult::Ok(__field) = #active_values() {
// Iff field exists -> use its value
__this.#active_members = __field;
}
)*
#FQOption::Some(__this)
#FQResult::Ok(__this)
)
} else {
let MemberValuePair(ignored_members, ignored_values) =
get_ignored_fields(reflect_struct, is_tuple);

quote!(
#FQOption::Some(
#FQResult::Ok(
Self {
#(#active_members: #active_values()?,)*
#(#ignored_members: #ignored_values,)*
Expand All @@ -134,11 +166,11 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
TokenStream::from(quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #struct_name #ty_generics #where_from_reflect_clause
{
fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQOption<Self> {
fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQResult<Self, #bevy_reflect_path::FromReflectError> {
if let #bevy_reflect_path::ReflectRef::#ref_struct_type(#ref_struct) = #bevy_reflect_path::Reflect::reflect_ref(reflect) {
#constructor
} else {
#FQOption::None
#FQResult::Err(#error)
}
}
}
Expand Down Expand Up @@ -187,31 +219,65 @@ fn get_active_fields(
let accessor = get_field_accessor(field.data, field.index, is_tuple);
let ty = field.data.ty.clone();

let missing_error = if is_tuple {
quote!(|| #bevy_reflect_path::FromReflectError::MissingUnnamedField {
from_type: #bevy_reflect_path::Reflect::get_type_info(reflect),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
index: #accessor,
})
} else {
quote!(|| #bevy_reflect_path::FromReflectError::MissingNamedField {
from_type: #bevy_reflect_path::Reflect::get_type_info(reflect),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
field: #accessor,
})
};

let get_field = quote! {
#bevy_reflect_path::#struct_type::field(#dyn_struct_name, #accessor)
#bevy_reflect_path::#struct_type::field(#dyn_struct_name, #accessor).ok_or_else(#missing_error)
};

let error = if is_tuple {
quote!(|err| #bevy_reflect_path::FromReflectError::UnnamedFieldError {
from_type: #bevy_reflect_path::Reflect::get_type_info(reflect),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
index: #accessor,
source: #FQBox::new(err),
})
} else {
quote!(|err| #bevy_reflect_path::FromReflectError::NamedFieldError {
from_type: #bevy_reflect_path::Reflect::get_type_info(reflect),
from_kind: #bevy_reflect_path::Reflect::reflect_kind(reflect),
to_type: <Self as #bevy_reflect_path::Typed>::type_info(),
field: #accessor,
source: #FQBox::new(err),
})
};

let value = match &field.attrs.default {
DefaultBehavior::Func(path) => quote! {
(||
if let #FQOption::Some(field) = #get_field {
<#ty as #bevy_reflect_path::FromReflect>::from_reflect(field)
if let #FQResult::Ok(field) = #get_field {
<#ty as #bevy_reflect_path::FromReflect>::from_reflect(field).map_err(#error)
} else {
#FQOption::Some(#path())
#FQResult::Ok(#path())
}
)
},
DefaultBehavior::Default => quote! {
(||
if let #FQOption::Some(field) = #get_field {
<#ty as #bevy_reflect_path::FromReflect>::from_reflect(field)
if let #FQResult::Ok(field) = #get_field {
<#ty as #bevy_reflect_path::FromReflect>::from_reflect(field).map_err(#error)
} else {
#FQOption::Some(#FQDefault::default())
#FQResult::Ok(#FQDefault::default())
}
)
},
DefaultBehavior::Required => quote! {
(|| <#ty as #bevy_reflect_path::FromReflect>::from_reflect(#get_field?))
(|| <#ty as #bevy_reflect_path::FromReflect>::from_reflect(#get_field?).map_err(#error))
},
};

Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
}
}

fn reflect_kind(&self) -> #bevy_reflect_path::ReflectKind {
#bevy_reflect_path::ReflectKind::Enum
}

fn reflect_ref(&self) -> #bevy_reflect_path::ReflectRef {
#bevy_reflect_path::ReflectRef::Enum(self)
}
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,10 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream {
}
}

fn reflect_kind(&self) -> #bevy_reflect_path::ReflectKind {
#bevy_reflect_path::ReflectKind::Struct
}

fn reflect_ref(&self) -> #bevy_reflect_path::ReflectRef {
#bevy_reflect_path::ReflectRef::Struct(self)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream {
}
}

fn reflect_kind(&self) -> #bevy_reflect_path::ReflectKind {
#bevy_reflect_path::ReflectKind::TupleStruct
}

fn reflect_ref(&self) -> #bevy_reflect_path::ReflectRef {
#bevy_reflect_path::ReflectRef::TupleStruct(self)
}
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream {
#FQResult::Ok(())
}

fn reflect_kind(&self) -> #bevy_reflect_path::ReflectKind {
#bevy_reflect_path::ReflectKind::Value
}

fn reflect_ref(&self) -> #bevy_reflect_path::ReflectRef {
#bevy_reflect_path::ReflectRef::Value(self)
}
Expand Down
9 changes: 7 additions & 2 deletions crates/bevy_reflect/src/array.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
utility::NonGenericTypeInfoCell, DynamicInfo, Reflect, ReflectMut, ReflectOwned, ReflectRef,
TypeInfo, Typed,
utility::NonGenericTypeInfoCell, DynamicInfo, Reflect, ReflectKind, ReflectMut, ReflectOwned,
ReflectRef, TypeInfo, Typed,
};
use std::{
any::{Any, TypeId},
Expand Down Expand Up @@ -222,6 +222,11 @@ impl Reflect for DynamicArray {
Ok(())
}

#[inline]
fn reflect_kind(&self) -> ReflectKind {
ReflectKind::Array
}

#[inline]
fn reflect_ref(&self) -> ReflectRef {
ReflectRef::Array(self)
Expand Down
7 changes: 6 additions & 1 deletion crates/bevy_reflect/src/enums/dynamic_enum.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::utility::NonGenericTypeInfoCell;
use crate::{
enum_debug, enum_hash, enum_partial_eq, DynamicInfo, DynamicStruct, DynamicTuple, Enum,
Reflect, ReflectMut, ReflectOwned, ReflectRef, Struct, Tuple, TypeInfo, Typed,
Reflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, Struct, Tuple, TypeInfo, Typed,
VariantFieldIter, VariantType,
};
use std::any::Any;
Expand Down Expand Up @@ -386,6 +386,11 @@ impl Reflect for DynamicEnum {
Ok(())
}

#[inline]
fn reflect_kind(&self) -> ReflectKind {
ReflectKind::Enum
}

#[inline]
fn reflect_ref(&self) -> ReflectRef {
ReflectRef::Enum(self)
Expand Down
Loading