From 5aa520eac08a5b470a492d245bad7b45451084f1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 1 Jul 2025 09:29:24 +1000 Subject: [PATCH] bevy_reflect: avoid useless `with_{custom_attributes,docs}` calls (#19875) # Objective `#[derive(Reflect)]` derives `Typed` impls whose `type_info` methods contain useless calls to `with_custom_attributes` and `with_docs`, e.g.: ``` ::bevy::reflect::NamedField::new::("x") .with_custom_attributes( ::bevy::reflect::attributes::CustomAttributes::default() ) .with_docs(::core::option::Option::None), ``` This hurts compile times and makes the `cargo expand` output harder to read. It might also hurt runtime speed, depending on whether the compiler can optimize away the no-op methods. Avoiding this will help with #19873. ## Solution Check if the attributes/docs are empty before appending the method calls. ## Testing I used `cargo expand` to confirm the useless calls are no longer produced. `-Zmacro-stats` outputs tells me this reduces the size of the `Reflect` impls produced for `bevy_ui` from 1_544_696 bytes to 1_511_214 bytes, a 2.2% drop. Only a small improvement, but it's a start. --- .../derive/src/custom_attributes.rs | 5 + crates/bevy_reflect/derive/src/derive_data.rs | 98 ++++++++++--------- .../bevy_reflect/derive/src/documentation.rs | 5 + 3 files changed, 63 insertions(+), 45 deletions(-) diff --git a/crates/bevy_reflect/derive/src/custom_attributes.rs b/crates/bevy_reflect/derive/src/custom_attributes.rs index f12b6d7c12..1f141f79dc 100644 --- a/crates/bevy_reflect/derive/src/custom_attributes.rs +++ b/crates/bevy_reflect/derive/src/custom_attributes.rs @@ -28,6 +28,11 @@ impl CustomAttributes { Ok(()) } + /// Is the collection empty? + pub fn is_empty(&self) -> bool { + self.attributes.is_empty() + } + /// Parse `@` (custom attribute) attribute. /// /// Examples: diff --git a/crates/bevy_reflect/derive/src/derive_data.rs b/crates/bevy_reflect/derive/src/derive_data.rs index f825cb2905..a0343f619f 100644 --- a/crates/bevy_reflect/derive/src/derive_data.rs +++ b/crates/bevy_reflect/derive/src/derive_data.rs @@ -514,25 +514,27 @@ impl<'a> StructField<'a> { }; let ty = self.reflected_type(); - let custom_attributes = self.attrs.custom_attributes.to_tokens(bevy_reflect_path); - #[cfg_attr( - not(feature = "documentation"), - expect( - unused_mut, - reason = "Needs to be mutable if `documentation` feature is enabled.", - ) - )] let mut info = quote! { - #field_info::new::<#ty>(#name).with_custom_attributes(#custom_attributes) + #field_info::new::<#ty>(#name) }; + let custom_attributes = &self.attrs.custom_attributes; + if !custom_attributes.is_empty() { + let custom_attributes = custom_attributes.to_tokens(bevy_reflect_path); + info.extend(quote! { + .with_custom_attributes(#custom_attributes) + }); + } + #[cfg(feature = "documentation")] { let docs = &self.doc; - info.extend(quote! { - .with_docs(#docs) - }); + if !docs.is_empty() { + info.extend(quote! { + .with_docs(#docs) + }); + } } info @@ -653,19 +655,20 @@ impl<'a> ReflectStruct<'a> { .active_fields() .map(|field| field.to_info_tokens(bevy_reflect_path)); - let custom_attributes = self - .meta - .attrs - .custom_attributes() - .to_tokens(bevy_reflect_path); - let mut info = quote! { #bevy_reflect_path::#info_struct::new::(&[ #(#field_infos),* ]) - .with_custom_attributes(#custom_attributes) }; + let custom_attributes = self.meta.attrs.custom_attributes(); + if !custom_attributes.is_empty() { + let custom_attributes = custom_attributes.to_tokens(bevy_reflect_path); + info.extend(quote! { + .with_custom_attributes(#custom_attributes) + }); + } + if let Some(generics) = generate_generics(self.meta()) { info.extend(quote! { .with_generics(#generics) @@ -675,9 +678,11 @@ impl<'a> ReflectStruct<'a> { #[cfg(feature = "documentation")] { let docs = self.meta().doc(); - info.extend(quote! { - .with_docs(#docs) - }); + if !docs.is_empty() { + info.extend(quote! { + .with_docs(#docs) + }); + } } quote! { @@ -884,19 +889,20 @@ impl<'a> ReflectEnum<'a> { .iter() .map(|variant| variant.to_info_tokens(bevy_reflect_path)); - let custom_attributes = self - .meta - .attrs - .custom_attributes() - .to_tokens(bevy_reflect_path); - let mut info = quote! { #bevy_reflect_path::EnumInfo::new::(&[ #(#variants),* ]) - .with_custom_attributes(#custom_attributes) }; + let custom_attributes = self.meta.attrs.custom_attributes(); + if !custom_attributes.is_empty() { + let custom_attributes = custom_attributes.to_tokens(bevy_reflect_path); + info.extend(quote! { + .with_custom_attributes(#custom_attributes) + }); + } + if let Some(generics) = generate_generics(self.meta()) { info.extend(quote! { .with_generics(#generics) @@ -906,9 +912,11 @@ impl<'a> ReflectEnum<'a> { #[cfg(feature = "documentation")] { let docs = self.meta().doc(); - info.extend(quote! { - .with_docs(#docs) - }); + if !docs.is_empty() { + info.extend(quote! { + .with_docs(#docs) + }); + } } quote! { @@ -1008,26 +1016,26 @@ impl<'a> EnumVariant<'a> { } }; - let custom_attributes = self.attrs.custom_attributes.to_tokens(bevy_reflect_path); - - #[cfg_attr( - not(feature = "documentation"), - expect( - unused_mut, - reason = "Needs to be mutable if `documentation` feature is enabled.", - ) - )] let mut info = quote! { #bevy_reflect_path::#info_struct::new(#args) - .with_custom_attributes(#custom_attributes) }; + let custom_attributes = &self.attrs.custom_attributes; + if !custom_attributes.is_empty() { + let custom_attributes = custom_attributes.to_tokens(bevy_reflect_path); + info.extend(quote! { + .with_custom_attributes(#custom_attributes) + }); + } + #[cfg(feature = "documentation")] { let docs = &self.doc; - info.extend(quote! { - .with_docs(#docs) - }); + if !docs.is_empty() { + info.extend(quote! { + .with_docs(#docs) + }); + } } quote! { diff --git a/crates/bevy_reflect/derive/src/documentation.rs b/crates/bevy_reflect/derive/src/documentation.rs index 33aec4c4f3..4fbcf775f4 100644 --- a/crates/bevy_reflect/derive/src/documentation.rs +++ b/crates/bevy_reflect/derive/src/documentation.rs @@ -61,6 +61,11 @@ impl Documentation { ) } + /// Is the collection empty? + pub fn is_empty(&self) -> bool { + self.docs.is_empty() + } + /// Push a new docstring to the collection pub fn push(&mut self, doc: String) { self.docs.push(doc);