From 95cb9e37f4098c0a284b36f79b18a291f0dad6b6 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 4 Jun 2023 13:49:58 -0700 Subject: [PATCH] Fixed bounds for normal `FromReflect` derive --- .../src/container_attributes.rs | 16 +++++++++++++--- .../bevy_reflect_derive/src/derive_data.rs | 7 ++++++- .../bevy_reflect/bevy_reflect_derive/src/lib.rs | 8 ++++---- .../tests/reflect_derive/from_reflect.fail.rs | 8 +++++++- .../reflect_derive/from_reflect.fail.stderr | 10 ++++++++++ .../tests/reflect_derive/from_reflect.pass.rs | 8 +++++++- examples/reflection/reflection.rs | 2 +- 7 files changed, 48 insertions(+), 11 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs index a21851618597b9..a7acbb2748d90e 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs @@ -170,7 +170,10 @@ pub(crate) struct ReflectTraits { } impl ReflectTraits { - pub fn from_metas(metas: Punctuated) -> Result { + pub fn from_metas( + metas: Punctuated, + is_from_reflect_derive: bool, + ) -> Result { let mut traits = ReflectTraits::default(); for meta in &metas { match meta { @@ -245,7 +248,14 @@ impl ReflectTraits { syn::Expr::Lit(syn::ExprLit { lit: syn::Lit::Bool(lit), .. - }) => Some(lit.clone()), + }) => { + // Overrride `lit` if this is a `FromReflect` derive. + // This typically means a user is opting out of the default implementation + // from the `Reflect` derive and using the `FromReflect` derive directly instead. + is_from_reflect_derive + .then(|| LitBool::new(true, Span::call_site())) + .or_else(|| Some(lit.clone())) + } _ => { return Err(syn::Error::new( pair.value.span(), @@ -366,7 +376,7 @@ impl ReflectTraits { impl Parse for ReflectTraits { fn parse(input: ParseStream) -> syn::Result { - ReflectTraits::from_metas(Punctuated::::parse_terminated(input)?) + ReflectTraits::from_metas(Punctuated::::parse_terminated(input)?, false) } } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 612e7aaea2080a..45e017ff445f59 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -134,7 +134,10 @@ enum ReflectMode { } impl<'a> ReflectDerive<'a> { - pub fn from_input(input: &'a DeriveInput) -> Result { + pub fn from_input( + input: &'a DeriveInput, + is_from_reflect_derive: bool, + ) -> Result { let mut traits = ReflectTraits::default(); // Should indicate whether `#[reflect_value]` was used. let mut reflect_mode = None; @@ -159,6 +162,7 @@ impl<'a> ReflectDerive<'a> { reflect_mode = Some(ReflectMode::Normal); let new_traits = ReflectTraits::from_metas( meta_list.parse_args_with(Punctuated::::parse_terminated)?, + is_from_reflect_derive, )?; traits.merge(new_traits)?; } @@ -173,6 +177,7 @@ impl<'a> ReflectDerive<'a> { reflect_mode = Some(ReflectMode::Value); let new_traits = ReflectTraits::from_metas( meta_list.parse_args_with(Punctuated::::parse_terminated)?, + is_from_reflect_derive, )?; traits.merge(new_traits)?; } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs index 56c5fa686e005a..2061782115988e 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/lib.rs @@ -156,7 +156,7 @@ pub(crate) static TYPE_NAME_ATTRIBUTE_NAME: &str = "type_name"; pub fn derive_reflect(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); - let derive_data = match ReflectDerive::from_input(&ast) { + let derive_data = match ReflectDerive::from_input(&ast, false) { Ok(data) => data, Err(err) => return err.into_compile_error().into(), }; @@ -232,7 +232,7 @@ pub fn derive_reflect(input: TokenStream) -> TokenStream { pub fn derive_from_reflect(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); - let derive_data = match ReflectDerive::from_input(&ast) { + let derive_data = match ReflectDerive::from_input(&ast, true) { Ok(data) => data, Err(err) => return err.into_compile_error().into(), }; @@ -266,7 +266,7 @@ pub fn derive_from_reflect(input: TokenStream) -> TokenStream { #[proc_macro_derive(TypePath, attributes(type_path, type_name))] pub fn derive_type_path(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); - let derive_data = match ReflectDerive::from_input(&ast) { + let derive_data = match ReflectDerive::from_input(&ast, false) { Ok(data) => data, Err(err) => return err.into_compile_error().into(), }; @@ -434,7 +434,7 @@ pub fn impl_reflect_value(input: TokenStream) -> TokenStream { #[proc_macro] pub fn impl_reflect_struct(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as DeriveInput); - let derive_data = match ReflectDerive::from_input(&ast) { + let derive_data = match ReflectDerive::from_input(&ast, false) { Ok(data) => data, Err(err) => return err.into_compile_error().into(), }; diff --git a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.fail.rs b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.fail.rs index a4c21f7b260a33..bd75c761d5011d 100644 --- a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.fail.rs +++ b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.fail.rs @@ -1,4 +1,4 @@ -use bevy_reflect::Reflect; +use bevy_reflect::{FromReflect, Reflect}; // Reason: Cannot have conflicting `from_reflect` attributes #[derive(Reflect)] @@ -16,4 +16,10 @@ struct Bar { value: String, } +// Reason: Conflicting `FromReflect` implementations +#[derive(Reflect, FromReflect)] +struct Baz { + value: String, +} + fn main() {} diff --git a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.fail.stderr b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.fail.stderr index c7a1355b44fba2..ce088970f39918 100644 --- a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.fail.stderr +++ b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.fail.stderr @@ -9,3 +9,13 @@ error: `from_reflect` already set to true | 14 | #[reflect(from_reflect = false)] | ^^^^^ + +error[E0119]: conflicting implementations of trait `FromReflect` for type `Baz` + --> tests/reflect_derive/from_reflect.fail.rs:20:19 + | +20 | #[derive(Reflect, FromReflect)] + | ------- ^^^^^^^^^^^ conflicting implementation for `Baz` + | | + | first implementation here + | + = note: this error originates in the derive macro `FromReflect` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.pass.rs b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.pass.rs index adb78fe4fd0748..4d8343e353bff1 100644 --- a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.pass.rs +++ b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/from_reflect.pass.rs @@ -1,4 +1,4 @@ -use bevy_reflect::Reflect; +use bevy_reflect::{FromReflect, Reflect}; #[derive(Reflect)] #[reflect(from_reflect = false)] @@ -14,4 +14,10 @@ struct Bar { value: String, } +#[derive(Reflect, FromReflect)] +#[reflect(from_reflect = false)] +struct Baz { + value: String, +} + fn main() {} diff --git a/examples/reflection/reflection.rs b/examples/reflection/reflection.rs index 3f3b99d1d60848..8f874633f8af68 100644 --- a/examples/reflection/reflection.rs +++ b/examples/reflection/reflection.rs @@ -33,7 +33,7 @@ fn main() { /// To do this, you can either define a `#[reflect(default = "...")]` attribute on the ignored field, or /// opt-out of `FromReflect`'s auto-derive using the `#[reflect(from_reflect = false)]` attribute. #[derive(Reflect)] -#[from_reflect(auto_derive = false)] +#[reflect(from_reflect = false)] pub struct Foo { a: usize, nested: Bar,