Skip to content

Commit f1df61e

Browse files
authored
bevy_reflect: Avoid trait bounds on non-generic types (#19929)
# Objective All the derived reflection methods currently have multiple trait bounds on non-generic field types, which serve no purpose. The are emitted because "emit bounds on all fields" is easier than "emit bounds on fields that need them". But improving things isn't too hard. Similarly, lots of useless `Any + Send + Sync` bounds exist on non-generic types. Helps a lot with #19873. ## Solution Remove the unnecessary bounds by only emitting them if the relevant type is generic. ## Testing I used `cargo expand` to confirm the unnecessary bounds are no longer produced. `-Zmacro-stats` output tells me this reduces the size of the `Reflect` code produced for `bevy_ui` by 21.2%.
1 parent f09c3a1 commit f1df61e

File tree

4 files changed

+122
-76
lines changed

4 files changed

+122
-76
lines changed

crates/bevy_reflect/derive/src/lib.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,11 @@ fn match_reflect_impls(ast: DeriveInput, source: ReflectImplSource) -> TokenStre
231231
/// // Generates a where clause like:
232232
/// // impl bevy_reflect::Reflect for Foo
233233
/// // where
234-
/// // Self: Any + Send + Sync,
235-
/// // Vec<Foo>: FromReflect + TypePath,
234+
/// // Foo: Any + Send + Sync,
235+
/// // Vec<Foo>: FromReflect + TypePath + MaybeTyped + RegisterForReflection,
236236
/// ```
237237
///
238-
/// In this case, `Foo` is given the bounds `Vec<Foo>: FromReflect + TypePath`,
238+
/// In this case, `Foo` is given the bounds `Vec<Foo>: FromReflect + ...`,
239239
/// which requires that `Foo` implements `FromReflect`,
240240
/// which requires that `Vec<Foo>` implements `FromReflect`,
241241
/// and so on, resulting in the error.
@@ -283,10 +283,10 @@ fn match_reflect_impls(ast: DeriveInput, source: ReflectImplSource) -> TokenStre
283283
/// //
284284
/// // impl<T: Trait> bevy_reflect::Reflect for Foo<T>
285285
/// // where
286-
/// // Self: Any + Send + Sync,
286+
/// // Foo<T>: Any + Send + Sync,
287287
/// // T::Assoc: Default,
288288
/// // T: TypePath,
289-
/// // T::Assoc: FromReflect + TypePath,
289+
/// // T::Assoc: FromReflect + TypePath + MaybeTyped + RegisterForReflection,
290290
/// // T::Assoc: List,
291291
/// // {/* ... */}
292292
/// ```

crates/bevy_reflect/derive/src/where_clause_options.rs

Lines changed: 107 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::derive_data::ReflectMeta;
22
use bevy_macro_utils::fq_std::{FQAny, FQSend, FQSync};
3-
use proc_macro2::TokenStream;
3+
use proc_macro2::{TokenStream, TokenTree};
44
use quote::{quote, ToTokens};
5-
use syn::{punctuated::Punctuated, Token, Type, WhereClause};
5+
use syn::{punctuated::Punctuated, Ident, Token, Type, WhereClause};
66

77
/// Options defining how to extend the `where` clause for reflection.
88
pub(crate) struct WhereClauseOptions<'a, 'b> {
@@ -29,15 +29,24 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> {
2929
self.meta
3030
}
3131

32-
/// Extends the `where` clause for a type with additional bounds needed for the reflection impls.
32+
/// Extends the `where` clause for a type with additional bounds needed for the reflection
33+
/// impls.
3334
///
3435
/// The default bounds added are as follows:
35-
/// - `Self` has the bounds `Any + Send + Sync`
36-
/// - Type parameters have the bound `TypePath` unless `#[reflect(type_path = false)]` is present
37-
/// - Active fields have the bounds `TypePath` and either `PartialReflect` if `#[reflect(from_reflect = false)]` is present
38-
/// or `FromReflect` otherwise (or no bounds at all if `#[reflect(no_field_bounds)]` is present)
36+
/// - `Self` has:
37+
/// - `Any + Send + Sync` bounds, if generic over types
38+
/// - An `Any` bound, if generic over lifetimes but not types
39+
/// - No bounds, if generic over neither types nor lifetimes
40+
/// - Any given bounds in a `where` clause on the type
41+
/// - Type parameters have the bound `TypePath` unless `#[reflect(type_path = false)]` is
42+
/// present
43+
/// - Active fields with non-generic types have the bounds `TypePath`, either `PartialReflect`
44+
/// if `#[reflect(from_reflect = false)]` is present or `FromReflect` otherwise,
45+
/// `MaybeTyped`, and `RegisterForReflection` (or no bounds at all if
46+
/// `#[reflect(no_field_bounds)]` is present)
3947
///
40-
/// When the derive is used with `#[reflect(where)]`, the bounds specified in the attribute are added as well.
48+
/// When the derive is used with `#[reflect(where)]`, the bounds specified in the attribute are
49+
/// added as well.
4150
///
4251
/// # Example
4352
///
@@ -55,57 +64,69 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> {
5564
/// ```ignore (bevy_reflect is not accessible from this crate)
5665
/// where
5766
/// // `Self` bounds:
58-
/// Self: Any + Send + Sync,
67+
/// Foo<T, U>: Any + Send + Sync,
5968
/// // Type parameter bounds:
6069
/// T: TypePath,
6170
/// U: TypePath,
62-
/// // Field bounds
63-
/// T: FromReflect + TypePath,
71+
/// // Active non-generic field bounds
72+
/// T: FromReflect + TypePath + MaybeTyped + RegisterForReflection,
73+
///
6474
/// ```
6575
///
66-
/// If we had added `#[reflect(where T: MyTrait)]` to the type, it would instead generate:
76+
/// If we add various things to the type:
6777
///
6878
/// ```ignore (bevy_reflect is not accessible from this crate)
69-
/// where
70-
/// // `Self` bounds:
71-
/// Self: Any + Send + Sync,
72-
/// // Type parameter bounds:
73-
/// T: TypePath,
74-
/// U: TypePath,
75-
/// // Field bounds
76-
/// T: FromReflect + TypePath,
77-
/// // Custom bounds
78-
/// T: MyTrait,
79+
/// #[derive(Reflect)]
80+
/// #[reflect(where T: MyTrait)]
81+
/// #[reflect(no_field_bounds)]
82+
/// struct Foo<T, U>
83+
/// where T: Clone
84+
/// {
85+
/// a: T,
86+
/// #[reflect(ignore)]
87+
/// b: U
88+
/// }
7989
/// ```
8090
///
81-
/// And if we also added `#[reflect(no_field_bounds)]` to the type, it would instead generate:
91+
/// It will instead generate the following where clause:
8292
///
8393
/// ```ignore (bevy_reflect is not accessible from this crate)
8494
/// where
8595
/// // `Self` bounds:
86-
/// Self: Any + Send + Sync,
96+
/// Foo<T, U>: Any + Send + Sync,
97+
/// // Given bounds:
98+
/// T: Clone,
8799
/// // Type parameter bounds:
88100
/// T: TypePath,
89101
/// U: TypePath,
102+
/// // No active non-generic field bounds
90103
/// // Custom bounds
91104
/// T: MyTrait,
92105
/// ```
93106
pub fn extend_where_clause(&self, where_clause: Option<&WhereClause>) -> TokenStream {
94-
// We would normally just use `Self`, but that won't work for generating things like assertion functions
95-
// and trait impls for a type's reference (e.g. `impl FromArg for &MyType`)
96-
let this = self.meta.type_path().true_type();
107+
let mut generic_where_clause = quote! { where };
97108

98-
let required_bounds = self.required_bounds();
109+
// Bounds on `Self`. We would normally just use `Self`, but that won't work for generating
110+
// things like assertion functions and trait impls for a type's reference (e.g. `impl
111+
// FromArg for &MyType`).
112+
let generics = self.meta.type_path().generics();
113+
if generics.type_params().next().is_some() {
114+
// Generic over types? We need `Any + Send + Sync`.
115+
let this = self.meta.type_path().true_type();
116+
generic_where_clause.extend(quote! { #this: #FQAny + #FQSend + #FQSync, });
117+
} else if generics.lifetimes().next().is_some() {
118+
// Generic only over lifetimes? We need `'static`.
119+
let this = self.meta.type_path().true_type();
120+
generic_where_clause.extend(quote! { #this: 'static, });
121+
}
99122

100-
// Maintain existing where clause, if any.
101-
let mut generic_where_clause = if let Some(where_clause) = where_clause {
123+
// Maintain existing where clause bounds, if any.
124+
if let Some(where_clause) = where_clause {
102125
let predicates = where_clause.predicates.iter();
103-
quote! {where #this: #required_bounds, #(#predicates,)*}
104-
} else {
105-
quote!(where #this: #required_bounds,)
106-
};
126+
generic_where_clause.extend(quote! { #(#predicates,)* });
127+
}
107128

108-
// Add additional reflection trait bounds
129+
// Add additional reflection trait bounds.
109130
let predicates = self.predicates();
110131
generic_where_clause.extend(quote! {
111132
#predicates
@@ -157,19 +178,57 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> {
157178
let bevy_reflect_path = self.meta.bevy_reflect_path();
158179
let reflect_bound = self.reflect_bound();
159180

160-
// `TypePath` is always required for active fields since they are used to
161-
// construct `NamedField` and `UnnamedField` instances for the `Typed` impl.
162-
// Likewise, `GetTypeRegistration` is always required for active fields since
163-
// they are used to register the type's dependencies.
164-
Some(self.active_fields.iter().map(move |ty| {
165-
quote!(
166-
#ty : #reflect_bound
167-
+ #bevy_reflect_path::TypePath
168-
// Needed for `Typed` impls
169-
+ #bevy_reflect_path::MaybeTyped
170-
// Needed for `GetTypeRegistration` impls
171-
+ #bevy_reflect_path::__macro_exports::RegisterForReflection
172-
)
181+
// Get the identifiers of all type parameters.
182+
let type_param_idents = self
183+
.meta
184+
.type_path()
185+
.generics()
186+
.type_params()
187+
.map(|type_param| type_param.ident.clone())
188+
.collect::<Vec<Ident>>();
189+
190+
// Do any of the identifiers in `idents` appear in `token_stream`?
191+
fn is_any_ident_in_token_stream(idents: &[Ident], token_stream: TokenStream) -> bool {
192+
for token_tree in token_stream {
193+
match token_tree {
194+
TokenTree::Ident(ident) => {
195+
if idents.contains(&ident) {
196+
return true;
197+
}
198+
}
199+
TokenTree::Group(group) => {
200+
if is_any_ident_in_token_stream(idents, group.stream()) {
201+
return true;
202+
}
203+
}
204+
TokenTree::Punct(_) | TokenTree::Literal(_) => {}
205+
}
206+
}
207+
false
208+
}
209+
210+
Some(self.active_fields.iter().filter_map(move |ty| {
211+
// Field type bounds are only required if `ty` is generic. How to determine that?
212+
// Search `ty`s token stream for identifiers that match the identifiers from the
213+
// function's type params. E.g. if `T` and `U` are the type param identifiers and
214+
// `ty` is `Vec<[T; 4]>` then the `T` identifiers match. This is a bit hacky, but
215+
// it works.
216+
let is_generic =
217+
is_any_ident_in_token_stream(&type_param_idents, ty.to_token_stream());
218+
219+
is_generic.then(|| {
220+
quote!(
221+
#ty: #reflect_bound
222+
// Needed to construct `NamedField` and `UnnamedField` instances for
223+
// the `Typed` impl.
224+
+ #bevy_reflect_path::TypePath
225+
// Needed for `Typed` impls
226+
+ #bevy_reflect_path::MaybeTyped
227+
// Needed for registering type dependencies in the
228+
// `GetTypeRegistration` impl.
229+
+ #bevy_reflect_path::__macro_exports::RegisterForReflection
230+
)
231+
})
173232
}))
174233
}
175234
}
@@ -194,9 +253,4 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> {
194253
None
195254
}
196255
}
197-
198-
/// The minimum required bounds for a type to be reflected.
199-
fn required_bounds(&self) -> TokenStream {
200-
quote!(#FQAny + #FQSend + #FQSync)
201-
}
202256
}

crates/bevy_reflect/src/impls/core/sync.rs

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use crate::{
1010
};
1111
use bevy_platform::prelude::*;
1212
use bevy_reflect_derive::impl_type_path;
13-
use core::any::Any;
1413
use core::fmt;
1514

1615
macro_rules! impl_reflect_for_atomic {
@@ -21,10 +20,7 @@ macro_rules! impl_reflect_for_atomic {
2120
#[cfg(feature = "functions")]
2221
crate::func::macros::impl_function_traits!($ty);
2322

24-
impl GetTypeRegistration for $ty
25-
where
26-
$ty: Any + Send + Sync,
27-
{
23+
impl GetTypeRegistration for $ty {
2824
fn get_type_registration() -> TypeRegistration {
2925
let mut registration = TypeRegistration::of::<Self>();
3026
registration.insert::<ReflectFromPtr>(FromType::<Self>::from_type());
@@ -42,10 +38,7 @@ macro_rules! impl_reflect_for_atomic {
4238
}
4339
}
4440

45-
impl Typed for $ty
46-
where
47-
$ty: Any + Send + Sync,
48-
{
41+
impl Typed for $ty {
4942
fn type_info() -> &'static TypeInfo {
5043
static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new();
5144
CELL.get_or_set(|| {
@@ -55,10 +48,7 @@ macro_rules! impl_reflect_for_atomic {
5548
}
5649
}
5750

58-
impl PartialReflect for $ty
59-
where
60-
$ty: Any + Send + Sync,
61-
{
51+
impl PartialReflect for $ty {
6252
#[inline]
6353
fn get_represented_type_info(&self) -> Option<&'static TypeInfo> {
6454
Some(<Self as Typed>::type_info())
@@ -128,10 +118,7 @@ macro_rules! impl_reflect_for_atomic {
128118
}
129119
}
130120

131-
impl FromReflect for $ty
132-
where
133-
$ty: Any + Send + Sync,
134-
{
121+
impl FromReflect for $ty {
135122
fn from_reflect(reflect: &dyn PartialReflect) -> Option<Self> {
136123
Some(<$ty>::new(
137124
reflect.try_downcast_ref::<$ty>()?.load($ordering),
@@ -140,7 +127,7 @@ macro_rules! impl_reflect_for_atomic {
140127
}
141128
};
142129

143-
impl_full_reflect!(for $ty where $ty: Any + Send + Sync);
130+
impl_full_reflect!(for $ty);
144131
};
145132
}
146133

crates/bevy_reflect/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2615,9 +2615,11 @@ bevy_reflect::tests::Test {
26152615
#[reflect(where T: Default)]
26162616
struct Foo<T>(String, #[reflect(ignore)] PhantomData<T>);
26172617

2618+
#[expect(dead_code, reason = "Bar is never constructed")]
26182619
#[derive(Default, TypePath)]
26192620
struct Bar;
26202621

2622+
#[expect(dead_code, reason = "Baz is never constructed")]
26212623
#[derive(TypePath)]
26222624
struct Baz;
26232625

@@ -2631,6 +2633,7 @@ bevy_reflect::tests::Test {
26312633
#[reflect(where)]
26322634
struct Foo<T>(String, #[reflect(ignore)] PhantomData<T>);
26332635

2636+
#[expect(dead_code, reason = "Bar is never constructed")]
26342637
#[derive(TypePath)]
26352638
struct Bar;
26362639

@@ -2665,13 +2668,15 @@ bevy_reflect::tests::Test {
26652668
#[reflect(where T::Assoc: core::fmt::Display)]
26662669
struct Foo<T: Trait>(T::Assoc);
26672670

2671+
#[expect(dead_code, reason = "Bar is never constructed")]
26682672
#[derive(TypePath)]
26692673
struct Bar;
26702674

26712675
impl Trait for Bar {
26722676
type Assoc = usize;
26732677
}
26742678

2679+
#[expect(dead_code, reason = "Baz is never constructed")]
26752680
#[derive(TypePath)]
26762681
struct Baz;
26772682

0 commit comments

Comments
 (0)