Skip to content

Commit 44e9cd4

Browse files
committed
Add attribute to ignore fields of derived labels (#5366)
# Objective Fixes #5362 ## Solution Add the attribute `#[label(ignore_fields)]` for `*Label` types. ```rust #[derive(SystemLabel)] pub enum MyLabel { One, // Previously this was not allowed since labels cannot contain data. #[system_label(ignore_fields)] Two(PhantomData<usize>), } ``` ## Notes This label makes it possible for equality to behave differently depending on whether or not you are treating the type as a label. For example: ```rust #[derive(SystemLabel, PartialEq, Eq)] #[system_label(ignore_fields)] pub struct Foo(usize); ``` If you compare it as a label, it will ignore the wrapped fields as the user requested. But if you compare it as a `Foo`, the derive will incorrectly compare the inner fields. I see a few solutions 1. Do nothing. This is technically intended behavior, but I think we should do our best to prevent footguns. 2. Generate impls of `PartialEq` and `Eq` along with the `#[derive(Label)]` macros. This is a breaking change as it requires all users to remove these derives from their types. 3. Only allow `PhantomData` to be used with `ignore_fields` -- seems needlessly prescriptive. --- ## Changelog * Added the `ignore_fields` attribute to the derive macros for `*Label` types. * Added an example showing off different forms of the derive macro. <!-- ## Migration Guide > This section is optional. If there are no breaking changes, you can delete this section. - If this PR is a breaking change (relative to the last release of Bevy), describe how a user might need to migrate their code to support these changes - Simply adding new functionality is not a breaking change. - Fixing behavior that was definitely a bug, rather than a questionable design choice is not a breaking change. -->
1 parent 1ac8a47 commit 44e9cd4

File tree

5 files changed

+166
-22
lines changed

5 files changed

+166
-22
lines changed

crates/bevy_derive/src/lib.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,14 @@ pub fn derive_enum_variant_meta(input: TokenStream) -> TokenStream {
8080
enum_variant_meta::derive_enum_variant_meta(input)
8181
}
8282

83-
#[proc_macro_derive(AppLabel)]
83+
/// Generates an impl of the `AppLabel` trait.
84+
///
85+
/// This works only for unit structs, or enums with only unit variants.
86+
/// You may force a struct or variant to behave as if it were fieldless with `#[app_label(ignore_fields)]`.
87+
#[proc_macro_derive(AppLabel, attributes(app_label))]
8488
pub fn derive_app_label(input: TokenStream) -> TokenStream {
8589
let input = syn::parse_macro_input!(input as syn::DeriveInput);
8690
let mut trait_path = BevyManifest::default().get_path("bevy_app");
8791
trait_path.segments.push(format_ident!("AppLabel").into());
88-
derive_label(input, &trait_path)
92+
derive_label(input, &trait_path, "app_label")
8993
}

crates/bevy_ecs/Cargo.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,7 @@ path = "examples/resources.rs"
4141
[[example]]
4242
name = "change_detection"
4343
path = "examples/change_detection.rs"
44+
45+
[[example]]
46+
name = "derive_label"
47+
path = "examples/derive_label.rs"
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
use std::marker::PhantomData;
2+
3+
use bevy_ecs::prelude::*;
4+
5+
fn main() {
6+
// Unit labels are always equal.
7+
assert_eq!(UnitLabel.as_label(), UnitLabel.as_label());
8+
9+
// Enum labels depend on the variant.
10+
assert_eq!(EnumLabel::One.as_label(), EnumLabel::One.as_label());
11+
assert_ne!(EnumLabel::One.as_label(), EnumLabel::Two.as_label());
12+
13+
// Labels annotated with `ignore_fields` ignore their fields.
14+
assert_eq!(WeirdLabel(1).as_label(), WeirdLabel(2).as_label());
15+
16+
// Labels don't depend only on the variant name but on the full type
17+
assert_ne!(
18+
GenericLabel::<f64>::One.as_label(),
19+
GenericLabel::<char>::One.as_label(),
20+
);
21+
}
22+
23+
#[derive(SystemLabel)]
24+
pub struct UnitLabel;
25+
26+
#[derive(SystemLabel)]
27+
pub enum EnumLabel {
28+
One,
29+
Two,
30+
}
31+
32+
#[derive(SystemLabel)]
33+
#[system_label(ignore_fields)]
34+
pub struct WeirdLabel(i32);
35+
36+
#[derive(SystemLabel)]
37+
pub enum GenericLabel<T> {
38+
One,
39+
#[system_label(ignore_fields)]
40+
Two(PhantomData<T>),
41+
}
42+
43+
// FIXME: this should be a compile_fail test
44+
/*#[derive(SystemLabel)]
45+
pub union Foo {
46+
x: i32,
47+
}*/
48+
49+
// FIXME: this should be a compile_fail test
50+
/*#[derive(SystemLabel)]
51+
#[system_label(ignore_fields)]
52+
pub enum BadLabel {
53+
One,
54+
Two,
55+
}*/
56+
57+
// FIXME: this should be a compile_fail test
58+
/*#[derive(SystemLabel)]
59+
pub struct BadLabel2 {
60+
#[system_label(ignore_fields)]
61+
x: (),
62+
}*/

crates/bevy_ecs/macros/src/lib.rs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -434,46 +434,62 @@ pub fn derive_world_query(input: TokenStream) -> TokenStream {
434434
derive_world_query_impl(ast)
435435
}
436436

437-
#[proc_macro_derive(SystemLabel)]
437+
/// Generates an impl of the `SystemLabel` trait.
438+
///
439+
/// This works only for unit structs, or enums with only unit variants.
440+
/// You may force a struct or variant to behave as if it were fieldless with `#[system_label(ignore_fields)]`.
441+
#[proc_macro_derive(SystemLabel, attributes(system_label))]
438442
pub fn derive_system_label(input: TokenStream) -> TokenStream {
439443
let input = parse_macro_input!(input as DeriveInput);
440444
let mut trait_path = bevy_ecs_path();
441445
trait_path.segments.push(format_ident!("schedule").into());
442446
trait_path
443447
.segments
444448
.push(format_ident!("SystemLabel").into());
445-
derive_label(input, &trait_path)
449+
derive_label(input, &trait_path, "system_label")
446450
}
447451

448-
#[proc_macro_derive(StageLabel)]
452+
/// Generates an impl of the `StageLabel` trait.
453+
///
454+
/// This works only for unit structs, or enums with only unit variants.
455+
/// You may force a struct or variant to behave as if it were fieldless with `#[stage_label(ignore_fields)]`.
456+
#[proc_macro_derive(StageLabel, attributes(stage_label))]
449457
pub fn derive_stage_label(input: TokenStream) -> TokenStream {
450458
let input = parse_macro_input!(input as DeriveInput);
451459
let mut trait_path = bevy_ecs_path();
452460
trait_path.segments.push(format_ident!("schedule").into());
453461
trait_path.segments.push(format_ident!("StageLabel").into());
454-
derive_label(input, &trait_path)
462+
derive_label(input, &trait_path, "stage_label")
455463
}
456464

457-
#[proc_macro_derive(AmbiguitySetLabel)]
465+
/// Generates an impl of the `AmbiguitySetLabel` trait.
466+
///
467+
/// This works only for unit structs, or enums with only unit variants.
468+
/// You may force a struct or variant to behave as if it were fieldless with `#[ambiguity_set_label(ignore_fields)]`.
469+
#[proc_macro_derive(AmbiguitySetLabel, attributes(ambiguity_set_label))]
458470
pub fn derive_ambiguity_set_label(input: TokenStream) -> TokenStream {
459471
let input = parse_macro_input!(input as DeriveInput);
460472
let mut trait_path = bevy_ecs_path();
461473
trait_path.segments.push(format_ident!("schedule").into());
462474
trait_path
463475
.segments
464476
.push(format_ident!("AmbiguitySetLabel").into());
465-
derive_label(input, &trait_path)
477+
derive_label(input, &trait_path, "ambiguity_set_label")
466478
}
467479

468-
#[proc_macro_derive(RunCriteriaLabel)]
480+
/// Generates an impl of the `RunCriteriaLabel` trait.
481+
///
482+
/// This works only for unit structs, or enums with only unit variants.
483+
/// You may force a struct or variant to behave as if it were fieldless with `#[run_criteria_label(ignore_fields)]`.
484+
#[proc_macro_derive(RunCriteriaLabel, attributes(run_criteria_label))]
469485
pub fn derive_run_criteria_label(input: TokenStream) -> TokenStream {
470486
let input = parse_macro_input!(input as DeriveInput);
471487
let mut trait_path = bevy_ecs_path();
472488
trait_path.segments.push(format_ident!("schedule").into());
473489
trait_path
474490
.segments
475491
.push(format_ident!("RunCriteriaLabel").into());
476-
derive_label(input, &trait_path)
492+
derive_label(input, &trait_path, "run_criteria_label")
477493
}
478494

479495
pub(crate) fn bevy_ecs_path() -> syn::Path {

crates/bevy_macro_utils/src/lib.rs

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ pub use shape::*;
99
pub use symbol::*;
1010

1111
use proc_macro::TokenStream;
12-
use quote::quote;
12+
use quote::{quote, quote_spanned};
1313
use std::{env, path::PathBuf};
14+
use syn::spanned::Spanned;
1415
use toml::{map::Map, Value};
1516

1617
pub struct BevyManifest {
@@ -110,8 +111,26 @@ impl BevyManifest {
110111
///
111112
/// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait
112113
/// - `trait_path`: The path [`syn::Path`] to the label trait
113-
pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream {
114-
let ident = input.ident;
114+
pub fn derive_label(
115+
input: syn::DeriveInput,
116+
trait_path: &syn::Path,
117+
attr_name: &str,
118+
) -> TokenStream {
119+
// return true if the variant specified is an `ignore_fields` attribute
120+
fn is_ignore(attr: &syn::Attribute, attr_name: &str) -> bool {
121+
if attr.path.get_ident().as_ref().unwrap() != &attr_name {
122+
return false;
123+
}
124+
125+
syn::custom_keyword!(ignore_fields);
126+
attr.parse_args_with(|input: syn::parse::ParseStream| {
127+
let ignore = input.parse::<Option<ignore_fields>>()?.is_some();
128+
Ok(ignore)
129+
})
130+
.unwrap()
131+
}
132+
133+
let ident = input.ident.clone();
115134

116135
let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
117136
let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause {
@@ -123,30 +142,69 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr
123142
.push(syn::parse2(quote! { Self: 'static }).unwrap());
124143

125144
let as_str = match input.data {
126-
syn::Data::Struct(d) => match d.fields {
127-
syn::Fields::Unit => {
145+
syn::Data::Struct(d) => {
146+
// see if the user tried to ignore fields incorrectly
147+
if let Some(attr) = d
148+
.fields
149+
.iter()
150+
.flat_map(|f| &f.attrs)
151+
.find(|a| is_ignore(a, attr_name))
152+
{
153+
let err_msg = format!("`#[{attr_name}(ignore_fields)]` cannot be applied to fields individually: add it to the struct declaration");
154+
return quote_spanned! {
155+
attr.span() => compile_error!(#err_msg);
156+
}
157+
.into();
158+
}
159+
// Structs must either be fieldless, or explicitly ignore the fields.
160+
let ignore_fields = input.attrs.iter().any(|a| is_ignore(a, attr_name));
161+
if matches!(d.fields, syn::Fields::Unit) || ignore_fields {
128162
let lit = ident.to_string();
129163
quote! { #lit }
164+
} else {
165+
let err_msg = format!("Labels cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`");
166+
return quote_spanned! {
167+
d.fields.span() => compile_error!(#err_msg);
168+
}
169+
.into();
130170
}
131-
_ => panic!("Labels cannot contain data."),
132-
},
171+
}
133172
syn::Data::Enum(d) => {
134-
let arms = d.variants.iter().map(|v| match v.fields {
135-
syn::Fields::Unit => {
173+
// check if the user put #[label(ignore_fields)] in the wrong place
174+
if let Some(attr) = input.attrs.iter().find(|a| is_ignore(a, attr_name)) {
175+
let err_msg = format!("`#[{attr_name}(ignore_fields)]` can only be applied to enum variants or struct declarations");
176+
return quote_spanned! {
177+
attr.span() => compile_error!(#err_msg);
178+
}
179+
.into();
180+
}
181+
let arms = d.variants.iter().map(|v| {
182+
// Variants must either be fieldless, or explicitly ignore the fields.
183+
let ignore_fields = v.attrs.iter().any(|a| is_ignore(a, attr_name));
184+
if matches!(v.fields, syn::Fields::Unit) | ignore_fields {
136185
let mut path = syn::Path::from(ident.clone());
137186
path.segments.push(v.ident.clone().into());
138187
let lit = format!("{ident}::{}", v.ident.clone());
139-
quote! { #path => #lit }
188+
quote! { #path { .. } => #lit }
189+
} else {
190+
let err_msg = format!("Label variants cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`");
191+
quote_spanned! {
192+
v.fields.span() => _ => { compile_error!(#err_msg); }
193+
}
140194
}
141-
_ => panic!("Label variants cannot contain data."),
142195
});
143196
quote! {
144197
match self {
145198
#(#arms),*
146199
}
147200
}
148201
}
149-
syn::Data::Union(_) => panic!("Unions cannot be used as labels."),
202+
syn::Data::Union(_) => {
203+
return quote_spanned! {
204+
input.span() => compile_error!("Unions cannot be used as labels.");
205+
}
206+
.into();
207+
}
150208
};
151209

152210
(quote! {

0 commit comments

Comments
 (0)