From 9ce39b6e4102c52e23174093f915542d6d74c204 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sat, 11 May 2024 18:36:44 -0700 Subject: [PATCH 01/11] Add #[reflect(Clone)] container attribute --- .../derive/src/container_attributes.rs | 48 +++++++++++++++++-- .../bevy_reflect/derive/src/impls/common.rs | 3 ++ crates/bevy_reflect/derive/src/lib.rs | 5 ++ crates/bevy_reflect/src/lib.rs | 33 +++++++++++++ crates/bevy_reflect/src/reflect.rs | 10 ++++ 5 files changed, 96 insertions(+), 3 deletions(-) diff --git a/crates/bevy_reflect/derive/src/container_attributes.rs b/crates/bevy_reflect/derive/src/container_attributes.rs index bdb94db06bc7e..3298f426e3c30 100644 --- a/crates/bevy_reflect/derive/src/container_attributes.rs +++ b/crates/bevy_reflect/derive/src/container_attributes.rs @@ -9,7 +9,7 @@ use crate::{ attribute_parser::terminated_parser, custom_attributes::CustomAttributes, derive_data::ReflectTraitToImpl, }; -use bevy_macro_utils::fq_std::{FQAny, FQOption}; +use bevy_macro_utils::fq_std::{FQAny, FQBox, FQClone, FQOption}; use proc_macro2::{Ident, Span}; use quote::quote_spanned; use syn::{ @@ -23,6 +23,7 @@ mod kw { syn::custom_keyword!(Debug); syn::custom_keyword!(PartialEq); syn::custom_keyword!(Hash); + syn::custom_keyword!(Clone); syn::custom_keyword!(no_field_bounds); syn::custom_keyword!(opaque); } @@ -175,6 +176,7 @@ impl TypePathAttrs { /// > __Note:__ Registering a custom function only works for special traits. #[derive(Default, Clone)] pub(crate) struct ContainerAttributes { + clone: TraitImpl, debug: TraitImpl, hash: TraitImpl, partial_eq: TraitImpl, @@ -236,12 +238,14 @@ impl ContainerAttributes { self.parse_opaque(input) } else if lookahead.peek(kw::no_field_bounds) { self.parse_no_field_bounds(input) + } else if lookahead.peek(kw::Clone) { + self.parse_clone(input) } else if lookahead.peek(kw::Debug) { self.parse_debug(input) - } else if lookahead.peek(kw::PartialEq) { - self.parse_partial_eq(input) } else if lookahead.peek(kw::Hash) { self.parse_hash(input) + } else if lookahead.peek(kw::PartialEq) { + self.parse_partial_eq(input) } else if lookahead.peek(Ident::peek_any) { self.parse_ident(input) } else { @@ -274,6 +278,26 @@ impl ContainerAttributes { Ok(()) } + /// Parse `clone` attribute. + /// + /// Examples: + /// - `#[reflect(Clone)]` + /// - `#[reflect(Clone(custom_clone_fn))]` + fn parse_clone(&mut self, input: ParseStream) -> syn::Result<()> { + let ident = input.parse::()?; + + if input.peek(token::Paren) { + let content; + parenthesized!(content in input); + let path = content.parse::()?; + self.clone.merge(TraitImpl::Custom(path, ident.span))?; + } else { + self.clone = TraitImpl::Implemented(ident.span); + } + + Ok(()) + } + /// Parse special `Debug` registration. /// /// Examples: @@ -523,6 +547,24 @@ impl ContainerAttributes { } } + pub fn get_clone_impl(&self, bevy_reflect_path: &Path) -> Option { + match &self.clone { + &TraitImpl::Implemented(span) => Some(quote_spanned! {span=> + #[inline] + fn reflect_clone(&self) -> #FQOption<#FQBox> { + #FQOption::Some(#FQBox::new(#FQClone::clone(self))) + } + }), + &TraitImpl::Custom(ref impl_fn, span) => Some(quote_spanned! {span=> + #[inline] + fn reflect_clone(&self) -> #FQOption<#FQBox> { + #FQOption::Some(#FQBox::new(#impl_fn(self))) + } + }), + TraitImpl::NotImplemented => None, + } + } + pub fn custom_attributes(&self) -> &CustomAttributes { &self.custom_attributes } diff --git a/crates/bevy_reflect/derive/src/impls/common.rs b/crates/bevy_reflect/derive/src/impls/common.rs index e8fdadb03e368..3d7b7f03adc42 100644 --- a/crates/bevy_reflect/derive/src/impls/common.rs +++ b/crates/bevy_reflect/derive/src/impls/common.rs @@ -115,6 +115,7 @@ pub fn common_partial_reflect_methods( } }) }); + let clone_fn = meta.attrs().get_clone_impl(bevy_reflect_path); quote! { #[inline] @@ -154,5 +155,7 @@ pub fn common_partial_reflect_methods( #partial_eq_fn #debug_fn + + #clone_fn } } diff --git a/crates/bevy_reflect/derive/src/lib.rs b/crates/bevy_reflect/derive/src/lib.rs index 276371427b620..743022c0001ff 100644 --- a/crates/bevy_reflect/derive/src/lib.rs +++ b/crates/bevy_reflect/derive/src/lib.rs @@ -156,6 +156,11 @@ fn match_reflect_impls(ast: DeriveInput, source: ReflectImplSource) -> TokenStre /// /// There are a few "special" identifiers that work a bit differently: /// +/// * `#[reflect(Clone)]` will force the implementation of `Reflect::reflect_clone` to rely on +/// the type's [`Clone`] implementation. +/// A custom implementation may be provided using `#[reflect(Clone(my_clone_func))]` where +/// `my_clone_func` is the path to a function matching the signature: +/// `(&self) -> Self`. /// * `#[reflect(Debug)]` will force the implementation of `Reflect::reflect_debug` to rely on /// the type's [`Debug`] implementation. /// A custom implementation may be provided using `#[reflect(Debug(my_debug_func))]` where diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index cf38c1da5086f..7860d58f4ac9e 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -986,6 +986,39 @@ mod tests { assert_eq!(values, vec![1]); } + #[test] + fn should_reflect_clone() { + #[derive(Reflect, Clone, Debug, PartialEq)] + #[reflect(Clone)] + struct Foo(usize); + + let foo = Foo(123); + let clone = foo.reflect_clone().unwrap(); + assert_eq!(foo, clone.take::().unwrap()); + + #[derive(Reflect, Clone, Debug, PartialEq)] + struct Bar(usize); + + let bar = Bar(123); + let clone = bar.reflect_clone(); + assert!(clone.is_none()); + } + + #[test] + fn should_custom_reflect_clone() { + #[derive(Reflect, Debug, PartialEq)] + #[reflect(Clone(clone_foo))] + struct Foo(usize); + + fn clone_foo(foo: &Foo) -> Foo { + Foo(foo.0 + 198) + } + + let foo = Foo(123); + let clone = foo.reflect_clone().unwrap(); + assert_eq!(Foo(321), clone.take::().unwrap()); + } + #[test] fn should_call_from_reflect_dynamically() { #[derive(Reflect)] diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 0898ff3630fe9..441fd7f87ea57 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -231,6 +231,16 @@ where /// [`Map`]: crate::Map fn clone_value(&self) -> Box; + /// Attempts to clone `Self` using reflection. + /// + /// Unlike [`Reflect::clone_value`], which often returns a dynamic representation of `Self`, + /// this method attempts create a clone of `Self` directly, if possible. + /// + /// If the clone cannot be performed, `None` is returned. + fn reflect_clone(&self) -> Option> { + None + } + /// Returns a hash of the value (which includes the type). /// /// If the underlying type does not support hashing, returns `None`. From 5856176aef63bc45c2b504fdc015b503158adaba Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sat, 11 May 2024 18:49:26 -0700 Subject: [PATCH 02/11] Add `Clone` attribute to reflect impls --- crates/bevy_reflect/src/impls/glam.rs | 100 ++++++++++---------- crates/bevy_reflect/src/impls/petgraph.rs | 3 +- crates/bevy_reflect/src/impls/smol_str.rs | 1 + crates/bevy_reflect/src/impls/std.rs | 72 +++++++++----- crates/bevy_reflect/src/impls/uuid.rs | 1 + crates/bevy_reflect/src/impls/wgpu_types.rs | 1 + 6 files changed, 102 insertions(+), 76 deletions(-) diff --git a/crates/bevy_reflect/src/impls/glam.rs b/crates/bevy_reflect/src/impls/glam.rs index 653eb3edaa55b..1e0a6a11d0675 100644 --- a/crates/bevy_reflect/src/impls/glam.rs +++ b/crates/bevy_reflect/src/impls/glam.rs @@ -18,7 +18,7 @@ macro_rules! reflect_enum { } impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct IVec2 { x: i32, @@ -26,7 +26,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct IVec3 { x: i32, @@ -35,7 +35,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct IVec4 { x: i32, @@ -46,7 +46,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct I8Vec2 { x: i8, @@ -55,7 +55,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct I8Vec3 { x: i8, @@ -65,7 +65,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct I8Vec4 { x: i8, @@ -76,7 +76,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct I16Vec2 { x: i16, @@ -85,7 +85,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct I16Vec3 { x: i16, @@ -95,7 +95,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct I16Vec4 { x: i16, @@ -106,7 +106,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct I64Vec2 { x: i64, @@ -115,7 +115,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct I64Vec3 { x: i64, @@ -125,7 +125,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct I64Vec4 { x: i64, @@ -136,7 +136,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct UVec2 { x: u32, @@ -144,7 +144,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct UVec3 { x: u32, @@ -153,7 +153,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct UVec4 { x: u32, @@ -164,7 +164,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct U8Vec2 { x: u8, @@ -172,7 +172,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct U8Vec3 { x: u8, @@ -181,7 +181,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct U8Vec4 { x: u8, @@ -192,7 +192,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct U16Vec2 { x: u16, @@ -200,7 +200,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct U16Vec3 { x: u16, @@ -209,7 +209,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct U16Vec4 { x: u16, @@ -220,7 +220,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct U64Vec2 { x: u64, @@ -228,7 +228,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct U64Vec3 { x: u64, @@ -237,7 +237,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, Hash, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct U64Vec4 { x: u64, @@ -248,7 +248,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Vec2 { x: f32, @@ -256,7 +256,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Vec3 { x: f32, @@ -265,7 +265,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Vec3A { x: f32, @@ -274,7 +274,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Vec4 { x: f32, @@ -285,7 +285,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct BVec2 { x: bool, @@ -293,7 +293,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct BVec3 { x: bool, @@ -302,7 +302,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct BVec4 { x: bool, @@ -313,7 +313,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct DVec2 { x: f64, @@ -321,7 +321,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct DVec3 { x: f64, @@ -330,7 +330,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct DVec4 { x: f64, @@ -341,7 +341,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Mat2 { x_axis: Vec2, @@ -349,7 +349,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Mat3 { x_axis: Vec3, @@ -358,7 +358,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Mat3A { x_axis: Vec3A, @@ -367,7 +367,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Mat4 { x_axis: Vec4, @@ -378,7 +378,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct DMat2 { x_axis: DVec2, @@ -386,7 +386,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct DMat3 { x_axis: DVec3, @@ -395,7 +395,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct DMat4 { x_axis: DVec4, @@ -406,7 +406,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Affine2 { matrix2: Mat2, @@ -414,7 +414,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Affine3A { matrix3: Mat3A, @@ -423,7 +423,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct DAffine2 { matrix2: DMat2, @@ -431,7 +431,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct DAffine3 { matrix3: DMat3, @@ -440,7 +440,7 @@ impl_reflect!( ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct Quat { x: f32, @@ -450,7 +450,7 @@ impl_reflect!( } ); impl_reflect!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] struct DQuat { x: f64, @@ -461,7 +461,7 @@ impl_reflect!( ); reflect_enum!( - #[reflect(Debug, PartialEq, Default, Deserialize, Serialize)] + #[reflect(Clone, Debug, PartialEq, Default, Deserialize, Serialize)] #[type_path = "glam"] enum EulerRot { ZYX, @@ -491,8 +491,8 @@ reflect_enum!( } ); -impl_reflect_opaque!(::glam::BVec3A(Debug, Default, Deserialize, Serialize)); -impl_reflect_opaque!(::glam::BVec4A(Debug, Default, Deserialize, Serialize)); +impl_reflect_opaque!(::glam::BVec3A(Clone, Debug, Default, Deserialize, Serialize)); +impl_reflect_opaque!(::glam::BVec4A(Clone, Debug, Default, Deserialize, Serialize)); #[cfg(test)] mod tests { diff --git a/crates/bevy_reflect/src/impls/petgraph.rs b/crates/bevy_reflect/src/impls/petgraph.rs index 2264c9cb4b9b2..1a1f3a34c4234 100644 --- a/crates/bevy_reflect/src/impls/petgraph.rs +++ b/crates/bevy_reflect/src/impls/petgraph.rs @@ -1,6 +1,7 @@ use crate::{impl_reflect_opaque, prelude::ReflectDefault, ReflectDeserialize, ReflectSerialize}; impl_reflect_opaque!(::petgraph::graph::NodeIndex( + Clone, Default, Serialize, Deserialize @@ -9,4 +10,4 @@ impl_reflect_opaque!(::petgraph::graph::DiGraph< N: ::core::clone::Clone, E: ::core::clone::Clone, Ix: ::petgraph::graph::IndexType ->()); +>(Clone)); diff --git a/crates/bevy_reflect/src/impls/smol_str.rs b/crates/bevy_reflect/src/impls/smol_str.rs index e2c09b206b3be..d07a00cd6f656 100644 --- a/crates/bevy_reflect/src/impls/smol_str.rs +++ b/crates/bevy_reflect/src/impls/smol_str.rs @@ -2,6 +2,7 @@ use crate::{std_traits::ReflectDefault, ReflectDeserialize, ReflectSerialize}; use bevy_reflect_derive::impl_reflect_opaque; impl_reflect_opaque!(::smol_str::SmolStr( + Clone, Debug, Hash, PartialEq, diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 119b66e3ed6d0..72381839b7ff2 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -34,6 +34,7 @@ use core::{ use std::path::Path; impl_reflect_opaque!(bool( + Clone, Debug, Hash, PartialEq, @@ -42,6 +43,7 @@ impl_reflect_opaque!(bool( Default )); impl_reflect_opaque!(char( + Clone, Debug, Hash, PartialEq, @@ -49,11 +51,12 @@ impl_reflect_opaque!(char( Deserialize, Default )); -impl_reflect_opaque!(u8(Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(u16(Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(u32(Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(u64(Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(u8(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(u16(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(u32(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(u64(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); impl_reflect_opaque!(u128( + Clone, Debug, Hash, PartialEq, @@ -62,6 +65,7 @@ impl_reflect_opaque!(u128( Default )); impl_reflect_opaque!(usize( + Clone, Debug, Hash, PartialEq, @@ -69,11 +73,12 @@ impl_reflect_opaque!(usize( Deserialize, Default )); -impl_reflect_opaque!(i8(Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(i16(Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(i32(Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(i64(Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(i8(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(i16(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(i32(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(i64(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); impl_reflect_opaque!(i128( + Clone, Debug, Hash, PartialEq, @@ -82,6 +87,7 @@ impl_reflect_opaque!(i128( Default )); impl_reflect_opaque!(isize( + Clone, Debug, Hash, PartialEq, @@ -89,10 +95,11 @@ impl_reflect_opaque!(isize( Deserialize, Default )); -impl_reflect_opaque!(f32(Debug, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(f64(Debug, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(f32(Clone, Debug, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(f64(Clone, Debug, PartialEq, Serialize, Deserialize, Default)); impl_type_path!(str); impl_reflect_opaque!(::alloc::string::String( + Clone, Debug, Hash, PartialEq, @@ -102,6 +109,7 @@ impl_reflect_opaque!(::alloc::string::String( )); #[cfg(feature = "std")] impl_reflect_opaque!(::std::path::PathBuf( + Clone, Debug, Hash, PartialEq, @@ -109,16 +117,17 @@ impl_reflect_opaque!(::std::path::PathBuf( Deserialize, Default )); -impl_reflect_opaque!(::core::any::TypeId(Debug, Hash, PartialEq,)); -impl_reflect_opaque!(::alloc::collections::BTreeSet()); -impl_reflect_opaque!(::core::ops::Range()); -impl_reflect_opaque!(::core::ops::RangeInclusive()); -impl_reflect_opaque!(::core::ops::RangeFrom()); -impl_reflect_opaque!(::core::ops::RangeTo()); -impl_reflect_opaque!(::core::ops::RangeToInclusive()); -impl_reflect_opaque!(::core::ops::RangeFull()); -impl_reflect_opaque!(::core::ops::Bound()); +impl_reflect_opaque!(::core::any::TypeId(Clone, Debug, Hash, PartialEq,)); +impl_reflect_opaque!(::alloc::collections::BTreeSet(Clone)); +impl_reflect_opaque!(::core::ops::Range(Clone)); +impl_reflect_opaque!(::core::ops::RangeInclusive(Clone)); +impl_reflect_opaque!(::core::ops::RangeFrom(Clone)); +impl_reflect_opaque!(::core::ops::RangeTo(Clone)); +impl_reflect_opaque!(::core::ops::RangeToInclusive(Clone)); +impl_reflect_opaque!(::core::ops::RangeFull(Clone)); +impl_reflect_opaque!(::core::ops::Bound(Clone)); impl_reflect_opaque!(::core::time::Duration( + Clone, Debug, Hash, PartialEq, @@ -127,9 +136,10 @@ impl_reflect_opaque!(::core::time::Duration( Default )); impl_reflect_opaque!(::bevy_platform_support::time::Instant( - Debug, Hash, PartialEq + Clone, Debug, Hash, PartialEq )); impl_reflect_opaque!(::core::num::NonZeroI128( + Clone, Debug, Hash, PartialEq, @@ -137,6 +147,7 @@ impl_reflect_opaque!(::core::num::NonZeroI128( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroU128( + Clone, Debug, Hash, PartialEq, @@ -144,6 +155,7 @@ impl_reflect_opaque!(::core::num::NonZeroU128( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroIsize( + Clone, Debug, Hash, PartialEq, @@ -151,6 +163,7 @@ impl_reflect_opaque!(::core::num::NonZeroIsize( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroUsize( + Clone, Debug, Hash, PartialEq, @@ -158,6 +171,7 @@ impl_reflect_opaque!(::core::num::NonZeroUsize( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroI64( + Clone, Debug, Hash, PartialEq, @@ -165,6 +179,7 @@ impl_reflect_opaque!(::core::num::NonZeroI64( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroU64( + Clone, Debug, Hash, PartialEq, @@ -172,6 +187,7 @@ impl_reflect_opaque!(::core::num::NonZeroU64( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroU32( + Clone, Debug, Hash, PartialEq, @@ -179,6 +195,7 @@ impl_reflect_opaque!(::core::num::NonZeroU32( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroI32( + Clone, Debug, Hash, PartialEq, @@ -186,6 +203,7 @@ impl_reflect_opaque!(::core::num::NonZeroI32( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroI16( + Clone, Debug, Hash, PartialEq, @@ -193,6 +211,7 @@ impl_reflect_opaque!(::core::num::NonZeroI16( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroU16( + Clone, Debug, Hash, PartialEq, @@ -200,6 +219,7 @@ impl_reflect_opaque!(::core::num::NonZeroU16( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroU8( + Clone, Debug, Hash, PartialEq, @@ -207,20 +227,22 @@ impl_reflect_opaque!(::core::num::NonZeroU8( Deserialize )); impl_reflect_opaque!(::core::num::NonZeroI8( + Clone, Debug, Hash, PartialEq, Serialize, Deserialize )); -impl_reflect_opaque!(::core::num::Wrapping()); -impl_reflect_opaque!(::core::num::Saturating()); -impl_reflect_opaque!(::bevy_platform_support::sync::Arc); +impl_reflect_opaque!(::core::num::Wrapping(Clone)); +impl_reflect_opaque!(::core::num::Saturating(Clone)); +impl_reflect_opaque!(::bevy_platform_support::sync::Arc(Clone)); // `Serialize` and `Deserialize` only for platforms supported by serde: // https://github.com/serde-rs/serde/blob/3ffb86fc70efd3d329519e2dddfa306cc04f167c/serde/src/de/impls.rs#L1732 #[cfg(all(any(unix, windows), feature = "std"))] impl_reflect_opaque!(::std::ffi::OsString( + Clone, Debug, Hash, PartialEq, @@ -228,8 +250,8 @@ impl_reflect_opaque!(::std::ffi::OsString( Deserialize )); #[cfg(all(not(any(unix, windows)), feature = "std"))] -impl_reflect_opaque!(::std::ffi::OsString(Debug, Hash, PartialEq)); -impl_reflect_opaque!(::alloc::collections::BinaryHeap); +impl_reflect_opaque!(::std::ffi::OsString(Clone, Debug, Hash, PartialEq)); +impl_reflect_opaque!(::alloc::collections::BinaryHeap(Clone)); macro_rules! impl_reflect_for_atomic { ($ty:ty, $ordering:expr) => { diff --git a/crates/bevy_reflect/src/impls/uuid.rs b/crates/bevy_reflect/src/impls/uuid.rs index 56beedd41ab41..7385304e28a26 100644 --- a/crates/bevy_reflect/src/impls/uuid.rs +++ b/crates/bevy_reflect/src/impls/uuid.rs @@ -5,6 +5,7 @@ impl_reflect_opaque!(::uuid::Uuid( Serialize, Deserialize, Default, + Clone, Debug, PartialEq, Hash diff --git a/crates/bevy_reflect/src/impls/wgpu_types.rs b/crates/bevy_reflect/src/impls/wgpu_types.rs index b4a1750ba1242..734eace938a04 100644 --- a/crates/bevy_reflect/src/impls/wgpu_types.rs +++ b/crates/bevy_reflect/src/impls/wgpu_types.rs @@ -1,6 +1,7 @@ use crate::{impl_reflect_opaque, ReflectDeserialize, ReflectSerialize}; impl_reflect_opaque!(::wgpu_types::TextureFormat( + Clone, Debug, Hash, PartialEq, From ef7e8f6c9aad9dc9cbf68aab7e51d54cdb1adbe5 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sat, 18 May 2024 16:24:28 -0700 Subject: [PATCH 03/11] Support #[reflect(clone)] on fields --- crates/bevy_reflect/derive/src/derive_data.rs | 164 +++++++++++++- .../bevy_reflect/derive/src/enum_utility.rs | 202 ++++++++++++++---- .../derive/src/field_attributes.rs | 38 ++++ .../bevy_reflect/derive/src/impls/common.rs | 3 - crates/bevy_reflect/derive/src/impls/enums.rs | 3 + .../bevy_reflect/derive/src/impls/opaque.rs | 3 + .../bevy_reflect/derive/src/impls/structs.rs | 3 + .../derive/src/impls/tuple_structs.rs | 3 + crates/bevy_reflect/derive/src/lib.rs | 20 +- .../bevy_reflect/derive/src/struct_utility.rs | 59 +---- crates/bevy_reflect/src/lib.rs | 126 ++++++++++- 11 files changed, 515 insertions(+), 109 deletions(-) diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index e739c91ebb912..01b0c51840516 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -12,13 +12,17 @@ use crate::{ where_clause_options::WhereClauseOptions, REFLECT_ATTRIBUTE_NAME, TYPE_NAME_ATTRIBUTE_NAME, TYPE_PATH_ATTRIBUTE_NAME, }; -use quote::{quote, ToTokens}; +use quote::{format_ident, quote, ToTokens}; use syn::token::Comma; +use crate::enum_utility::{EnumVariantOutputData, ReflectCloneVariantBuilder, VariantBuilder}; +use crate::field_attributes::CloneBehavior; use crate::generics::generate_generics; +use bevy_macro_utils::fq_std::{FQBox, FQClone, FQOption}; use syn::{ parse_str, punctuated::Punctuated, spanned::Spanned, Data, DeriveInput, Field, Fields, - GenericParam, Generics, Ident, LitStr, Meta, Path, PathSegment, Type, TypeParam, Variant, + GenericParam, Generics, Ident, LitStr, Member, Meta, Path, PathSegment, Type, TypeParam, + Variant, }; pub(crate) enum ReflectDerive<'a> { @@ -266,7 +270,7 @@ impl<'a> ReflectDerive<'a> { { return Err(syn::Error::new( meta.type_path().span(), - format!("a #[{TYPE_PATH_ATTRIBUTE_NAME} = \"...\"] attribute must be specified when using {provenance}") + format!("a #[{TYPE_PATH_ATTRIBUTE_NAME} = \"...\"] attribute must be specified when using {provenance}"), )); } @@ -546,6 +550,17 @@ impl<'a> StructField<'a> { pub fn attrs(&self) -> &FieldAttributes { &self.attrs } + + /// Generates a [`Member`] based on this field. + /// + /// If the field is unnamed, the declaration index is used. + /// This allows this member to be used for both active and ignored fields. + pub fn to_member(&self) -> Member { + match &self.data.ident { + Some(ident) => Member::Named(ident.clone()), + None => Member::Unnamed(self.declaration_index.into()), + } + } } impl<'a> ReflectStruct<'a> { @@ -655,6 +670,108 @@ impl<'a> ReflectStruct<'a> { #bevy_reflect_path::TypeInfo::#info_variant(#info) } } + /// Returns the `Reflect::reflect_clone` impl, if any, as a `TokenStream`. + pub fn get_clone_impl(&self) -> Option { + let bevy_reflect_path = self.meta().bevy_reflect_path(); + + if let container_clone @ Some(_) = self.meta().attrs().get_clone_impl(bevy_reflect_path) { + return container_clone; + } + + let mut tokens = proc_macro2::TokenStream::new(); + + for field in self.fields().iter() { + let member = field.to_member(); + let accessor = self.access_for_field(field, false); + + match &field.attrs.clone { + CloneBehavior::Default => { + if field.attrs.ignore.is_ignored() { + return None; + } + + tokens.extend(quote! { + #member: #bevy_reflect_path::PartialReflect::reflect_clone(#accessor)?.take().ok()?, + }); + } + CloneBehavior::Trait => { + tokens.extend(quote! { + #member: #FQClone::clone(#accessor), + }); + } + CloneBehavior::Func(clone_fn) => { + tokens.extend(quote! { + #member: #clone_fn(#accessor), + }); + } + } + } + + let ctor = match self.meta.remote_ty() { + Some(ty) => { + let ty = ty.as_expr_path().ok()?.to_token_stream(); + quote! { + Self(#ty { + #tokens + }) + } + } + None => { + quote! { + Self { + #tokens + } + } + } + }; + + Some(quote! { + #[inline] + fn reflect_clone(&self) -> #FQOption<#FQBox> { + #FQOption::Some(#FQBox::new(#ctor)) + } + }) + } + + /// Generates an accessor for the given field. + /// + /// The mutability of the access can be controlled by the `is_mut` parameter. + /// + /// Generally, this just returns something like `&self.field`. + /// However, if the struct is a remote wrapper, this then becomes `&self.0.field` in order to access the field on the inner type. + /// + /// If the field itself is a remote type, the above accessor is further wrapped in a call to `ReflectRemote::as_wrapper[_mut]`. + pub fn access_for_field( + &self, + field: &StructField<'a>, + is_mutable: bool, + ) -> proc_macro2::TokenStream { + let bevy_reflect_path = self.meta().bevy_reflect_path(); + let member = field.to_member(); + + let prefix_tokens = if is_mutable { quote!(&mut) } else { quote!(&) }; + + let accessor = if self.meta.is_remote_wrapper() { + quote!(self.0.#member) + } else { + quote!(self.#member) + }; + + match &field.attrs.remote { + Some(wrapper_ty) => { + let method = if is_mutable { + format_ident!("as_wrapper_mut") + } else { + format_ident!("as_wrapper") + }; + + quote! { + <#wrapper_ty as #bevy_reflect_path::ReflectRemote>::#method(#prefix_tokens #accessor) + } + } + None => quote!(#prefix_tokens #accessor), + } + } } impl<'a> ReflectEnum<'a> { @@ -757,6 +874,47 @@ impl<'a> ReflectEnum<'a> { #bevy_reflect_path::TypeInfo::Enum(#info) } } + + /// Returns the `Reflect::reflect_clone` impl, if any, as a `TokenStream`. + pub fn get_clone_impl(&self) -> Option { + let bevy_reflect_path = self.meta().bevy_reflect_path(); + + if let container_clone @ Some(_) = self.meta().attrs().get_clone_impl(bevy_reflect_path) { + return container_clone; + } + + let this = Ident::new("this", Span::call_site()); + let EnumVariantOutputData { + variant_patterns, + variant_constructors, + .. + } = ReflectCloneVariantBuilder::new(self).build(&this)?; + + let inner = quote! { + match #this { + #(#variant_patterns => #variant_constructors),* + } + }; + + let body = if self.meta.is_remote_wrapper() { + quote! { + let #this = ::as_remote(self); + #FQOption::Some(#FQBox::new(::into_wrapper(#inner))) + } + } else { + quote! { + let #this = self; + #FQOption::Some(#FQBox::new(#inner)) + } + }; + + Some(quote! { + #[inline] + fn reflect_clone(&self) -> #FQOption<#FQBox> { + #body + } + }) + } } impl<'a> EnumVariant<'a> { diff --git a/crates/bevy_reflect/derive/src/enum_utility.rs b/crates/bevy_reflect/derive/src/enum_utility.rs index f4b1e5ede8b77..e0d6ea6c1bc6f 100644 --- a/crates/bevy_reflect/derive/src/enum_utility.rs +++ b/crates/bevy_reflect/derive/src/enum_utility.rs @@ -1,16 +1,21 @@ +use crate::field_attributes::CloneBehavior; use crate::{ derive_data::ReflectEnum, derive_data::StructField, field_attributes::DefaultBehavior, ident::ident_or_index, }; -use bevy_macro_utils::fq_std::{FQDefault, FQOption}; +use bevy_macro_utils::fq_std::{FQClone, FQDefault, FQOption}; use proc_macro2::{Ident, TokenStream}; -use quote::{format_ident, quote}; +use quote::{format_ident, quote, ToTokens}; pub(crate) struct EnumVariantOutputData { /// The names of each variant as a string. /// /// For example, `Some` and `None` for the `Option` enum. pub variant_names: Vec, + /// The pattern matching portion of each variant. + /// + /// For example, `Option::Some { 0: _0 }` and `Option::None {}` for the `Option` enum. + pub variant_patterns: Vec, /// The constructor portion of each variant. /// /// For example, `Option::Some { 0: value }` and `Option::None {}` for the `Option` enum. @@ -31,6 +36,9 @@ pub(crate) struct VariantField<'a, 'b> { /// Trait used to control how enum variants are built. pub(crate) trait VariantBuilder: Sized { + /// The variant output data. + type Output; + /// Returns the enum data. fn reflect_enum(&self) -> &ReflectEnum; @@ -125,67 +133,83 @@ pub(crate) trait VariantBuilder: Sized { /// Returns a token stream that constructs an instance of an ignored field. /// + /// If the ignored field cannot be adequately handled, `None` should be returned. + /// /// # Parameters /// * `field`: The field to access - fn on_ignored_field(&self, field: VariantField) -> TokenStream { - match &field.field.attrs.default { + fn on_ignored_field(&self, field: VariantField) -> Option { + Some(match &field.field.attrs.default { DefaultBehavior::Func(path) => quote! { #path() }, _ => quote! { #FQDefault::default() }, - } + }) } /// Builds the enum variant output data. - fn build(&self, this: &Ident) -> EnumVariantOutputData { - let variants = self.reflect_enum().variants(); - - let mut variant_names = Vec::with_capacity(variants.len()); - let mut variant_constructors = Vec::with_capacity(variants.len()); + fn build(&self, this: &Ident) -> Self::Output; +} - for variant in variants { - let variant_ident = &variant.data.ident; - let variant_name = variant_ident.to_string(); - let variant_path = self.reflect_enum().get_unit(variant_ident); +fn build(builder: &V, this: &Ident) -> Option { + let variants = builder.reflect_enum().variants(); - let fields = variant.fields(); + let mut variant_names = Vec::with_capacity(variants.len()); + let mut variant_patterns = Vec::with_capacity(variants.len()); + let mut variant_constructors = Vec::with_capacity(variants.len()); - let field_constructors = fields.iter().map(|field| { - let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); - let alias = format_ident!("_{}", member); + for variant in variants { + let variant_ident = &variant.data.ident; + let variant_name = variant_ident.to_string(); + let variant_path = builder.reflect_enum().get_unit(variant_ident); - let variant_field = VariantField { - alias: &alias, - variant_name: &variant_name, - field, - }; + let fields = variant.fields(); - let value = if field.attrs.ignore.is_ignored() { - self.on_ignored_field(variant_field) - } else { - self.on_active_field(this, variant_field) - }; + let mut field_patterns = Vec::with_capacity(fields.len()); + let mut field_constructors = Vec::with_capacity(fields.len()); - let constructor = quote! { - #member: #value - }; + for field in fields { + let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); + let alias = format_ident!("_{}", member); - constructor - }); + let variant_field = VariantField { + alias: &alias, + variant_name: &variant_name, + field, + }; - let constructor = quote! { - #variant_path { - #( #field_constructors ),* - } + let value = if field.attrs.ignore.is_ignored() { + builder.on_ignored_field(variant_field)? + } else { + builder.on_active_field(this, variant_field) }; - variant_names.push(variant_name); - variant_constructors.push(constructor); - } + field_patterns.push(quote! { + #member: #alias + }); - EnumVariantOutputData { - variant_names, - variant_constructors, + field_constructors.push(quote! { + #member: #value + }); } + + let pattern = quote! { + #variant_path { #( #field_patterns ),* } + }; + + let constructor = quote! { + #variant_path { + #( #field_constructors ),* + } + }; + + variant_names.push(variant_name); + variant_patterns.push(pattern); + variant_constructors.push(constructor); } + + Some(EnumVariantOutputData { + variant_names, + variant_patterns, + variant_constructors, + }) } /// Generates the enum variant output data needed to build the `FromReflect::from_reflect` implementation. @@ -200,6 +224,8 @@ impl<'a> FromReflectVariantBuilder<'a> { } impl<'a> VariantBuilder for FromReflectVariantBuilder<'a> { + type Output = EnumVariantOutputData; + fn reflect_enum(&self) -> &ReflectEnum { self.reflect_enum } @@ -218,6 +244,10 @@ impl<'a> VariantBuilder for FromReflectVariantBuilder<'a> { <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#alias)? } } + + fn build(&self, this: &Ident) -> Self::Output { + build(self, this).expect("internal bevy_reflect error: ignored fields should be handled") + } } /// Generates the enum variant output data needed to build the `PartialReflect::try_apply` implementation. @@ -232,6 +262,8 @@ impl<'a> TryApplyVariantBuilder<'a> { } impl<'a> VariantBuilder for TryApplyVariantBuilder<'a> { + type Output = EnumVariantOutputData; + fn reflect_enum(&self) -> &ReflectEnum { self.reflect_enum } @@ -274,4 +306,88 @@ impl<'a> VariantBuilder for TryApplyVariantBuilder<'a> { })? } } + + fn build(&self, this: &Ident) -> Self::Output { + build(self, this).expect("internal bevy_reflect error: ignored fields should be handled") + } +} + +/// Generates the enum variant output data needed to build the `Reflect::reflect_clone` implementation. +pub(crate) struct ReflectCloneVariantBuilder<'a> { + reflect_enum: &'a ReflectEnum<'a>, +} + +impl<'a> ReflectCloneVariantBuilder<'a> { + pub fn new(reflect_enum: &'a ReflectEnum) -> Self { + Self { reflect_enum } + } +} + +impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> { + type Output = Option; + fn reflect_enum(&self) -> &ReflectEnum { + self.reflect_enum + } + + fn access_field(&self, _ident: &Ident, field: VariantField) -> TokenStream { + let alias = field.alias; + quote!(#FQOption::Some(#alias)) + } + + fn unwrap_field(&self, field: VariantField) -> TokenStream { + let alias = field.alias; + quote!(#alias.unwrap()) + } + + fn construct_field(&self, field: VariantField) -> TokenStream { + let bevy_reflect_path = self.reflect_enum.meta().bevy_reflect_path(); + + let alias = field.alias; + let alias = match &field.field.attrs.remote { + Some(wrapper_ty) => { + quote! { + <#wrapper_ty as #bevy_reflect_path::ReflectRemote>::as_wrapper(#alias) + } + } + None => alias.to_token_stream(), + }; + + match &field.field.attrs.clone { + CloneBehavior::Default => { + quote! { + #bevy_reflect_path::PartialReflect::reflect_clone(#alias)?.take().ok()? + } + } + CloneBehavior::Trait => { + quote! { + #FQClone::clone(#alias) + } + } + CloneBehavior::Func(clone_fn) => { + quote! { + #clone_fn(#alias) + } + } + } + } + + fn on_active_field(&self, _this: &Ident, field: VariantField) -> TokenStream { + self.construct_field(field) + } + + fn on_ignored_field(&self, field: VariantField) -> Option { + let alias = field.alias; + + match &field.field.attrs.clone { + CloneBehavior::Default => None, + CloneBehavior::Trait => Some(quote! { + #FQClone::clone(#alias) + }), + CloneBehavior::Func(clone_fn) => Some(quote! { #clone_fn() }), + } + } + + fn build(&self, this: &Ident) -> Self::Output { + build(self, this) + } } diff --git a/crates/bevy_reflect/derive/src/field_attributes.rs b/crates/bevy_reflect/derive/src/field_attributes.rs index 6cddb50e61bda..06d64791c414e 100644 --- a/crates/bevy_reflect/derive/src/field_attributes.rs +++ b/crates/bevy_reflect/derive/src/field_attributes.rs @@ -14,6 +14,7 @@ use syn::{parse::ParseStream, Attribute, LitStr, Meta, Token, Type}; mod kw { syn::custom_keyword!(ignore); syn::custom_keyword!(skip_serializing); + syn::custom_keyword!(clone); syn::custom_keyword!(default); syn::custom_keyword!(remote); } @@ -22,6 +23,7 @@ pub(crate) const IGNORE_SERIALIZATION_ATTR: &str = "skip_serializing"; pub(crate) const IGNORE_ALL_ATTR: &str = "ignore"; pub(crate) const DEFAULT_ATTR: &str = "default"; +pub(crate) const CLONE_ATTR: &str = "clone"; /// Stores data about if the field should be visible via the Reflect and serialization interfaces /// @@ -54,6 +56,14 @@ impl ReflectIgnoreBehavior { } } +#[derive(Default, Clone)] +pub(crate) enum CloneBehavior { + #[default] + Default, + Trait, + Func(syn::ExprPath), +} + /// Controls how the default value is determined for a field. #[derive(Default, Clone)] pub(crate) enum DefaultBehavior { @@ -74,6 +84,8 @@ pub(crate) enum DefaultBehavior { pub(crate) struct FieldAttributes { /// Determines how this field should be ignored if at all. pub ignore: ReflectIgnoreBehavior, + /// Sets the clone behavior of this field. + pub clone: CloneBehavior, /// Sets the default behavior of this field. pub default: DefaultBehavior, /// Custom attributes created via `#[reflect(@...)]`. @@ -121,6 +133,8 @@ impl FieldAttributes { self.parse_ignore(input) } else if lookahead.peek(kw::skip_serializing) { self.parse_skip_serializing(input) + } else if lookahead.peek(kw::clone) { + self.parse_clone(input) } else if lookahead.peek(kw::default) { self.parse_default(input) } else if lookahead.peek(kw::remote) { @@ -164,6 +178,30 @@ impl FieldAttributes { Ok(()) } + /// Parse `clone` attribute. + /// + /// Examples: + /// - `#[reflect(clone)]` + /// - `#[reflect(clone = "path::to::func")]` + fn parse_clone(&mut self, input: ParseStream) -> syn::Result<()> { + if !matches!(self.clone, CloneBehavior::Default) { + return Err(input.error(format!("only one of {:?} is allowed", [CLONE_ATTR]))); + } + + input.parse::()?; + + if input.peek(Token![=]) { + input.parse::()?; + + let lit = input.parse::()?; + self.clone = CloneBehavior::Func(lit.parse()?); + } else { + self.clone = CloneBehavior::Trait; + } + + Ok(()) + } + /// Parse `default` attribute. /// /// Examples: diff --git a/crates/bevy_reflect/derive/src/impls/common.rs b/crates/bevy_reflect/derive/src/impls/common.rs index 3d7b7f03adc42..e8fdadb03e368 100644 --- a/crates/bevy_reflect/derive/src/impls/common.rs +++ b/crates/bevy_reflect/derive/src/impls/common.rs @@ -115,7 +115,6 @@ pub fn common_partial_reflect_methods( } }) }); - let clone_fn = meta.attrs().get_clone_impl(bevy_reflect_path); quote! { #[inline] @@ -155,7 +154,5 @@ pub fn common_partial_reflect_methods( #partial_eq_fn #debug_fn - - #clone_fn } } diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index 235a7cff1c0e0..338d60795be89 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -70,6 +70,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream || Some(quote!(#bevy_reflect_path::enum_partial_eq)), || Some(quote!(#bevy_reflect_path::enum_hash)), ); + let clone_fn = reflect_enum.get_clone_impl(); #[cfg(not(feature = "functions"))] let function_impls = None::; @@ -261,6 +262,8 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream } #common_methods + + #clone_fn } } } diff --git a/crates/bevy_reflect/derive/src/impls/opaque.rs b/crates/bevy_reflect/derive/src/impls/opaque.rs index bdee656a964cc..11893a62e92e2 100644 --- a/crates/bevy_reflect/derive/src/impls/opaque.rs +++ b/crates/bevy_reflect/derive/src/impls/opaque.rs @@ -32,6 +32,7 @@ pub(crate) fn impl_opaque(meta: &ReflectMeta) -> proc_macro2::TokenStream { let type_path_impl = impl_type_path(meta); let full_reflect_impl = impl_full_reflect(meta, &where_clause_options); let common_methods = common_partial_reflect_methods(meta, || None, || None); + let clone_fn = meta.attrs().get_clone_impl(bevy_reflect_path); let apply_impl = if let Some(remote_ty) = meta.remote_ty() { let ty = remote_ty.type_path(); @@ -117,6 +118,8 @@ pub(crate) fn impl_opaque(meta: &ReflectMeta) -> proc_macro2::TokenStream { } #common_methods + + #clone_fn } } } diff --git a/crates/bevy_reflect/derive/src/impls/structs.rs b/crates/bevy_reflect/derive/src/impls/structs.rs index c1db19ca9c80b..9487fbc6e7f2a 100644 --- a/crates/bevy_reflect/derive/src/impls/structs.rs +++ b/crates/bevy_reflect/derive/src/impls/structs.rs @@ -47,6 +47,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS || Some(quote!(#bevy_reflect_path::struct_partial_eq)), || None, ); + let clone_fn = reflect_struct.get_clone_impl(); #[cfg(not(feature = "functions"))] let function_impls = None::; @@ -179,6 +180,8 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> proc_macro2::TokenS } #common_methods + + #clone_fn } } } diff --git a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs index a0037c64ca57e..753412b4b9b19 100644 --- a/crates/bevy_reflect/derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/derive/src/impls/tuple_structs.rs @@ -37,6 +37,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: || Some(quote!(#bevy_reflect_path::tuple_struct_partial_eq)), || None, ); + let clone_fn = reflect_struct.get_clone_impl(); #[cfg(not(feature = "functions"))] let function_impls = None::; @@ -144,6 +145,8 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> proc_macro2:: } #common_methods + + #clone_fn } } } diff --git a/crates/bevy_reflect/derive/src/lib.rs b/crates/bevy_reflect/derive/src/lib.rs index 743022c0001ff..2d9dfca681568 100644 --- a/crates/bevy_reflect/derive/src/lib.rs +++ b/crates/bevy_reflect/derive/src/lib.rs @@ -160,21 +160,21 @@ fn match_reflect_impls(ast: DeriveInput, source: ReflectImplSource) -> TokenStre /// the type's [`Clone`] implementation. /// A custom implementation may be provided using `#[reflect(Clone(my_clone_func))]` where /// `my_clone_func` is the path to a function matching the signature: -/// `(&self) -> Self`. +/// `(&Self) -> Self`. /// * `#[reflect(Debug)]` will force the implementation of `Reflect::reflect_debug` to rely on /// the type's [`Debug`] implementation. /// A custom implementation may be provided using `#[reflect(Debug(my_debug_func))]` where /// `my_debug_func` is the path to a function matching the signature: -/// `(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result`. +/// `(&Self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result`. /// * `#[reflect(PartialEq)]` will force the implementation of `Reflect::reflect_partial_eq` to rely on /// the type's [`PartialEq`] implementation. /// A custom implementation may be provided using `#[reflect(PartialEq(my_partial_eq_func))]` where /// `my_partial_eq_func` is the path to a function matching the signature: -/// `(&self, value: &dyn #bevy_reflect_path::Reflect) -> bool`. +/// `(&Self, value: &dyn #bevy_reflect_path::Reflect) -> bool`. /// * `#[reflect(Hash)]` will force the implementation of `Reflect::reflect_hash` to rely on /// the type's [`Hash`] implementation. /// A custom implementation may be provided using `#[reflect(Hash(my_hash_func))]` where -/// `my_hash_func` is the path to a function matching the signature: `(&self) -> u64`. +/// `my_hash_func` is the path to a function matching the signature: `(&Self) -> u64`. /// * `#[reflect(Default)]` will register the `ReflectDefault` type data as normal. /// However, it will also affect how certain other operations are performed in order /// to improve performance and/or robustness. @@ -344,6 +344,18 @@ fn match_reflect_impls(ast: DeriveInput, source: ReflectImplSource) -> TokenStre /// What this does is register the `SerializationData` type within the `GetTypeRegistration` implementation, /// which will be used by the reflection serializers to determine whether or not the field is serializable. /// +/// ## `#[reflect(clone)]` +/// +/// This attribute affects the `Reflect::reflect_clone` implementation. +/// +/// Without this attribute, the implementation will rely on the field's own `Reflect::reflect_clone` implementation. +/// When this attribute is present, the implementation will instead use the field's `Clone` implementation directly. +/// +/// The attribute may also take the path to a custom function like `#[reflect(clone = "path::to::my_clone_func")]`, +/// where `my_clone_func` matches the signature `(&Self) -> Self`. +/// +/// This attribute does nothing if the containing struct/enum has the `#[reflect(Clone)]` attribute. +/// /// ## `#[reflect(@...)]` /// /// This attribute can be used to register custom attributes to the field's `TypeInfo`. diff --git a/crates/bevy_reflect/derive/src/struct_utility.rs b/crates/bevy_reflect/derive/src/struct_utility.rs index 09604419b6043..9bfd72de60596 100644 --- a/crates/bevy_reflect/derive/src/struct_utility.rs +++ b/crates/bevy_reflect/derive/src/struct_utility.rs @@ -1,5 +1,4 @@ -use crate::{derive_data::StructField, ReflectStruct}; -use quote::quote; +use crate::ReflectStruct; /// A helper struct for creating remote-aware field accessors. /// @@ -20,27 +19,15 @@ pub(crate) struct FieldAccessors { impl FieldAccessors { pub fn new(reflect_struct: &ReflectStruct) -> Self { - let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path(); - let fields_ref = Self::get_fields(reflect_struct, |field, accessor| { - match &field.attrs.remote { - Some(wrapper_ty) => { - quote! { - <#wrapper_ty as #bevy_reflect_path::ReflectRemote>::as_wrapper(&#accessor) - } - } - None => quote!(& #accessor), - } - }); - let fields_mut = Self::get_fields(reflect_struct, |field, accessor| { - match &field.attrs.remote { - Some(wrapper_ty) => { - quote! { - <#wrapper_ty as #bevy_reflect_path::ReflectRemote>::as_wrapper_mut(&mut #accessor) - } - } - None => quote!(&mut #accessor), - } - }); + let (fields_ref, fields_mut): (Vec<_>, Vec<_>) = reflect_struct + .active_fields() + .map(|field| { + ( + reflect_struct.access_for_field(field, false), + reflect_struct.access_for_field(field, true), + ) + }) + .unzip(); let field_count = fields_ref.len(); let field_indices = (0..field_count).collect(); @@ -52,30 +39,4 @@ impl FieldAccessors { field_count, } } - - fn get_fields( - reflect_struct: &ReflectStruct, - mut wrapper_fn: F, - ) -> Vec - where - F: FnMut(&StructField, proc_macro2::TokenStream) -> proc_macro2::TokenStream, - { - let is_remote = reflect_struct.meta().is_remote_wrapper(); - reflect_struct - .active_fields() - .map(|field| { - let member = crate::ident::ident_or_index( - field.data.ident.as_ref(), - field.declaration_index, - ); - let accessor = if is_remote { - quote!(self.0.#member) - } else { - quote!(self.#member) - }; - - wrapper_fn(field, accessor) - }) - .collect::>() - } } diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 7860d58f4ac9e..ee3bfd2278c3c 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -995,13 +995,6 @@ mod tests { let foo = Foo(123); let clone = foo.reflect_clone().unwrap(); assert_eq!(foo, clone.take::().unwrap()); - - #[derive(Reflect, Clone, Debug, PartialEq)] - struct Bar(usize); - - let bar = Bar(123); - let clone = bar.reflect_clone(); - assert!(clone.is_none()); } #[test] @@ -1019,6 +1012,125 @@ mod tests { assert_eq!(Foo(321), clone.take::().unwrap()); } + #[test] + fn should_not_clone_ignored_fields() { + #[derive(Reflect, Clone, Debug, PartialEq)] + struct Foo(#[reflect(ignore)] usize); + + let foo = Foo(123); + let clone = foo.reflect_clone(); + assert!(clone.is_none()); + } + + #[test] + fn should_clone_ignored_fields_with_clone_attributes() { + #[derive(Reflect, Clone, Debug, PartialEq)] + struct Foo(#[reflect(ignore, clone)] usize); + + let foo = Foo(123); + let clone = foo.reflect_clone().unwrap(); + assert_eq!(Foo(123), clone.take::().unwrap()); + + #[derive(Reflect, Clone, Debug, PartialEq)] + struct Bar(#[reflect(ignore, clone = "clone_usize")] usize); + + fn clone_usize(this: &usize) -> usize { + *this + 198 + } + + let bar = Bar(123); + let clone = bar.reflect_clone().unwrap(); + assert_eq!(Bar(321), clone.take::().unwrap()); + } + + #[test] + fn should_composite_reflect_clone() { + #[derive(Reflect, Debug, PartialEq)] + enum MyEnum { + Unit, + Tuple( + Foo, + #[reflect(ignore, clone)] Bar, + #[reflect(clone = "clone_baz")] Baz, + ), + Struct { + foo: Foo, + #[reflect(ignore, clone)] + bar: Bar, + #[reflect(clone = "clone_baz")] + baz: Baz, + }, + } + + #[derive(Reflect, Debug, PartialEq)] + struct Foo { + #[reflect(clone = "clone_bar")] + bar: Bar, + baz: Baz, + } + + #[derive(Reflect, Default, Clone, Debug, PartialEq)] + #[reflect(Clone)] + struct Bar(String); + + #[derive(Reflect, Debug, PartialEq)] + struct Baz(String); + + fn clone_bar(bar: &Bar) -> Bar { + Bar(format!("{}!", bar.0)) + } + + fn clone_baz(baz: &Baz) -> Baz { + Baz(format!("{}!", baz.0)) + } + + let my_enum = MyEnum::Unit; + let clone = my_enum.reflect_clone().unwrap(); + assert_eq!(MyEnum::Unit, clone.take::().unwrap()); + + let my_enum = MyEnum::Tuple( + Foo { + bar: Bar("bar".to_string()), + baz: Baz("baz".to_string()), + }, + Bar("bar".to_string()), + Baz("baz".to_string()), + ); + let clone = my_enum.reflect_clone().unwrap(); + assert_eq!( + MyEnum::Tuple( + Foo { + bar: Bar("bar!".to_string()), + baz: Baz("baz".to_string()), + }, + Bar("bar".to_string()), + Baz("baz!".to_string()), + ), + clone.take::().unwrap() + ); + + let my_enum = MyEnum::Struct { + foo: Foo { + bar: Bar("bar".to_string()), + baz: Baz("baz".to_string()), + }, + bar: Bar("bar".to_string()), + baz: Baz("baz".to_string()), + }; + let clone = my_enum.reflect_clone().unwrap(); + assert_eq!( + MyEnum::Struct { + foo: Foo { + bar: Bar("bar!".to_string()), + baz: Baz("baz".to_string()), + }, + bar: Bar("bar".to_string()), + baz: Baz("baz!".to_string()), + }, + clone.take::().unwrap() + ); + } + #[test] fn should_call_from_reflect_dynamically() { #[derive(Reflect)] From 512c4ee7f69c7ae956b98625a1b1f88ed6a64448 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sat, 18 May 2024 17:35:21 -0700 Subject: [PATCH 04/11] Support Reflect::reflect_clone on tuples --- crates/bevy_reflect/src/lib.rs | 12 +++++++++--- crates/bevy_reflect/src/tuple.rs | 8 ++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index ee3bfd2278c3c..0f71e4e3f0d8c 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -988,13 +988,19 @@ mod tests { #[test] fn should_reflect_clone() { + // Struct #[derive(Reflect, Clone, Debug, PartialEq)] #[reflect(Clone)] struct Foo(usize); - let foo = Foo(123); - let clone = foo.reflect_clone().unwrap(); - assert_eq!(foo, clone.take::().unwrap()); + let value = Foo(123); + let clone = value.reflect_clone().expect("should reflect_clone struct"); + assert_eq!(value, clone.take::().unwrap()); + + // Tuple + let foo = (123, 4.56); + let clone = foo.reflect_clone().expect("should reflect_clone tuple"); + assert_eq!(foo, clone.take::<(u32, f32)>().unwrap()); } #[test] diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index 28f95b6886cd8..91b86e49aa224 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -593,6 +593,14 @@ macro_rules! impl_reflect_tuple { fn try_apply(&mut self, value: &dyn PartialReflect) -> Result<(), ApplyError> { crate::tuple_try_apply(self, value) } + + fn reflect_clone(&self) -> Option> { + Some(Box::new(( + $( + self.$index.reflect_clone()?.take::<$name>().ok()?, + )* + ))) + } } impl<$($name: Reflect + MaybeTyped + TypePath + GetTypeRegistration),*> Reflect for ($($name,)*) { From d021b603cd5bb64b8b2705f72e9d2e99fd59eef1 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sat, 18 May 2024 17:44:28 -0700 Subject: [PATCH 05/11] Update docs on Reflect clone methods --- crates/bevy_reflect/src/reflect.rs | 46 +++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 441fd7f87ea57..bd66748af709d 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -216,27 +216,51 @@ where /// See [`ReflectOwned`]. fn reflect_owned(self: Box) -> ReflectOwned; - /// Clones the value as a `Reflect` trait object. + /// Clones `Self` into its dynamic representation. /// - /// When deriving `Reflect` for a struct, tuple struct or enum, the value is - /// cloned via [`Struct::clone_dynamic`], [`TupleStruct::clone_dynamic`], - /// or [`Enum::clone_dynamic`], respectively. - /// Implementors of other `Reflect` subtraits (e.g. [`List`], [`Map`]) should - /// use those subtraits' respective `clone_dynamic` methods. + /// For value types or types marked with `#[reflect_value]`, + /// this will simply return a clone of `Self`. + /// + /// Otherwise the associated dynamic type will be returned. + /// + /// For example, a [`List`] type will invoke [`List::clone_dynamic`], returning [`DynamicList`]. + /// A [`Struct`] type will invoke [`Struct::clone_dynamic`], returning [`DynamicStruct`]. + /// And so on. + /// + /// If the dynamic behavior is not desired, a concrete clone can be obtained using [`PartialReflect::reflect_clone`]. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::{Reflect, DynamicTuple}; + /// let value = (1, true, 3.14); + /// let cloned = value.clone_value(); + /// assert!(cloned.is::()) + /// ``` /// - /// [`Struct::clone_dynamic`]: crate::Struct::clone_dynamic - /// [`TupleStruct::clone_dynamic`]: crate::TupleStruct::clone_dynamic - /// [`Enum::clone_dynamic`]: crate::Enum::clone_dynamic /// [`List`]: crate::List - /// [`Map`]: crate::Map + /// [`List::clone_dynamic`]: crate::List::clone_dynamic + /// [`DynamicList`]: crate::DynamicList + /// [`Struct`]: crate::Struct + /// [`Struct::clone_dynamic`]: crate::Struct::clone_dynamic + /// [`DynamicStruct`]: crate::DynamicStruct fn clone_value(&self) -> Box; /// Attempts to clone `Self` using reflection. /// - /// Unlike [`Reflect::clone_value`], which often returns a dynamic representation of `Self`, + /// Unlike [`PartialReflect::clone_value`], which often returns a dynamic representation of `Self`, /// this method attempts create a clone of `Self` directly, if possible. /// /// If the clone cannot be performed, `None` is returned. + /// + /// # Example + /// + /// ``` + /// # use bevy_reflect::Reflect; + /// let value = (1, true, 3.14); + /// let cloned = value.reflect_clone().unwrap(); + /// assert!(cloned.is::<(i32, bool, f64)>()) + /// ``` fn reflect_clone(&self) -> Option> { None } From f7616a99eaed13ea4454a4178c938ea389efc054 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 19 May 2024 18:20:14 -0700 Subject: [PATCH 06/11] Add test for generic types --- crates/bevy_reflect/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 0f71e4e3f0d8c..f648c5fbf574d 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -997,6 +997,18 @@ mod tests { let clone = value.reflect_clone().expect("should reflect_clone struct"); assert_eq!(value, clone.take::().unwrap()); + // Generic Struct + #[derive(Reflect, Debug, PartialEq)] + struct Bar(T, #[reflect(ignore, clone)] PhantomData); + #[derive(TypePath, Debug, PartialEq)] + struct Baz; + + let value = Bar::(123, PhantomData); + let clone = value + .reflect_clone() + .expect("should reflect_clone generic struct"); + assert_eq!(value, clone.take::>().unwrap()); + // Tuple let foo = (123, 4.56); let clone = foo.reflect_clone().expect("should reflect_clone tuple"); From a49e03dcc08cf3a0e56956c6e6269c07c11f3e78 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 20 May 2024 13:35:30 -0700 Subject: [PATCH 07/11] Better separate tests --- crates/bevy_reflect/src/lib.rs | 88 +++++++++++++++++++++++++++++----- 1 file changed, 77 insertions(+), 11 deletions(-) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index f648c5fbf574d..9046d5a61d644 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -989,30 +989,96 @@ mod tests { #[test] fn should_reflect_clone() { // Struct - #[derive(Reflect, Clone, Debug, PartialEq)] - #[reflect(Clone)] + #[derive(Reflect, Debug, PartialEq)] struct Foo(usize); let value = Foo(123); let clone = value.reflect_clone().expect("should reflect_clone struct"); assert_eq!(value, clone.take::().unwrap()); - // Generic Struct + // Tuple + let foo = (123, 4.56); + let clone = foo.reflect_clone().expect("should reflect_clone tuple"); + assert_eq!(foo, clone.take::<(u32, f32)>().unwrap()); + } + + #[test] + fn should_reflect_clone_generic_type() { #[derive(Reflect, Debug, PartialEq)] - struct Bar(T, #[reflect(ignore, clone)] PhantomData); + struct Foo(T, #[reflect(ignore, clone)] PhantomData); #[derive(TypePath, Debug, PartialEq)] - struct Baz; + struct Bar; - let value = Bar::(123, PhantomData); + // `usize` will be cloned via `Reflect::reflect_clone` + // `PhantomData` will be cloned via `Clone::clone` + let value = Foo::(123, PhantomData); let clone = value .reflect_clone() .expect("should reflect_clone generic struct"); - assert_eq!(value, clone.take::>().unwrap()); + assert_eq!(value, clone.take::>().unwrap()); + } - // Tuple - let foo = (123, 4.56); - let clone = foo.reflect_clone().expect("should reflect_clone tuple"); - assert_eq!(foo, clone.take::<(u32, f32)>().unwrap()); + #[test] + fn should_reflect_clone_with_clone() { + // A custom clone function to verify that the `#[reflect(Clone)]` container attribute + // takes precedence over the `#[reflect(clone)]` field attribute. + #[allow(dead_code, unused_variables)] + fn custom_clone(value: &usize) -> usize { + panic!("should not be called"); + } + + // Tuple Struct + #[derive(Reflect, Clone, Debug, PartialEq)] + #[reflect(Clone)] + struct Foo(#[reflect(clone = "custom_clone")] usize); + + let value = Foo(123); + let clone = value + .reflect_clone() + .expect("should reflect_clone tuple struct"); + assert_eq!(value, clone.take::().unwrap()); + + // Struct + #[derive(Reflect, Clone, Debug, PartialEq)] + #[reflect(Clone)] + struct Bar { + #[reflect(clone = "custom_clone")] + value: usize, + } + + let value = Bar { value: 123 }; + let clone = value.reflect_clone().expect("should reflect_clone struct"); + assert_eq!(value, clone.take::().unwrap()); + + // Enum + #[derive(Reflect, Clone, Debug, PartialEq)] + #[reflect(Clone)] + enum Baz { + Unit, + Tuple(#[reflect(clone = "custom_clone")] usize), + Struct { + #[reflect(clone = "custom_clone")] + value: usize, + }, + } + + let value = Baz::Unit; + let clone = value + .reflect_clone() + .expect("should reflect_clone unit variant"); + assert_eq!(value, clone.take::().unwrap()); + + let value = Baz::Tuple(123); + let clone = value + .reflect_clone() + .expect("should reflect_clone tuple variant"); + assert_eq!(value, clone.take::().unwrap()); + + let value = Baz::Struct { value: 123 }; + let clone = value + .reflect_clone() + .expect("should reflect_clone struct variant"); + assert_eq!(value, clone.take::().unwrap()); } #[test] From bc009273e194f52f1e7ee643351f829760ab5ad6 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 21 May 2024 12:43:41 -0700 Subject: [PATCH 08/11] Change return type to Result --- .../derive/src/container_attributes.rs | 10 +- crates/bevy_reflect/derive/src/derive_data.rs | 64 +++++-- .../bevy_reflect/derive/src/enum_utility.rs | 169 +++++++++--------- crates/bevy_reflect/src/error.rs | 61 +++++++ crates/bevy_reflect/src/fields.rs | 18 ++ crates/bevy_reflect/src/impls/std.rs | 125 +++++++++++-- crates/bevy_reflect/src/lib.rs | 62 ++++++- crates/bevy_reflect/src/reflect.rs | 14 +- crates/bevy_reflect/src/tuple.rs | 14 +- 9 files changed, 412 insertions(+), 125 deletions(-) create mode 100644 crates/bevy_reflect/src/error.rs diff --git a/crates/bevy_reflect/derive/src/container_attributes.rs b/crates/bevy_reflect/derive/src/container_attributes.rs index 3298f426e3c30..2cec1db0b44b2 100644 --- a/crates/bevy_reflect/derive/src/container_attributes.rs +++ b/crates/bevy_reflect/derive/src/container_attributes.rs @@ -9,7 +9,7 @@ use crate::{ attribute_parser::terminated_parser, custom_attributes::CustomAttributes, derive_data::ReflectTraitToImpl, }; -use bevy_macro_utils::fq_std::{FQAny, FQBox, FQClone, FQOption}; +use bevy_macro_utils::fq_std::{FQAny, FQClone, FQOption, FQResult}; use proc_macro2::{Ident, Span}; use quote::quote_spanned; use syn::{ @@ -551,14 +551,14 @@ impl ContainerAttributes { match &self.clone { &TraitImpl::Implemented(span) => Some(quote_spanned! {span=> #[inline] - fn reflect_clone(&self) -> #FQOption<#FQBox> { - #FQOption::Some(#FQBox::new(#FQClone::clone(self))) + fn reflect_clone(&self) -> #FQResult<#bevy_reflect_path::__macro_exports::alloc_utils::Box, #bevy_reflect_path::ReflectCloneError> { + #FQResult::Ok(#bevy_reflect_path::__macro_exports::alloc_utils::Box::new(#FQClone::clone(self))) } }), &TraitImpl::Custom(ref impl_fn, span) => Some(quote_spanned! {span=> #[inline] - fn reflect_clone(&self) -> #FQOption<#FQBox> { - #FQOption::Some(#FQBox::new(#impl_fn(self))) + fn reflect_clone(&self) -> #FQResult<#bevy_reflect_path::__macro_exports::alloc_utils::Box, #bevy_reflect_path::ReflectCloneError> { + #FQResult::Ok(#bevy_reflect_path::__macro_exports::alloc_utils::Box::new(#impl_fn(self))) } }), TraitImpl::NotImplemented => None, diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index 01b0c51840516..f9e1f7305808d 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -18,7 +18,7 @@ use syn::token::Comma; use crate::enum_utility::{EnumVariantOutputData, ReflectCloneVariantBuilder, VariantBuilder}; use crate::field_attributes::CloneBehavior; use crate::generics::generate_generics; -use bevy_macro_utils::fq_std::{FQBox, FQClone, FQOption}; +use bevy_macro_utils::fq_std::{FQClone, FQOption, FQResult}; use syn::{ parse_str, punctuated::Punctuated, spanned::Spanned, Data, DeriveInput, Field, Fields, GenericParam, Generics, Ident, LitStr, Member, Meta, Path, PathSegment, Type, TypeParam, @@ -561,6 +561,20 @@ impl<'a> StructField<'a> { None => Member::Unnamed(self.declaration_index.into()), } } + + /// Returns a token stream for generating a `FieldId` for this field. + pub fn field_id(&self, bevy_reflect_path: &Path) -> proc_macro2::TokenStream { + match &self.data.ident { + Some(ident) => { + let name = ident.to_string(); + quote!(#bevy_reflect_path::FieldId::Named(#bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed(#name))) + } + None => { + let index = self.declaration_index; + quote!(#bevy_reflect_path::FieldId::Unnamed(#index)) + } + } + } } impl<'a> ReflectStruct<'a> { @@ -681,17 +695,43 @@ impl<'a> ReflectStruct<'a> { let mut tokens = proc_macro2::TokenStream::new(); for field in self.fields().iter() { + let field_ty = field.reflected_type(); let member = field.to_member(); let accessor = self.access_for_field(field, false); match &field.attrs.clone { CloneBehavior::Default => { - if field.attrs.ignore.is_ignored() { - return None; - } + let value = if field.attrs.ignore.is_ignored() { + let field_id = field.field_id(bevy_reflect_path); + + quote! { + return #FQResult::Err(#bevy_reflect_path::ReflectCloneError::FieldNotClonable { + field: #field_id, + variant: #FQOption::None, + container_type_path: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed( + ::type_path() + ) + }) + } + } else { + quote! { + #bevy_reflect_path::PartialReflect::reflect_clone(#accessor)? + .take() + .map_err(|value| #bevy_reflect_path::ReflectCloneError::FailedDowncast { + expected: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed( + <#field_ty as #bevy_reflect_path::TypePath>::type_path() + ), + received: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Owned( + #bevy_reflect_path::__macro_exports::alloc_utils::ToString::to_string( + #bevy_reflect_path::DynamicTypePath::reflect_type_path(&*value) + ) + ), + })? + } + }; tokens.extend(quote! { - #member: #bevy_reflect_path::PartialReflect::reflect_clone(#accessor)?.take().ok()?, + #member: #value, }); } CloneBehavior::Trait => { @@ -727,8 +767,9 @@ impl<'a> ReflectStruct<'a> { Some(quote! { #[inline] - fn reflect_clone(&self) -> #FQOption<#FQBox> { - #FQOption::Some(#FQBox::new(#ctor)) + #[allow(unreachable_code, reason = "Ignored fields without a `clone` attribute will early-return with an error")] + fn reflect_clone(&self) -> #FQResult<#bevy_reflect_path::__macro_exports::alloc_utils::Box, #bevy_reflect_path::ReflectCloneError> { + #FQResult::Ok(#bevy_reflect_path::__macro_exports::alloc_utils::Box::new(#ctor)) } }) } @@ -888,7 +929,7 @@ impl<'a> ReflectEnum<'a> { variant_patterns, variant_constructors, .. - } = ReflectCloneVariantBuilder::new(self).build(&this)?; + } = ReflectCloneVariantBuilder::new(self).build(&this); let inner = quote! { match #this { @@ -899,18 +940,19 @@ impl<'a> ReflectEnum<'a> { let body = if self.meta.is_remote_wrapper() { quote! { let #this = ::as_remote(self); - #FQOption::Some(#FQBox::new(::into_wrapper(#inner))) + #FQResult::Ok(#bevy_reflect_path::__macro_exports::alloc_utils::Box::new(::into_wrapper(#inner))) } } else { quote! { let #this = self; - #FQOption::Some(#FQBox::new(#inner)) + #FQResult::Ok(#bevy_reflect_path::__macro_exports::alloc_utils::Box::new(#inner)) } }; Some(quote! { #[inline] - fn reflect_clone(&self) -> #FQOption<#FQBox> { + #[allow(unreachable_code, reason = "Ignored fields without a `clone` attribute will early-return with an error")] + fn reflect_clone(&self) -> #FQResult<#bevy_reflect_path::__macro_exports::alloc_utils::Box, #bevy_reflect_path::ReflectCloneError> { #body } }) diff --git a/crates/bevy_reflect/derive/src/enum_utility.rs b/crates/bevy_reflect/derive/src/enum_utility.rs index e0d6ea6c1bc6f..bd21983304f68 100644 --- a/crates/bevy_reflect/derive/src/enum_utility.rs +++ b/crates/bevy_reflect/derive/src/enum_utility.rs @@ -3,7 +3,7 @@ use crate::{ derive_data::ReflectEnum, derive_data::StructField, field_attributes::DefaultBehavior, ident::ident_or_index, }; -use bevy_macro_utils::fq_std::{FQClone, FQDefault, FQOption}; +use bevy_macro_utils::fq_std::{FQClone, FQDefault, FQOption, FQResult}; use proc_macro2::{Ident, TokenStream}; use quote::{format_ident, quote, ToTokens}; @@ -36,9 +36,6 @@ pub(crate) struct VariantField<'a, 'b> { /// Trait used to control how enum variants are built. pub(crate) trait VariantBuilder: Sized { - /// The variant output data. - type Output; - /// Returns the enum data. fn reflect_enum(&self) -> &ReflectEnum; @@ -133,83 +130,79 @@ pub(crate) trait VariantBuilder: Sized { /// Returns a token stream that constructs an instance of an ignored field. /// - /// If the ignored field cannot be adequately handled, `None` should be returned. - /// /// # Parameters /// * `field`: The field to access - fn on_ignored_field(&self, field: VariantField) -> Option { - Some(match &field.field.attrs.default { + fn on_ignored_field(&self, field: VariantField) -> TokenStream { + match &field.field.attrs.default { DefaultBehavior::Func(path) => quote! { #path() }, _ => quote! { #FQDefault::default() }, - }) + } } /// Builds the enum variant output data. - fn build(&self, this: &Ident) -> Self::Output; -} + fn build(&self, this: &Ident) -> EnumVariantOutputData { + let variants = self.reflect_enum().variants(); -fn build(builder: &V, this: &Ident) -> Option { - let variants = builder.reflect_enum().variants(); + let mut variant_names = Vec::with_capacity(variants.len()); + let mut variant_patterns = Vec::with_capacity(variants.len()); + let mut variant_constructors = Vec::with_capacity(variants.len()); - let mut variant_names = Vec::with_capacity(variants.len()); - let mut variant_patterns = Vec::with_capacity(variants.len()); - let mut variant_constructors = Vec::with_capacity(variants.len()); + for variant in variants { + let variant_ident = &variant.data.ident; + let variant_name = variant_ident.to_string(); + let variant_path = self.reflect_enum().get_unit(variant_ident); - for variant in variants { - let variant_ident = &variant.data.ident; - let variant_name = variant_ident.to_string(); - let variant_path = builder.reflect_enum().get_unit(variant_ident); + let fields = variant.fields(); - let fields = variant.fields(); + let mut field_patterns = Vec::with_capacity(fields.len()); + let mut field_constructors = Vec::with_capacity(fields.len()); - let mut field_patterns = Vec::with_capacity(fields.len()); - let mut field_constructors = Vec::with_capacity(fields.len()); + for field in fields { + let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); + let alias = format_ident!("_{}", member); - for field in fields { - let member = ident_or_index(field.data.ident.as_ref(), field.declaration_index); - let alias = format_ident!("_{}", member); + let variant_field = VariantField { + alias: &alias, + variant_name: &variant_name, + field, + }; - let variant_field = VariantField { - alias: &alias, - variant_name: &variant_name, - field, - }; + let value = if field.attrs.ignore.is_ignored() { + self.on_ignored_field(variant_field) + } else { + self.on_active_field(this, variant_field) + }; - let value = if field.attrs.ignore.is_ignored() { - builder.on_ignored_field(variant_field)? - } else { - builder.on_active_field(this, variant_field) - }; + field_patterns.push(quote! { + #member: #alias + }); - field_patterns.push(quote! { - #member: #alias - }); + field_constructors.push(quote! { + #member: #value + }); + } - field_constructors.push(quote! { - #member: #value - }); - } + let pattern = quote! { + #variant_path { #( #field_patterns ),* } + }; - let pattern = quote! { - #variant_path { #( #field_patterns ),* } - }; + let constructor = quote! { + #variant_path { + #( #field_constructors ),* + } + }; - let constructor = quote! { - #variant_path { - #( #field_constructors ),* - } - }; + variant_names.push(variant_name); + variant_patterns.push(pattern); + variant_constructors.push(constructor); + } - variant_names.push(variant_name); - variant_patterns.push(pattern); - variant_constructors.push(constructor); + EnumVariantOutputData { + variant_names, + variant_patterns, + variant_constructors, + } } - - Some(EnumVariantOutputData { - variant_names, - variant_patterns, - variant_constructors, - }) } /// Generates the enum variant output data needed to build the `FromReflect::from_reflect` implementation. @@ -224,8 +217,6 @@ impl<'a> FromReflectVariantBuilder<'a> { } impl<'a> VariantBuilder for FromReflectVariantBuilder<'a> { - type Output = EnumVariantOutputData; - fn reflect_enum(&self) -> &ReflectEnum { self.reflect_enum } @@ -244,10 +235,6 @@ impl<'a> VariantBuilder for FromReflectVariantBuilder<'a> { <#field_ty as #bevy_reflect_path::FromReflect>::from_reflect(#alias)? } } - - fn build(&self, this: &Ident) -> Self::Output { - build(self, this).expect("internal bevy_reflect error: ignored fields should be handled") - } } /// Generates the enum variant output data needed to build the `PartialReflect::try_apply` implementation. @@ -262,8 +249,6 @@ impl<'a> TryApplyVariantBuilder<'a> { } impl<'a> VariantBuilder for TryApplyVariantBuilder<'a> { - type Output = EnumVariantOutputData; - fn reflect_enum(&self) -> &ReflectEnum { self.reflect_enum } @@ -306,10 +291,6 @@ impl<'a> VariantBuilder for TryApplyVariantBuilder<'a> { })? } } - - fn build(&self, this: &Ident) -> Self::Output { - build(self, this).expect("internal bevy_reflect error: ignored fields should be handled") - } } /// Generates the enum variant output data needed to build the `Reflect::reflect_clone` implementation. @@ -324,7 +305,6 @@ impl<'a> ReflectCloneVariantBuilder<'a> { } impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> { - type Output = Option; fn reflect_enum(&self) -> &ReflectEnum { self.reflect_enum } @@ -342,6 +322,8 @@ impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> { fn construct_field(&self, field: VariantField) -> TokenStream { let bevy_reflect_path = self.reflect_enum.meta().bevy_reflect_path(); + let field_ty = field.field.reflected_type(); + let alias = field.alias; let alias = match &field.field.attrs.remote { Some(wrapper_ty) => { @@ -355,7 +337,18 @@ impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> { match &field.field.attrs.clone { CloneBehavior::Default => { quote! { - #bevy_reflect_path::PartialReflect::reflect_clone(#alias)?.take().ok()? + #bevy_reflect_path::PartialReflect::reflect_clone(#alias)? + .take() + .map_err(|value| #bevy_reflect_path::ReflectCloneError::FailedDowncast { + expected: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed( + <#field_ty as #bevy_reflect_path::TypePath>::type_path() + ), + received: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Owned( + #bevy_reflect_path::__macro_exports::alloc_utils::ToString::to_string( + #bevy_reflect_path::DynamicTypePath::reflect_type_path(&*value) + ) + ), + })? } } CloneBehavior::Trait => { @@ -375,19 +368,27 @@ impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> { self.construct_field(field) } - fn on_ignored_field(&self, field: VariantField) -> Option { + fn on_ignored_field(&self, field: VariantField) -> TokenStream { + let bevy_reflect_path = self.reflect_enum.meta().bevy_reflect_path(); + let variant_name = field.variant_name; let alias = field.alias; match &field.field.attrs.clone { - CloneBehavior::Default => None, - CloneBehavior::Trait => Some(quote! { - #FQClone::clone(#alias) - }), - CloneBehavior::Func(clone_fn) => Some(quote! { #clone_fn() }), - } - } + CloneBehavior::Default => { + let field_id = field.field.field_id(bevy_reflect_path); - fn build(&self, this: &Ident) -> Self::Output { - build(self, this) + quote! { + return #FQResult::Err( + #bevy_reflect_path::ReflectCloneError::FieldNotClonable { + field: #field_id, + variant: #FQOption::Some(#bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed(#variant_name)), + container_type_path: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed(::type_path()) + } + ) + } + } + CloneBehavior::Trait => quote! { #FQClone::clone(#alias) }, + CloneBehavior::Func(clone_fn) => quote! { #clone_fn() }, + } } } diff --git a/crates/bevy_reflect/src/error.rs b/crates/bevy_reflect/src/error.rs new file mode 100644 index 0000000000000..bcd12d1f6ecce --- /dev/null +++ b/crates/bevy_reflect/src/error.rs @@ -0,0 +1,61 @@ +use crate::FieldId; +use alloc::{borrow::Cow, format}; +use thiserror::Error; + +/// An error that occurs when cloning a type via [`Reflect::reflect_clone`]. +/// +/// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone +#[derive(Clone, Debug, Error, PartialEq, Eq)] +pub enum ReflectCloneError { + /// The type does not have a custom implementation for [`Reflect::reflect_clone`]. + /// + /// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone + #[error("`Reflect::reflect_clone` not implemented for `{type_path}`")] + NotImplemented { type_path: Cow<'static, str> }, + /// The type cannot be cloned via [`Reflect::reflect_clone`]. + /// + /// This type should be returned when a type is intentionally opting out of reflection cloning. + /// + /// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone + #[error("`{type_path}` cannot be made clonable for `Reflect::reflect_clone`")] + NotClonable { type_path: Cow<'static, str> }, + /// The field cannot be cloned via [`Reflect::reflect_clone`]. + /// + /// When [deriving `Reflect`], this usually means that a field marked with `#[reflect(ignore)]` + /// is missing a `#[reflect(clone)]` attribute. + /// + /// This may be intentional if the field is not meant/able to be cloned. + /// + /// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone + /// [deriving `Reflect`]: derive@crate::Reflect + #[error( + "field `{}` cannot be made clonable for `Reflect::reflect_clone` (are you missing a `#[reflect(clone)]` attribute?)", + full_path(.field, .variant, .container_type_path) + )] + FieldNotClonable { + field: FieldId, + variant: Option>, + container_type_path: Cow<'static, str>, + }, + /// Could not downcast to the expected type. + /// + /// Realistically this should only occur when a type has incorrectly implemented [`Reflect`]. + /// + /// [`Reflect`]: crate::Reflect + #[error("expected downcast to `{expected}`, but received `{received}`")] + FailedDowncast { + expected: Cow<'static, str>, + received: Cow<'static, str>, + }, +} + +fn full_path( + field: &FieldId, + variant: &Option>, + container_type_path: &Cow<'static, str>, +) -> alloc::string::String { + match variant { + Some(variant) => format!("{}::{}::{}", container_type_path, variant, field), + None => format!("{}::{}", container_type_path, field), + } +} diff --git a/crates/bevy_reflect/src/fields.rs b/crates/bevy_reflect/src/fields.rs index 3a521c21ccbee..393bfcb2e2ea6 100644 --- a/crates/bevy_reflect/src/fields.rs +++ b/crates/bevy_reflect/src/fields.rs @@ -3,7 +3,9 @@ use crate::{ type_info::impl_type_methods, MaybeTyped, PartialReflect, Type, TypeInfo, TypePath, }; +use alloc::borrow::Cow; use bevy_platform_support::sync::Arc; +use core::fmt::{Display, Formatter}; /// The named field of a reflected struct. #[derive(Clone, Debug)] @@ -129,3 +131,19 @@ impl UnnamedField { impl_custom_attribute_methods!(self.custom_attributes, "field"); } + +/// A representation of a field's accessor. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum FieldId { + Named(Cow<'static, str>), + Unnamed(usize), +} + +impl Display for FieldId { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + match self { + Self::Named(name) => Display::fmt(name, f), + Self::Unnamed(index) => Display::fmt(index, f), + } + } +} diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 72381839b7ff2..73fc831bea9e6 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -11,9 +11,10 @@ use crate::{ utility::{reflect_hasher, GenericTypeInfoCell, GenericTypePathCell, NonGenericTypeInfoCell}, ApplyError, Array, ArrayInfo, ArrayIter, DynamicMap, DynamicSet, DynamicTypePath, FromReflect, FromType, Generics, GetTypeRegistration, List, ListInfo, ListIter, Map, MapInfo, MapIter, - MaybeTyped, OpaqueInfo, PartialReflect, Reflect, ReflectDeserialize, ReflectFromPtr, - ReflectFromReflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, ReflectSerialize, Set, - SetInfo, TypeInfo, TypeParamInfo, TypePath, TypeRegistration, TypeRegistry, Typed, + MaybeTyped, OpaqueInfo, PartialReflect, Reflect, ReflectCloneError, ReflectDeserialize, + ReflectFromPtr, ReflectFromReflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, + ReflectSerialize, Set, SetInfo, TypeInfo, TypeParamInfo, TypePath, TypeRegistration, + TypeRegistry, Typed, }; use alloc::{ borrow::{Cow, ToOwned}, @@ -51,10 +52,42 @@ impl_reflect_opaque!(char( Deserialize, Default )); -impl_reflect_opaque!(u8(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(u16(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(u32(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(u64(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(u8( + Clone, + Debug, + Hash, + PartialEq, + Serialize, + Deserialize, + Default +)); +impl_reflect_opaque!(u16( + Clone, + Debug, + Hash, + PartialEq, + Serialize, + Deserialize, + Default +)); +impl_reflect_opaque!(u32( + Clone, + Debug, + Hash, + PartialEq, + Serialize, + Deserialize, + Default +)); +impl_reflect_opaque!(u64( + Clone, + Debug, + Hash, + PartialEq, + Serialize, + Deserialize, + Default +)); impl_reflect_opaque!(u128( Clone, Debug, @@ -73,10 +106,42 @@ impl_reflect_opaque!(usize( Deserialize, Default )); -impl_reflect_opaque!(i8(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(i16(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(i32(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(i64(Clone, Debug, Hash, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(i8( + Clone, + Debug, + Hash, + PartialEq, + Serialize, + Deserialize, + Default +)); +impl_reflect_opaque!(i16( + Clone, + Debug, + Hash, + PartialEq, + Serialize, + Deserialize, + Default +)); +impl_reflect_opaque!(i32( + Clone, + Debug, + Hash, + PartialEq, + Serialize, + Deserialize, + Default +)); +impl_reflect_opaque!(i64( + Clone, + Debug, + Hash, + PartialEq, + Serialize, + Deserialize, + Default +)); impl_reflect_opaque!(i128( Clone, Debug, @@ -95,8 +160,22 @@ impl_reflect_opaque!(isize( Deserialize, Default )); -impl_reflect_opaque!(f32(Clone, Debug, PartialEq, Serialize, Deserialize, Default)); -impl_reflect_opaque!(f64(Clone, Debug, PartialEq, Serialize, Deserialize, Default)); +impl_reflect_opaque!(f32( + Clone, + Debug, + PartialEq, + Serialize, + Deserialize, + Default +)); +impl_reflect_opaque!(f64( + Clone, + Debug, + PartialEq, + Serialize, + Deserialize, + Default +)); impl_type_path!(str); impl_reflect_opaque!(::alloc::string::String( Clone, @@ -1638,6 +1717,10 @@ impl PartialReflect for Cow<'static, str> { Box::new(self.clone()) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(self.clone())) + } + fn reflect_hash(&self) -> Option { let mut hasher = reflect_hasher(); Hash::hash(&Any::type_id(self), &mut hasher); @@ -1826,6 +1909,10 @@ impl Parti Box::new(List::clone_dynamic(self)) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(self.clone())) + } + fn reflect_hash(&self) -> Option { crate::list_hash(self) } @@ -1935,6 +2022,10 @@ impl PartialReflect for &'static str { Box::new(*self) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(*self)) + } + fn reflect_hash(&self) -> Option { let mut hasher = reflect_hasher(); Hash::hash(&Any::type_id(self), &mut hasher); @@ -2074,6 +2165,10 @@ impl PartialReflect for &'static Path { Box::new(*self) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(*self)) + } + fn reflect_hash(&self) -> Option { let mut hasher = reflect_hasher(); Hash::hash(&Any::type_id(self), &mut hasher); @@ -2213,6 +2308,10 @@ impl PartialReflect for Cow<'static, Path> { Box::new(self.clone()) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(self.clone())) + } + fn reflect_hash(&self) -> Option { let mut hasher = reflect_hasher(); Hash::hash(&Any::type_id(self), &mut hasher); diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 9046d5a61d644..8448684bbbdd6 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -568,6 +568,7 @@ extern crate alloc; extern crate self as bevy_reflect; mod array; +mod error; mod fields; mod from_reflect; #[cfg(feature = "functions")] @@ -633,6 +634,7 @@ pub mod prelude { pub use array::*; pub use enums::*; +pub use error::*; pub use fields::*; pub use from_reflect::*; pub use generics::*; @@ -1098,12 +1100,70 @@ mod tests { #[test] fn should_not_clone_ignored_fields() { + // Tuple Struct #[derive(Reflect, Clone, Debug, PartialEq)] struct Foo(#[reflect(ignore)] usize); let foo = Foo(123); let clone = foo.reflect_clone(); - assert!(clone.is_none()); + assert_eq!( + clone.unwrap_err(), + ReflectCloneError::FieldNotClonable { + field: FieldId::Unnamed(0), + variant: None, + container_type_path: Cow::Borrowed(Foo::type_path()), + } + ); + + // Struct + #[derive(Reflect, Clone, Debug, PartialEq)] + struct Bar { + #[reflect(ignore)] + value: usize, + } + + let bar = Bar { value: 123 }; + let clone = bar.reflect_clone(); + assert_eq!( + clone.unwrap_err(), + ReflectCloneError::FieldNotClonable { + field: FieldId::Named(Cow::Borrowed("value")), + variant: None, + container_type_path: Cow::Borrowed(Bar::type_path()), + } + ); + + // Enum + #[derive(Reflect, Clone, Debug, PartialEq)] + enum Baz { + Tuple(#[reflect(ignore)] usize), + Struct { + #[reflect(ignore)] + value: usize, + }, + } + + let baz = Baz::Tuple(123); + let clone = baz.reflect_clone(); + assert_eq!( + clone.unwrap_err(), + ReflectCloneError::FieldNotClonable { + field: FieldId::Unnamed(0), + variant: Some(Cow::Borrowed("Tuple")), + container_type_path: Cow::Borrowed(Baz::type_path()), + } + ); + + let baz = Baz::Struct { value: 123 }; + let clone = baz.reflect_clone(); + assert_eq!( + clone.unwrap_err(), + ReflectCloneError::FieldNotClonable { + field: FieldId::Named(Cow::Borrowed("value")), + variant: Some(Cow::Borrowed("Struct")), + container_type_path: Cow::Borrowed(Baz::type_path()), + } + ); } #[test] diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index bd66748af709d..b9e9288b525f0 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -1,9 +1,11 @@ use crate::{ array_debug, enum_debug, list_debug, map_debug, set_debug, struct_debug, tuple_debug, - tuple_struct_debug, DynamicTypePath, DynamicTyped, OpaqueInfo, ReflectKind, + tuple_struct_debug, DynamicTypePath, DynamicTyped, OpaqueInfo, ReflectCloneError, ReflectKind, ReflectKindMismatchError, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypePath, Typed, }; +use alloc::borrow::Cow; use alloc::boxed::Box; +use alloc::string::ToString; use core::{ any::{Any, TypeId}, fmt::Debug, @@ -226,7 +228,7 @@ where /// For example, a [`List`] type will invoke [`List::clone_dynamic`], returning [`DynamicList`]. /// A [`Struct`] type will invoke [`Struct::clone_dynamic`], returning [`DynamicStruct`]. /// And so on. - /// + /// /// If the dynamic behavior is not desired, a concrete clone can be obtained using [`PartialReflect::reflect_clone`]. /// /// # Example @@ -251,7 +253,7 @@ where /// Unlike [`PartialReflect::clone_value`], which often returns a dynamic representation of `Self`, /// this method attempts create a clone of `Self` directly, if possible. /// - /// If the clone cannot be performed, `None` is returned. + /// If the clone cannot be performed, an appropriate [`ReflectCloneError`] is returned. /// /// # Example /// @@ -261,8 +263,10 @@ where /// let cloned = value.reflect_clone().unwrap(); /// assert!(cloned.is::<(i32, bool, f64)>()) /// ``` - fn reflect_clone(&self) -> Option> { - None + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Err(ReflectCloneError::NotImplemented { + type_path: Cow::Owned(self.reflect_type_path().to_string()), + }) } /// Returns a hash of the value (which includes the type). diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index 91b86e49aa224..1916a92f7eb18 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -4,9 +4,9 @@ use variadics_please::all_tuples; use crate::generics::impl_generic_info_methods; use crate::{ type_info::impl_type_methods, utility::GenericTypePathCell, ApplyError, FromReflect, Generics, - GetTypeRegistration, MaybeTyped, PartialReflect, Reflect, ReflectKind, ReflectMut, - ReflectOwned, ReflectRef, Type, TypeInfo, TypePath, TypeRegistration, TypeRegistry, Typed, - UnnamedField, + GetTypeRegistration, MaybeTyped, PartialReflect, Reflect, ReflectCloneError, ReflectKind, + ReflectMut, ReflectOwned, ReflectRef, Type, TypeInfo, TypePath, TypeRegistration, TypeRegistry, + Typed, UnnamedField, }; use alloc::{boxed::Box, vec, vec::Vec}; use core::{ @@ -594,10 +594,12 @@ macro_rules! impl_reflect_tuple { crate::tuple_try_apply(self, value) } - fn reflect_clone(&self) -> Option> { - Some(Box::new(( + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(( $( - self.$index.reflect_clone()?.take::<$name>().ok()?, + self.$index.reflect_clone()? + .take::<$name>() + .expect("`Reflect::reflect_clone` should return the same type"), )* ))) } From fb7c64768fef0f010860b0360ada64c40250bc58 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Wed, 22 May 2024 16:17:17 -0700 Subject: [PATCH 09/11] Fix CI errors --- crates/bevy_reflect/src/error.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_reflect/src/error.rs b/crates/bevy_reflect/src/error.rs index bcd12d1f6ecce..0d78e363bd346 100644 --- a/crates/bevy_reflect/src/error.rs +++ b/crates/bevy_reflect/src/error.rs @@ -30,7 +30,7 @@ pub enum ReflectCloneError { /// [deriving `Reflect`]: derive@crate::Reflect #[error( "field `{}` cannot be made clonable for `Reflect::reflect_clone` (are you missing a `#[reflect(clone)]` attribute?)", - full_path(.field, .variant, .container_type_path) + full_path(.field, .variant.as_deref(), .container_type_path) )] FieldNotClonable { field: FieldId, @@ -51,8 +51,8 @@ pub enum ReflectCloneError { fn full_path( field: &FieldId, - variant: &Option>, - container_type_path: &Cow<'static, str>, + variant: Option<&str>, + container_type_path: &str, ) -> alloc::string::String { match variant { Some(variant) => format!("{}::{}::{}", container_type_path, variant, field), From b3b514793cf4437eb98767e036f0099e2669115a Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sat, 1 Mar 2025 23:16:43 -0700 Subject: [PATCH 10/11] Add more reflect_clone impls --- crates/bevy_reflect/src/impls/smallvec.rs | 25 ++++-- crates/bevy_reflect/src/impls/std.rs | 103 +++++++++++++++++++--- 2 files changed, 113 insertions(+), 15 deletions(-) diff --git a/crates/bevy_reflect/src/impls/smallvec.rs b/crates/bevy_reflect/src/impls/smallvec.rs index afb75aff8d997..56c6da310c703 100644 --- a/crates/bevy_reflect/src/impls/smallvec.rs +++ b/crates/bevy_reflect/src/impls/smallvec.rs @@ -1,14 +1,14 @@ -use alloc::{boxed::Box, vec::Vec}; -use bevy_reflect_derive::impl_type_path; -use core::any::Any; -use smallvec::{Array as SmallArray, SmallVec}; - use crate::{ utility::GenericTypeInfoCell, ApplyError, FromReflect, FromType, Generics, GetTypeRegistration, List, ListInfo, ListIter, MaybeTyped, PartialReflect, Reflect, ReflectFromPtr, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypeParamInfo, TypePath, TypeRegistration, Typed, }; +use alloc::{borrow::Cow, boxed::Box, string::ToString, vec::Vec}; +use bevy_reflect::ReflectCloneError; +use bevy_reflect_derive::impl_type_path; +use core::any::Any; +use smallvec::{Array as SmallArray, SmallVec}; impl List for SmallVec where @@ -138,6 +138,21 @@ where Box::new(self.clone_dynamic()) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new( + self.iter() + .map(|value| { + value.reflect_clone()?.take().or_else(|_| { + Err(ReflectCloneError::FailedDowncast { + expected: Cow::Borrowed(::type_path()), + received: Cow::Owned(value.reflect_type_path().to_string()), + }) + }) + }) + .collect::>()?, + )) + } + fn reflect_partial_eq(&self, value: &dyn PartialReflect) -> Option { crate::list_partial_eq(self, value) } diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 73fc831bea9e6..f503390da1f28 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -21,6 +21,7 @@ use alloc::{ boxed::Box, collections::VecDeque, format, + string::ToString, vec::Vec, }; use bevy_reflect_derive::{impl_reflect, impl_reflect_opaque}; @@ -412,6 +413,12 @@ macro_rules! impl_reflect_for_atomic { fn clone_value(&self) -> Box { Box::new(<$ty>::new(self.load($ordering))) } + + #[inline] + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(<$ty>::new(self.load($ordering)))) + } + #[inline] fn try_apply(&mut self, value: &dyn PartialReflect) -> Result<(), ApplyError> { if let Some(value) = value.try_downcast_ref::() { @@ -623,6 +630,21 @@ macro_rules! impl_reflect_for_veclike { Box::new(self.clone_dynamic()) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new( + self.iter() + .map(|value| { + value.reflect_clone()?.take().or_else(|_| { + Err(ReflectCloneError::FailedDowncast { + expected: Cow::Borrowed(::type_path()), + received: Cow::Owned(value.reflect_type_path().to_string()), + }) + }) + }) + .collect::>()?, + )) + } + fn reflect_hash(&self) -> Option { crate::list_hash(self) } @@ -709,7 +731,7 @@ macro_rules! impl_reflect_for_hashmap { where K: FromReflect + MaybeTyped + TypePath + GetTypeRegistration + Eq + Hash, V: FromReflect + MaybeTyped + TypePath + GetTypeRegistration, - S: TypePath + BuildHasher + Send + Sync, + S: TypePath + BuildHasher + Default + Send + Sync, { fn get(&self, key: &dyn PartialReflect) -> Option<&dyn PartialReflect> { key.try_downcast_ref::() @@ -809,7 +831,7 @@ macro_rules! impl_reflect_for_hashmap { where K: FromReflect + MaybeTyped + TypePath + GetTypeRegistration + Eq + Hash, V: FromReflect + MaybeTyped + TypePath + GetTypeRegistration, - S: TypePath + BuildHasher + Send + Sync, + S: TypePath + BuildHasher + Default + Send + Sync, { fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { Some(::type_info()) @@ -862,6 +884,27 @@ macro_rules! impl_reflect_for_hashmap { Box::new(self.clone_dynamic()) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + let mut map = Self::with_capacity_and_hasher(self.len(), S::default()); + for (key, value) in self.iter() { + let key = key.reflect_clone()?.take().or_else(|_| { + Err(ReflectCloneError::FailedDowncast { + expected: Cow::Borrowed(::type_path()), + received: Cow::Owned(key.reflect_type_path().to_string()), + }) + })?; + let value = value.reflect_clone()?.take().or_else(|_| { + Err(ReflectCloneError::FailedDowncast { + expected: Cow::Borrowed(::type_path()), + received: Cow::Owned(value.reflect_type_path().to_string()), + }) + })?; + map.insert(key, value); + } + + Ok(Box::new(map)) + } + fn reflect_partial_eq(&self, value: &dyn PartialReflect) -> Option { map_partial_eq(self, value) } @@ -880,14 +923,14 @@ macro_rules! impl_reflect_for_hashmap { where K: FromReflect + MaybeTyped + TypePath + GetTypeRegistration + Eq + Hash, V: FromReflect + MaybeTyped + TypePath + GetTypeRegistration, - S: TypePath + BuildHasher + Send + Sync, + S: TypePath + BuildHasher + Default + Send + Sync, ); impl Typed for $ty where K: FromReflect + MaybeTyped + TypePath + GetTypeRegistration + Eq + Hash, V: FromReflect + MaybeTyped + TypePath + GetTypeRegistration, - S: TypePath + BuildHasher + Send + Sync, + S: TypePath + BuildHasher + Default + Send + Sync, { fn type_info() -> &'static TypeInfo { static CELL: GenericTypeInfoCell = GenericTypeInfoCell::new(); @@ -906,7 +949,7 @@ macro_rules! impl_reflect_for_hashmap { where K: FromReflect + MaybeTyped + TypePath + GetTypeRegistration + Eq + Hash, V: FromReflect + MaybeTyped + TypePath + GetTypeRegistration, - S: TypePath + BuildHasher + Send + Sync + Default, + S: TypePath + BuildHasher + Default + Send + Sync + Default, { fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::(); @@ -976,7 +1019,7 @@ macro_rules! impl_reflect_for_hashset { impl Set for $ty where V: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, - S: TypePath + BuildHasher + Send + Sync, + S: TypePath + BuildHasher + Default + Send + Sync, { fn get(&self, value: &dyn PartialReflect) -> Option<&dyn PartialReflect> { value @@ -1045,7 +1088,7 @@ macro_rules! impl_reflect_for_hashset { impl PartialReflect for $ty where V: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, - S: TypePath + BuildHasher + Send + Sync, + S: TypePath + BuildHasher + Default + Send + Sync, { fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { Some(::type_info()) @@ -1107,6 +1150,21 @@ macro_rules! impl_reflect_for_hashset { Box::new(self.clone_dynamic()) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + let mut set = Self::with_capacity_and_hasher(self.len(), S::default()); + for value in self.iter() { + let value = value.reflect_clone()?.take().or_else(|_| { + Err(ReflectCloneError::FailedDowncast { + expected: Cow::Borrowed(::type_path()), + received: Cow::Owned(value.reflect_type_path().to_string()), + }) + })?; + set.insert(value); + } + + Ok(Box::new(set)) + } + fn reflect_partial_eq(&self, value: &dyn PartialReflect) -> Option { set_partial_eq(self, value) } @@ -1115,7 +1173,7 @@ macro_rules! impl_reflect_for_hashset { impl Typed for $ty where V: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, - S: TypePath + BuildHasher + Send + Sync, + S: TypePath + BuildHasher + Default + Send + Sync, { fn type_info() -> &'static TypeInfo { static CELL: GenericTypeInfoCell = GenericTypeInfoCell::new(); @@ -1132,7 +1190,7 @@ macro_rules! impl_reflect_for_hashset { impl GetTypeRegistration for $ty where V: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, - S: TypePath + BuildHasher + Send + Sync + Default, + S: TypePath + BuildHasher + Default + Send + Sync + Default, { fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::(); @@ -1150,7 +1208,7 @@ macro_rules! impl_reflect_for_hashset { for $ty where V: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, - S: TypePath + BuildHasher + Send + Sync, + S: TypePath + BuildHasher + Default + Send + Sync, ); impl FromReflect for $ty @@ -1353,6 +1411,27 @@ where Box::new(self.clone_dynamic()) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + let mut map = Self::new(); + for (key, value) in self.iter() { + let key = key.reflect_clone()?.take().or_else(|_| { + Err(ReflectCloneError::FailedDowncast { + expected: Cow::Borrowed(::type_path()), + received: Cow::Owned(key.reflect_type_path().to_string()), + }) + })?; + let value = value.reflect_clone()?.take().or_else(|_| { + Err(ReflectCloneError::FailedDowncast { + expected: Cow::Borrowed(::type_path()), + received: Cow::Owned(value.reflect_type_path().to_string()), + }) + })?; + map.insert(key, value); + } + + Ok(Box::new(map)) + } + fn reflect_partial_eq(&self, value: &dyn PartialReflect) -> Option { map_partial_eq(self, value) } @@ -2470,6 +2549,10 @@ impl PartialReflect for &'static Location<'static> { Box::new(*self) } + fn reflect_clone(&self) -> Result, ReflectCloneError> { + Ok(Box::new(*self)) + } + fn reflect_hash(&self) -> Option { let mut hasher = reflect_hasher(); Hash::hash(&Any::type_id(self), &mut hasher); From 1a2f5b3ee2c950f90c441d895690438835771684 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sat, 1 Mar 2025 23:24:55 -0700 Subject: [PATCH 11/11] Fix clippy and formatting and docs and typos and compile tests --- .../tests/reflect_remote/nested_fail.rs | 1 + crates/bevy_reflect/derive/src/derive_data.rs | 2 +- .../bevy_reflect/derive/src/enum_utility.rs | 2 +- crates/bevy_reflect/derive/src/impls/enums.rs | 2 +- crates/bevy_reflect/src/error.rs | 26 +++++----- crates/bevy_reflect/src/impls/glam.rs | 16 +++++- crates/bevy_reflect/src/impls/smallvec.rs | 7 +-- crates/bevy_reflect/src/impls/std.rs | 51 ++++++++++--------- crates/bevy_reflect/src/lib.rs | 15 +++--- crates/bevy_reflect/src/reflect.rs | 6 +-- 10 files changed, 74 insertions(+), 54 deletions(-) diff --git a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs index 0f8ade8e234ca..457f1f75e5f52 100644 --- a/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs +++ b/crates/bevy_reflect/compile_fail/tests/reflect_remote/nested_fail.rs @@ -26,6 +26,7 @@ mod incorrect_inner_type { //~| ERROR: `TheirInner` does not implement `PartialReflect` so cannot be introspected //~| ERROR: `TheirInner` does not implement `PartialReflect` so cannot be introspected //~| ERROR: `TheirInner` does not implement `TypePath` so cannot provide dynamic type path information + //~| ERROR: `TheirInner` does not implement `TypePath` so cannot provide dynamic type path information //~| ERROR: `?` operator has incompatible types struct MyOuter { // Reason: Should not use `MyInner` directly diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index f9e1f7305808d..bcff582a4e367 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -705,7 +705,7 @@ impl<'a> ReflectStruct<'a> { let field_id = field.field_id(bevy_reflect_path); quote! { - return #FQResult::Err(#bevy_reflect_path::ReflectCloneError::FieldNotClonable { + return #FQResult::Err(#bevy_reflect_path::ReflectCloneError::FieldNotCloneable { field: #field_id, variant: #FQOption::None, container_type_path: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed( diff --git a/crates/bevy_reflect/derive/src/enum_utility.rs b/crates/bevy_reflect/derive/src/enum_utility.rs index bd21983304f68..5571b861a6a81 100644 --- a/crates/bevy_reflect/derive/src/enum_utility.rs +++ b/crates/bevy_reflect/derive/src/enum_utility.rs @@ -379,7 +379,7 @@ impl<'a> VariantBuilder for ReflectCloneVariantBuilder<'a> { quote! { return #FQResult::Err( - #bevy_reflect_path::ReflectCloneError::FieldNotClonable { + #bevy_reflect_path::ReflectCloneError::FieldNotCloneable { field: #field_id, variant: #FQOption::Some(#bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed(#variant_name)), container_type_path: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed(::type_path()) diff --git a/crates/bevy_reflect/derive/src/impls/enums.rs b/crates/bevy_reflect/derive/src/impls/enums.rs index 338d60795be89..cec99a23ce8a9 100644 --- a/crates/bevy_reflect/derive/src/impls/enums.rs +++ b/crates/bevy_reflect/derive/src/impls/enums.rs @@ -262,7 +262,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream } #common_methods - + #clone_fn } } diff --git a/crates/bevy_reflect/src/error.rs b/crates/bevy_reflect/src/error.rs index 0d78e363bd346..e783a33775a06 100644 --- a/crates/bevy_reflect/src/error.rs +++ b/crates/bevy_reflect/src/error.rs @@ -2,37 +2,37 @@ use crate::FieldId; use alloc::{borrow::Cow, format}; use thiserror::Error; -/// An error that occurs when cloning a type via [`Reflect::reflect_clone`]. +/// An error that occurs when cloning a type via [`PartialReflect::reflect_clone`]. /// -/// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone +/// [`PartialReflect::reflect_clone`]: crate::PartialReflect::reflect_clone #[derive(Clone, Debug, Error, PartialEq, Eq)] pub enum ReflectCloneError { - /// The type does not have a custom implementation for [`Reflect::reflect_clone`]. + /// The type does not have a custom implementation for [`PartialReflect::reflect_clone`]. /// - /// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone - #[error("`Reflect::reflect_clone` not implemented for `{type_path}`")] + /// [`PartialReflect::reflect_clone`]: crate::PartialReflect::reflect_clone + #[error("`PartialReflect::reflect_clone` not implemented for `{type_path}`")] NotImplemented { type_path: Cow<'static, str> }, - /// The type cannot be cloned via [`Reflect::reflect_clone`]. + /// The type cannot be cloned via [`PartialReflect::reflect_clone`]. /// /// This type should be returned when a type is intentionally opting out of reflection cloning. /// - /// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone - #[error("`{type_path}` cannot be made clonable for `Reflect::reflect_clone`")] - NotClonable { type_path: Cow<'static, str> }, - /// The field cannot be cloned via [`Reflect::reflect_clone`]. + /// [`PartialReflect::reflect_clone`]: crate::PartialReflect::reflect_clone + #[error("`{type_path}` cannot be made cloneable for `PartialReflect::reflect_clone`")] + NotCloneable { type_path: Cow<'static, str> }, + /// The field cannot be cloned via [`PartialReflect::reflect_clone`]. /// /// When [deriving `Reflect`], this usually means that a field marked with `#[reflect(ignore)]` /// is missing a `#[reflect(clone)]` attribute. /// /// This may be intentional if the field is not meant/able to be cloned. /// - /// [`Reflect::reflect_clone`]: crate::Reflect::reflect_clone + /// [`PartialReflect::reflect_clone`]: crate::PartialReflect::reflect_clone /// [deriving `Reflect`]: derive@crate::Reflect #[error( - "field `{}` cannot be made clonable for `Reflect::reflect_clone` (are you missing a `#[reflect(clone)]` attribute?)", + "field `{}` cannot be made cloneable for `PartialReflect::reflect_clone` (are you missing a `#[reflect(clone)]` attribute?)", full_path(.field, .variant.as_deref(), .container_type_path) )] - FieldNotClonable { + FieldNotCloneable { field: FieldId, variant: Option>, container_type_path: Cow<'static, str>, diff --git a/crates/bevy_reflect/src/impls/glam.rs b/crates/bevy_reflect/src/impls/glam.rs index 1e0a6a11d0675..139557ddb61a2 100644 --- a/crates/bevy_reflect/src/impls/glam.rs +++ b/crates/bevy_reflect/src/impls/glam.rs @@ -491,8 +491,20 @@ reflect_enum!( } ); -impl_reflect_opaque!(::glam::BVec3A(Clone, Debug, Default, Deserialize, Serialize)); -impl_reflect_opaque!(::glam::BVec4A(Clone, Debug, Default, Deserialize, Serialize)); +impl_reflect_opaque!(::glam::BVec3A( + Clone, + Debug, + Default, + Deserialize, + Serialize +)); +impl_reflect_opaque!(::glam::BVec4A( + Clone, + Debug, + Default, + Deserialize, + Serialize +)); #[cfg(test)] mod tests { diff --git a/crates/bevy_reflect/src/impls/smallvec.rs b/crates/bevy_reflect/src/impls/smallvec.rs index 56c6da310c703..495abcbd800f6 100644 --- a/crates/bevy_reflect/src/impls/smallvec.rs +++ b/crates/bevy_reflect/src/impls/smallvec.rs @@ -142,12 +142,13 @@ where Ok(Box::new( self.iter() .map(|value| { - value.reflect_clone()?.take().or_else(|_| { - Err(ReflectCloneError::FailedDowncast { + value + .reflect_clone()? + .take() + .map_err(|_| ReflectCloneError::FailedDowncast { expected: Cow::Borrowed(::type_path()), received: Cow::Owned(value.reflect_type_path().to_string()), }) - }) }) .collect::>()?, )) diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index f503390da1f28..f364389449622 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -634,11 +634,11 @@ macro_rules! impl_reflect_for_veclike { Ok(Box::new( self.iter() .map(|value| { - value.reflect_clone()?.take().or_else(|_| { - Err(ReflectCloneError::FailedDowncast { + value.reflect_clone()?.take().map_err(|_| { + ReflectCloneError::FailedDowncast { expected: Cow::Borrowed(::type_path()), received: Cow::Owned(value.reflect_type_path().to_string()), - }) + } }) }) .collect::>()?, @@ -887,17 +887,17 @@ macro_rules! impl_reflect_for_hashmap { fn reflect_clone(&self) -> Result, ReflectCloneError> { let mut map = Self::with_capacity_and_hasher(self.len(), S::default()); for (key, value) in self.iter() { - let key = key.reflect_clone()?.take().or_else(|_| { - Err(ReflectCloneError::FailedDowncast { + let key = key.reflect_clone()?.take().map_err(|_| { + ReflectCloneError::FailedDowncast { expected: Cow::Borrowed(::type_path()), received: Cow::Owned(key.reflect_type_path().to_string()), - }) + } })?; - let value = value.reflect_clone()?.take().or_else(|_| { - Err(ReflectCloneError::FailedDowncast { + let value = value.reflect_clone()?.take().map_err(|_| { + ReflectCloneError::FailedDowncast { expected: Cow::Borrowed(::type_path()), received: Cow::Owned(value.reflect_type_path().to_string()), - }) + } })?; map.insert(key, value); } @@ -1153,11 +1153,11 @@ macro_rules! impl_reflect_for_hashset { fn reflect_clone(&self) -> Result, ReflectCloneError> { let mut set = Self::with_capacity_and_hasher(self.len(), S::default()); for value in self.iter() { - let value = value.reflect_clone()?.take().or_else(|_| { - Err(ReflectCloneError::FailedDowncast { + let value = value.reflect_clone()?.take().map_err(|_| { + ReflectCloneError::FailedDowncast { expected: Cow::Borrowed(::type_path()), received: Cow::Owned(value.reflect_type_path().to_string()), - }) + } })?; set.insert(value); } @@ -1414,18 +1414,21 @@ where fn reflect_clone(&self) -> Result, ReflectCloneError> { let mut map = Self::new(); for (key, value) in self.iter() { - let key = key.reflect_clone()?.take().or_else(|_| { - Err(ReflectCloneError::FailedDowncast { - expected: Cow::Borrowed(::type_path()), - received: Cow::Owned(key.reflect_type_path().to_string()), - }) - })?; - let value = value.reflect_clone()?.take().or_else(|_| { - Err(ReflectCloneError::FailedDowncast { - expected: Cow::Borrowed(::type_path()), - received: Cow::Owned(value.reflect_type_path().to_string()), - }) - })?; + let key = + key.reflect_clone()? + .take() + .map_err(|_| ReflectCloneError::FailedDowncast { + expected: Cow::Borrowed(::type_path()), + received: Cow::Owned(key.reflect_type_path().to_string()), + })?; + let value = + value + .reflect_clone()? + .take() + .map_err(|_| ReflectCloneError::FailedDowncast { + expected: Cow::Borrowed(::type_path()), + received: Cow::Owned(value.reflect_type_path().to_string()), + })?; map.insert(key, value); } diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 8448684bbbdd6..25a7e3a0c57cb 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -1024,8 +1024,11 @@ mod tests { fn should_reflect_clone_with_clone() { // A custom clone function to verify that the `#[reflect(Clone)]` container attribute // takes precedence over the `#[reflect(clone)]` field attribute. - #[allow(dead_code, unused_variables)] - fn custom_clone(value: &usize) -> usize { + #[expect( + dead_code, + reason = "if things are working correctly, this function should never be called" + )] + fn custom_clone(_value: &usize) -> usize { panic!("should not be called"); } @@ -1108,7 +1111,7 @@ mod tests { let clone = foo.reflect_clone(); assert_eq!( clone.unwrap_err(), - ReflectCloneError::FieldNotClonable { + ReflectCloneError::FieldNotCloneable { field: FieldId::Unnamed(0), variant: None, container_type_path: Cow::Borrowed(Foo::type_path()), @@ -1126,7 +1129,7 @@ mod tests { let clone = bar.reflect_clone(); assert_eq!( clone.unwrap_err(), - ReflectCloneError::FieldNotClonable { + ReflectCloneError::FieldNotCloneable { field: FieldId::Named(Cow::Borrowed("value")), variant: None, container_type_path: Cow::Borrowed(Bar::type_path()), @@ -1147,7 +1150,7 @@ mod tests { let clone = baz.reflect_clone(); assert_eq!( clone.unwrap_err(), - ReflectCloneError::FieldNotClonable { + ReflectCloneError::FieldNotCloneable { field: FieldId::Unnamed(0), variant: Some(Cow::Borrowed("Tuple")), container_type_path: Cow::Borrowed(Baz::type_path()), @@ -1158,7 +1161,7 @@ mod tests { let clone = baz.reflect_clone(); assert_eq!( clone.unwrap_err(), - ReflectCloneError::FieldNotClonable { + ReflectCloneError::FieldNotCloneable { field: FieldId::Named(Cow::Borrowed("value")), variant: Some(Cow::Borrowed("Struct")), container_type_path: Cow::Borrowed(Baz::type_path()), diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index b9e9288b525f0..e1130572cbe88 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -234,10 +234,10 @@ where /// # Example /// /// ``` - /// # use bevy_reflect::{Reflect, DynamicTuple}; + /// # use bevy_reflect::{PartialReflect}; /// let value = (1, true, 3.14); /// let cloned = value.clone_value(); - /// assert!(cloned.is::()) + /// assert!(cloned.is_dynamic()) /// ``` /// /// [`List`]: crate::List @@ -258,7 +258,7 @@ where /// # Example /// /// ``` - /// # use bevy_reflect::Reflect; + /// # use bevy_reflect::PartialReflect; /// let value = (1, true, 3.14); /// let cloned = value.reflect_clone().unwrap(); /// assert!(cloned.is::<(i32, bool, f64)>())