Skip to content

Commit 4f340c6

Browse files
MrGVSVexjam
authored andcommitted
bevy_reflect_derive: Tidying up the code (bevyengine#4712)
# Objective The `bevy_reflect_derive` crate is not the cleanest or easiest to follow/maintain. The `lib.rs` file is especially difficult with over 1000 lines of code written in a confusing order. This is just a result of growth within the crate and it would be nice to clean it up for future work. ## Solution Split `bevy_reflect_derive` into many more submodules. The submodules include: * `container_attributes` - Code relating to container attributes * `derive_data` - Code relating to reflection-based derive metadata * `field_attributes` - Code relating to field attributes * `impls` - Code containing actual reflection implementations * `reflect_value` - Code relating to reflection-based value metadata * `registration` - Code relating to type registration * `utility` - General-purpose utility functions This leaves the `lib.rs` file to contain only the public macros, making it much easier to digest (and fewer than 200 lines). By breaking up the code into smaller modules, we make it easier for future contributors to find the code they're looking for or identify which module best fits their own additions. ### Metadata Structs This cleanup also adds two big metadata structs: `ReflectFieldAttr` and `ReflectDeriveData`. The former is used to store all attributes for a struct field (if any). The latter is used to store all metadata for struct-based derive inputs. Both significantly reduce code duplication and make editing these macros much simpler. The tradeoff is that we may collect more metadata than needed. However, this is usually a small thing (such as checking for attributes when they're not really needed or creating a `ReflectFieldAttr` for every field regardless of whether they actually have an attribute). We could try to remove these tradeoffs and squeeze some more performance out, but doing so might come at the cost of developer experience. Personally, I think it's much nicer to create a `ReflectFieldAttr` for every field since it means I don't have to do two `Option` checks. Others may disagree, though, and so we can discuss changing this either in this PR or in a future one. ### Out of Scope _Some_ documentation has been added or improved, but ultimately good docs are probably best saved for a dedicated PR. ## 🔍 Focus Points (for reviewers) I know it's a lot to sift through, so here is a list of **key points for reviewers**: - The following files contain code that was mostly just relocated: - `reflect_value.rs` - `registration.rs` - `container_attributes.rs` was also mostly moved but features some general cleanup (reducing nesting, removing hardcoded strings, etc.) and lots of doc comments - Most impl logic was moved from `lib.rs` to `impls.rs`, but they have been significantly modified to use the new `ReflectDeriveData` metadata struct in order to reduce duplication. - `derive_data.rs` and `field_attributes.rs` contain almost entirely new code and should probably be given the most attention. - Likewise, `from_reflect.rs` saw major changes using `ReflectDeriveData` so it should also be given focus. - There was no change to the `lib.rs` exports so the end-user API should be the same. ## Prior Work This task was initially tackled by @NathanSWard in bevyengine#2377 (which was closed in favor of this PR), so hats off to them for beating me to the punch by nearly a year! --- ## Changelog * **[INTERNAL]** Split `bevy_reflect_derive` into smaller submodules * **[INTERNAL]** Add `ReflectFieldAttr` * **[INTERNAL]** Add `ReflectDeriveData` * Add `BevyManifest::get_path_direct()` method (`bevy_macro_utils`) Co-authored-by: MrGVSV <49806985+MrGVSV@users.noreply.github.com>
1 parent 7127338 commit 4f340c6

File tree

13 files changed

+1257
-1118
lines changed

13 files changed

+1257
-1118
lines changed

crates/bevy_macro_utils/src/lib.rs

+16
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,22 @@ impl BevyManifest {
7878
deps.and_then(find_in_deps)
7979
.or_else(|| deps_dev.and_then(find_in_deps))
8080
}
81+
82+
/// Returns the path for the crate with the given name.
83+
///
84+
/// This is a convenience method for constructing a [manifest] and
85+
/// calling the [`get_path`] method.
86+
///
87+
/// This method should only be used where you just need the path and can't
88+
/// cache the [manifest]. If caching is possible, it's recommended to create
89+
/// the [manifest] yourself and use the [`get_path`] method.
90+
///
91+
/// [`get_path`]: Self::get_path
92+
/// [manifest]: Self
93+
pub fn get_path_direct(name: &str) -> syn::Path {
94+
Self::default().get_path(name)
95+
}
96+
8197
pub fn get_path(&self, name: &str) -> syn::Path {
8298
self.maybe_get_path(name)
8399
.unwrap_or_else(|| Self::parse_str(name))
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
//! Contains code related to container attributes for reflected types.
2+
//!
3+
//! A container attribute is an attribute which applies to an entire struct or enum
4+
//! as opposed to a particular field or variant. An example of such an attribute is
5+
//! the derive helper attribute for `Reflect`, which looks like:
6+
//! `#[reflect(PartialEq, Default, ...)]` and `#[reflect_value(PartialEq, Default, ...)]`.
7+
8+
use crate::utility;
9+
use proc_macro2::Ident;
10+
use quote::quote;
11+
use syn::parse::{Parse, ParseStream};
12+
use syn::punctuated::Punctuated;
13+
use syn::token::Comma;
14+
use syn::{Meta, NestedMeta, Path};
15+
16+
// The "special" trait idents that are used internally for reflection.
17+
// Received via attributes like `#[reflect(PartialEq, Hash, ...)]`
18+
const PARTIAL_EQ_ATTR: &str = "PartialEq";
19+
const HASH_ATTR: &str = "Hash";
20+
const SERIALIZE_ATTR: &str = "Serialize";
21+
22+
/// A marker for trait implementations registered via the `Reflect` derive macro.
23+
#[derive(Clone)]
24+
pub(crate) enum TraitImpl {
25+
/// The trait is not registered as implemented.
26+
NotImplemented,
27+
/// The trait is registered as implemented.
28+
Implemented,
29+
30+
// TODO: This can be made to use `ExprPath` instead of `Ident`, allowing for fully qualified paths to be used
31+
/// The trait is registered with a custom function rather than an actual implementation.
32+
Custom(Ident),
33+
}
34+
35+
impl Default for TraitImpl {
36+
fn default() -> Self {
37+
Self::NotImplemented
38+
}
39+
}
40+
41+
/// A collection of traits that have been registered for a reflected type.
42+
///
43+
/// This keeps track of a few traits that are utilized internally for reflection
44+
/// (we'll call these traits _special traits_ within this context), but it
45+
/// will also keep track of all registered traits. Traits are registered as part of the
46+
/// `Reflect` derive macro using the helper attribute: `#[reflect(...)]`.
47+
///
48+
/// The list of special traits are as follows:
49+
/// * `Hash`
50+
/// * `PartialEq`
51+
/// * `Serialize`
52+
///
53+
/// When registering a trait, there are a few things to keep in mind:
54+
/// * Traits must have a valid `Reflect{}` struct in scope. For example, `Default`
55+
/// needs `bevy_reflect::prelude::ReflectDefault` in scope.
56+
/// * Traits must be single path identifiers. This means you _must_ use `Default`
57+
/// instead of `std::default::Default` (otherwise it will try to register `Reflectstd`!)
58+
/// * A custom function may be supplied in place of an actual implementation
59+
/// for the special traits (but still follows the same single-path identifier
60+
/// rules as normal).
61+
///
62+
/// # Example
63+
///
64+
/// Registering the `Default` implementation:
65+
///
66+
/// ```ignore
67+
/// // Import ReflectDefault so it's accessible by the derive macro
68+
/// use bevy_reflect::prelude::ReflectDefault.
69+
///
70+
/// #[derive(Reflect, Default)]
71+
/// #[reflect(Default)]
72+
/// struct Foo;
73+
/// ```
74+
///
75+
/// Registering the `Hash` implementation:
76+
///
77+
/// ```ignore
78+
/// // `Hash` is a "special trait" and does not need (nor have) a ReflectHash struct
79+
///
80+
/// #[derive(Reflect, Hash)]
81+
/// #[reflect(Hash)]
82+
/// struct Foo;
83+
/// ```
84+
///
85+
/// Registering the `Hash` implementation using a custom function:
86+
///
87+
/// ```ignore
88+
/// // This function acts as our `Hash` implementation and
89+
/// // corresponds to the `Reflect::reflect_hash` method.
90+
/// fn get_hash(foo: &Foo) -> Option<u64> {
91+
/// Some(123)
92+
/// }
93+
///
94+
/// #[derive(Reflect)]
95+
/// // Register the custom `Hash` function
96+
/// #[reflect(Hash(get_hash))]
97+
/// struct Foo;
98+
/// ```
99+
///
100+
/// > __Note:__ Registering a custom function only works for special traits.
101+
///
102+
#[derive(Default)]
103+
pub(crate) struct ReflectTraits {
104+
hash: TraitImpl,
105+
partial_eq: TraitImpl,
106+
serialize: TraitImpl,
107+
idents: Vec<Ident>,
108+
}
109+
110+
impl ReflectTraits {
111+
/// Create a new [`ReflectTraits`] instance from a set of nested metas.
112+
pub fn from_nested_metas(nested_metas: &Punctuated<NestedMeta, Comma>) -> Self {
113+
let mut traits = ReflectTraits::default();
114+
for nested_meta in nested_metas.iter() {
115+
match nested_meta {
116+
// Handles `#[reflect( Hash, Default, ... )]`
117+
NestedMeta::Meta(Meta::Path(path)) => {
118+
// Get the first ident in the path (hopefully the path only contains one and not `std::hash::Hash`)
119+
let ident = if let Some(segment) = path.segments.iter().next() {
120+
segment.ident.to_string()
121+
} else {
122+
continue;
123+
};
124+
125+
match ident.as_str() {
126+
PARTIAL_EQ_ATTR => traits.partial_eq = TraitImpl::Implemented,
127+
HASH_ATTR => traits.hash = TraitImpl::Implemented,
128+
SERIALIZE_ATTR => traits.serialize = TraitImpl::Implemented,
129+
// We only track reflected idents for traits not considered special
130+
_ => traits.idents.push(utility::get_reflect_ident(&ident)),
131+
}
132+
}
133+
// Handles `#[reflect( Hash(custom_hash_fn) )]`
134+
NestedMeta::Meta(Meta::List(list)) => {
135+
// Get the first ident in the path (hopefully the path only contains one and not `std::hash::Hash`)
136+
let ident = if let Some(segment) = list.path.segments.iter().next() {
137+
segment.ident.to_string()
138+
} else {
139+
continue;
140+
};
141+
142+
let list_meta = list.nested.iter().next();
143+
if let Some(NestedMeta::Meta(Meta::Path(path))) = list_meta {
144+
if let Some(segment) = path.segments.iter().next() {
145+
// This should be the ident of the custom function
146+
let trait_func_ident = TraitImpl::Custom(segment.ident.clone());
147+
match ident.as_str() {
148+
PARTIAL_EQ_ATTR => traits.partial_eq = trait_func_ident,
149+
HASH_ATTR => traits.hash = trait_func_ident,
150+
SERIALIZE_ATTR => traits.serialize = trait_func_ident,
151+
_ => {}
152+
}
153+
}
154+
}
155+
}
156+
_ => {}
157+
}
158+
}
159+
160+
traits
161+
}
162+
163+
/// Returns true if the given reflected trait name (i.e. `ReflectDefault` for `Default`)
164+
/// is registered for this type.
165+
pub fn contains(&self, name: &str) -> bool {
166+
self.idents.iter().any(|ident| ident == name)
167+
}
168+
169+
/// The list of reflected traits by their reflected ident (i.e. `ReflectDefault` for `Default`).
170+
pub fn idents(&self) -> &[Ident] {
171+
&self.idents
172+
}
173+
174+
/// Returns the logic for `Reflect::reflect_hash` as a `TokenStream`.
175+
///
176+
/// If `Hash` was not registered, returns `None`.
177+
pub fn get_hash_impl(&self, path: &Path) -> Option<proc_macro2::TokenStream> {
178+
match &self.hash {
179+
TraitImpl::Implemented => Some(quote! {
180+
use std::hash::{Hash, Hasher};
181+
let mut hasher = #path::ReflectHasher::default();
182+
Hash::hash(&std::any::Any::type_id(self), &mut hasher);
183+
Hash::hash(self, &mut hasher);
184+
Some(hasher.finish())
185+
}),
186+
TraitImpl::Custom(impl_fn) => Some(quote! {
187+
Some(#impl_fn(self))
188+
}),
189+
TraitImpl::NotImplemented => None,
190+
}
191+
}
192+
193+
/// Returns the logic for `Reflect::reflect_partial_eq` as a `TokenStream`.
194+
///
195+
/// If `PartialEq` was not registered, returns `None`.
196+
pub fn get_partial_eq_impl(&self) -> Option<proc_macro2::TokenStream> {
197+
match &self.partial_eq {
198+
TraitImpl::Implemented => Some(quote! {
199+
let value = value.any();
200+
if let Some(value) = value.downcast_ref::<Self>() {
201+
Some(std::cmp::PartialEq::eq(self, value))
202+
} else {
203+
Some(false)
204+
}
205+
}),
206+
TraitImpl::Custom(impl_fn) => Some(quote! {
207+
Some(#impl_fn(self, value))
208+
}),
209+
TraitImpl::NotImplemented => None,
210+
}
211+
}
212+
213+
/// Returns the logic for `Reflect::serializable` as a `TokenStream`.
214+
///
215+
/// If `Serialize` was not registered, returns `None`.
216+
pub fn get_serialize_impl(&self, path: &Path) -> Option<proc_macro2::TokenStream> {
217+
match &self.serialize {
218+
TraitImpl::Implemented => Some(quote! {
219+
Some(#path::serde::Serializable::Borrowed(self))
220+
}),
221+
TraitImpl::Custom(impl_fn) => Some(quote! {
222+
Some(#impl_fn(self))
223+
}),
224+
TraitImpl::NotImplemented => None,
225+
}
226+
}
227+
}
228+
229+
impl Parse for ReflectTraits {
230+
fn parse(input: ParseStream) -> syn::Result<Self> {
231+
let result = Punctuated::<NestedMeta, Comma>::parse_terminated(input)?;
232+
Ok(ReflectTraits::from_nested_metas(&result))
233+
}
234+
}

0 commit comments

Comments
 (0)