Skip to content

Commit

Permalink
bevy_reflect: Reflection-based cloning (#13432)
Browse files Browse the repository at this point in the history
# Objective

Using `Reflect::clone_value` can be somewhat confusing to those
unfamiliar with how Bevy's reflection crate works. For example take the
following code:

```rust
let value: usize = 123;
let clone: Box<dyn Reflect> = value.clone_value();
```

What can we expect to be the underlying type of `clone`? If you guessed
`usize`, then you're correct! Let's try another:

```rust
#[derive(Reflect, Clone)]
struct Foo(usize);

let value: Foo = Foo(123);
let clone: Box<dyn Reflect> = value.clone_value();
```

What about this code? What is the underlying type of `clone`? If you
guessed `Foo`, unfortunately you'd be wrong. It's actually
`DynamicStruct`.

It's not obvious that the generated `Reflect` impl actually calls
`Struct::clone_dynamic` under the hood, which always returns
`DynamicStruct`.

There are already some efforts to make this a bit more apparent to the
end-user: #7207 changes the signature of `Reflect::clone_value` to
instead return `Box<dyn PartialReflect>`, signaling that we're
potentially returning a dynamic type.

But why _can't_ we return `Foo`?

`Foo` can obviously be cloned— in fact, we already derived `Clone` on
it. But even without the derive, this seems like something `Reflect`
should be able to handle. Almost all types that implement `Reflect`
either contain no data (trivially clonable), they contain a
`#[reflect_value]` type (which, by definition, must implement `Clone`),
or they contain another `Reflect` type (which recursively fall into one
of these three categories).

This PR aims to enable true reflection-based cloning where you get back
exactly the type that you think you do.

## Solution

Add a `Reflect::reflect_clone` method which returns `Result<Box<dyn
Reflect>, ReflectCloneError>`, where the `Box<dyn Reflect>` is
guaranteed to be the same type as `Self`.

```rust
#[derive(Reflect)]
struct Foo(usize);

let value: Foo = Foo(123);
let clone: Box<dyn Reflect> = value.reflect_clone().unwrap();
assert!(clone.is::<Foo>());
```

Notice that we didn't even need to derive `Clone` for this to work: it's
entirely powered via reflection!

Under the hood, the macro generates something like this:

```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
    Ok(Box::new(Self {
        // The `reflect_clone` impl for `usize` just makes use of its `Clone` impl
        0: Reflect::reflect_clone(&self.0)?.take().map_err(/* ... */)?,
    }))
}
```

If we did derive `Clone`, we can tell `Reflect` to rely on that instead:

```rust
#[derive(Reflect, Clone)]
#[reflect(Clone)]
struct Foo(usize);
```

<details>
<summary>Generated Code</summary>

```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
    Ok(Box::new(Clone::clone(self)))
}
```

</details>

Or, we can specify our own cloning function:

```rust
#[derive(Reflect)]
#[reflect(Clone(incremental_clone))]
struct Foo(usize);

fn incremental_clone(value: &usize) -> usize {
  *value + 1
}
```

<details>
<summary>Generated Code</summary>

```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
    Ok(Box::new(incremental_clone(self)))
}
```

</details>

Similarly, we can specify how fields should be cloned. This is important
for fields that are `#[reflect(ignore)]`'d as we otherwise have no way
to know how they should be cloned.

```rust
#[derive(Reflect)]
struct Foo {
 #[reflect(ignore, clone)]
  bar: usize,
  #[reflect(ignore, clone = "incremental_clone")]
  baz: usize,
}

fn incremental_clone(value: &usize) -> usize {
  *value + 1
}
```

<details>
<summary>Generated Code</summary>

```rust
fn reflect_clone(&self) -> Result<Box<dyn Reflect>, ReflectCloneError> {
    Ok(Box::new(Self {
        bar: Clone::clone(&self.bar),
        baz: incremental_clone(&self.baz),
    }))
}
```

</details>

If we don't supply a `clone` attribute for an ignored field, then the
method will automatically return
`Err(ReflectCloneError::FieldNotClonable {/* ... */})`.

`Err` values "bubble up" to the caller. So if `Foo` contains `Bar` and
the `reflect_clone` method for `Bar` returns `Err`, then the
`reflect_clone` method for `Foo` also returns `Err`.

### Attribute Syntax

You might have noticed the differing syntax between the container
attribute and the field attribute.

This was purely done for consistency with the current attributes. There
are PRs aimed at improving this. #7317 aims at making the
"special-cased" attributes more in line with the field attributes
syntactically. And #9323 aims at moving away from the stringified paths
in favor of just raw function paths.

### Compatibility with Unique Reflect

This PR was designed with Unique Reflect (#7207) in mind. This method
actually wouldn't change that much (if at all) under Unique Reflect. It
would still exist on `Reflect` and it would still `Option<Box<dyn
Reflect>>`. In fact, Unique Reflect would only _improve_ the user's
understanding of what this method returns.

We may consider moving what's currently `Reflect::clone_value` to
`PartialReflect` and possibly renaming it to `partial_reflect_clone` or
`clone_dynamic` to better indicate how it differs from `reflect_clone`.

## Testing

You can test locally by running the following command:

```
cargo test --package bevy_reflect
```

---

## Changelog

- Added `Reflect::reflect_clone` method
- Added `ReflectCloneError` error enum
- Added `#[reflect(Clone)]` container attribute
- Added `#[reflect(clone)]` field attribute
  • Loading branch information
MrGVSV authored Mar 11, 2025
1 parent 32d53e7 commit f5210c5
Show file tree
Hide file tree
Showing 23 changed files with 1,219 additions and 173 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ mod incorrect_inner_type {
//~| ERROR: `TheirInner<T>` does not implement `PartialReflect` so cannot be introspected
//~| ERROR: `TheirInner<T>` does not implement `PartialReflect` so cannot be introspected
//~| ERROR: `TheirInner<T>` does not implement `TypePath` so cannot provide dynamic type path information
//~| ERROR: `TheirInner<T>` does not implement `TypePath` so cannot provide dynamic type path information
//~| ERROR: `?` operator has incompatible types
struct MyOuter<T: FromReflect + GetTypeRegistration> {
// Reason: Should not use `MyInner<T>` directly
Expand Down
48 changes: 45 additions & 3 deletions crates/bevy_reflect/derive/src/container_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, FQClone, FQOption, FQResult};
use proc_macro2::{Ident, Span};
use quote::quote_spanned;
use syn::{
Expand All @@ -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);
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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::<kw::Clone>()?;

if input.peek(token::Paren) {
let content;
parenthesized!(content in input);
let path = content.parse::<Path>()?;
self.clone.merge(TraitImpl::Custom(path, ident.span))?;
} else {
self.clone = TraitImpl::Implemented(ident.span);
}

Ok(())
}

/// Parse special `Debug` registration.
///
/// Examples:
Expand Down Expand Up @@ -523,6 +547,24 @@ impl ContainerAttributes {
}
}

pub fn get_clone_impl(&self, bevy_reflect_path: &Path) -> Option<proc_macro2::TokenStream> {
match &self.clone {
&TraitImpl::Implemented(span) => Some(quote_spanned! {span=>
#[inline]
fn reflect_clone(&self) -> #FQResult<#bevy_reflect_path::__macro_exports::alloc_utils::Box<dyn #bevy_reflect_path::Reflect>, #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) -> #FQResult<#bevy_reflect_path::__macro_exports::alloc_utils::Box<dyn #bevy_reflect_path::Reflect>, #bevy_reflect_path::ReflectCloneError> {
#FQResult::Ok(#bevy_reflect_path::__macro_exports::alloc_utils::Box::new(#impl_fn(self)))
}
}),
TraitImpl::NotImplemented => None,
}
}

pub fn custom_attributes(&self) -> &CustomAttributes {
&self.custom_attributes
}
Expand Down
206 changes: 203 additions & 3 deletions crates/bevy_reflect/derive/src/derive_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{FQClone, FQOption, FQResult};
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> {
Expand Down Expand Up @@ -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}"),
));
}

Expand Down Expand Up @@ -546,6 +550,31 @@ 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()),
}
}

/// 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> {
Expand Down Expand Up @@ -655,6 +684,135 @@ 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<proc_macro2::TokenStream> {
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 field_ty = field.reflected_type();
let member = field.to_member();
let accessor = self.access_for_field(field, false);

match &field.attrs.clone {
CloneBehavior::Default => {
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::FieldNotCloneable {
field: #field_id,
variant: #FQOption::None,
container_type_path: #bevy_reflect_path::__macro_exports::alloc_utils::Cow::Borrowed(
<Self as #bevy_reflect_path::TypePath>::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: #value,
});
}
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]
#[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<dyn #bevy_reflect_path::Reflect>, #bevy_reflect_path::ReflectCloneError> {
#FQResult::Ok(#bevy_reflect_path::__macro_exports::alloc_utils::Box::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> {
Expand Down Expand Up @@ -757,6 +915,48 @@ 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<proc_macro2::TokenStream> {
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 = <Self as #bevy_reflect_path::ReflectRemote>::as_remote(self);
#FQResult::Ok(#bevy_reflect_path::__macro_exports::alloc_utils::Box::new(<Self as #bevy_reflect_path::ReflectRemote>::into_wrapper(#inner)))
}
} else {
quote! {
let #this = self;
#FQResult::Ok(#bevy_reflect_path::__macro_exports::alloc_utils::Box::new(#inner))
}
};

Some(quote! {
#[inline]
#[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<dyn #bevy_reflect_path::Reflect>, #bevy_reflect_path::ReflectCloneError> {
#body
}
})
}
}

impl<'a> EnumVariant<'a> {
Expand Down
Loading

0 comments on commit f5210c5

Please sign in to comment.