forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add attribute to ignore fields of derived labels (#5366)

Browse Source 
# 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.
-->
This commit is contained in:
JoJoJet 2022-07-19 05:21:19 +00:00
parent 1ac8a476cf
commit 44e9cd4bfc
5 changed files with 166 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
crates/bevy_derive/src/lib.rs
View File

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

4
crates/bevy_ecs/Cargo.toml
View File

@ -41,3 +41,7 @@ path = "examples/resources.rs"
[[example]]
name = "change_detection"
path = "examples/change_detection.rs"
[[example]]
name = "derive_label"
path = "examples/derive_label.rs"

62
crates/bevy_ecs/examples/derive_label.rs Normal file
View File

@ -0,0 +1,62 @@
use std::marker::PhantomData;
use bevy_ecs::prelude::*;
fn main() {
// Unit labels are always equal.
assert_eq!(UnitLabel.as_label(), UnitLabel.as_label());
// Enum labels depend on the variant.
assert_eq!(EnumLabel::One.as_label(), EnumLabel::One.as_label());
assert_ne!(EnumLabel::One.as_label(), EnumLabel::Two.as_label());
// Labels annotated with `ignore_fields` ignore their fields.
assert_eq!(WeirdLabel(1).as_label(), WeirdLabel(2).as_label());
// Labels don't depend only on the variant name but on the full type
assert_ne!(
GenericLabel::<f64>::One.as_label(),
GenericLabel::<char>::One.as_label(),
);
}
#[derive(SystemLabel)]
pub struct UnitLabel;
#[derive(SystemLabel)]
pub enum EnumLabel {
One,
Two,
}
#[derive(SystemLabel)]
#[system_label(ignore_fields)]
pub struct WeirdLabel(i32);
#[derive(SystemLabel)]
pub enum GenericLabel<T> {
One,
#[system_label(ignore_fields)]
Two(PhantomData<T>),
}
// FIXME: this should be a compile_fail test
/*#[derive(SystemLabel)]
pub union Foo {
x: i32,
}*/
// FIXME: this should be a compile_fail test
/*#[derive(SystemLabel)]
#[system_label(ignore_fields)]
pub enum BadLabel {
One,
Two,
}*/
// FIXME: this should be a compile_fail test
/*#[derive(SystemLabel)]
pub struct BadLabel2 {
#[system_label(ignore_fields)]
x: (),
}*/

32
crates/bevy_ecs/macros/src/lib.rs
View File

@ -434,7 +434,11 @@ pub fn derive_world_query(input: TokenStream) -> TokenStream {
derive_world_query_impl(ast)
}
#[proc_macro_derive(SystemLabel)]
/// Generates an impl of the `SystemLabel` trait.
///
/// This works only for unit structs, or enums with only unit variants.
/// You may force a struct or variant to behave as if it were fieldless with `#[system_label(ignore_fields)]`.
#[proc_macro_derive(SystemLabel, attributes(system_label))]
pub fn derive_system_label(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let mut trait_path = bevy_ecs_path();
@ -442,19 +446,27 @@ pub fn derive_system_label(input: TokenStream) -> TokenStream {
trait_path
.segments
.push(format_ident!("SystemLabel").into());
derive_label(input, &trait_path)
derive_label(input, &trait_path, "system_label")
}
#[proc_macro_derive(StageLabel)]
/// Generates an impl of the `StageLabel` trait.
///
/// This works only for unit structs, or enums with only unit variants.
/// You may force a struct or variant to behave as if it were fieldless with `#[stage_label(ignore_fields)]`.
#[proc_macro_derive(StageLabel, attributes(stage_label))]
pub fn derive_stage_label(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let mut trait_path = bevy_ecs_path();
trait_path.segments.push(format_ident!("schedule").into());
trait_path.segments.push(format_ident!("StageLabel").into());
derive_label(input, &trait_path)
derive_label(input, &trait_path, "stage_label")
}
#[proc_macro_derive(AmbiguitySetLabel)]
/// Generates an impl of the `AmbiguitySetLabel` trait.
///
/// This works only for unit structs, or enums with only unit variants.
/// You may force a struct or variant to behave as if it were fieldless with `#[ambiguity_set_label(ignore_fields)]`.
#[proc_macro_derive(AmbiguitySetLabel, attributes(ambiguity_set_label))]
pub fn derive_ambiguity_set_label(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let mut trait_path = bevy_ecs_path();
@ -462,10 +474,14 @@ pub fn derive_ambiguity_set_label(input: TokenStream) -> TokenStream {
trait_path
.segments
.push(format_ident!("AmbiguitySetLabel").into());
derive_label(input, &trait_path)
derive_label(input, &trait_path, "ambiguity_set_label")
}
#[proc_macro_derive(RunCriteriaLabel)]
/// Generates an impl of the `RunCriteriaLabel` trait.
///
/// This works only for unit structs, or enums with only unit variants.
/// You may force a struct or variant to behave as if it were fieldless with `#[run_criteria_label(ignore_fields)]`.
#[proc_macro_derive(RunCriteriaLabel, attributes(run_criteria_label))]
pub fn derive_run_criteria_label(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let mut trait_path = bevy_ecs_path();
@ -473,7 +489,7 @@ pub fn derive_run_criteria_label(input: TokenStream) -> TokenStream {
trait_path
.segments
.push(format_ident!("RunCriteriaLabel").into());
derive_label(input, &trait_path)
derive_label(input, &trait_path, "run_criteria_label")
}
pub(crate) fn bevy_ecs_path() -> syn::Path {

82
crates/bevy_macro_utils/src/lib.rs
View File

@ -9,8 +9,9 @@ pub use shape::*;
pub use symbol::*;
use proc_macro::TokenStream;
use quote::quote;
use quote::{quote, quote_spanned};
use std::{env, path::PathBuf};
use syn::spanned::Spanned;
use toml::{map::Map, Value};
pub struct BevyManifest {
@ -110,8 +111,26 @@ impl BevyManifest {
///
/// - `input`: The [`syn::DeriveInput`] for struct that is deriving the label trait
/// - `trait_path`: The path [`syn::Path`] to the label trait
pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStream {
let ident = input.ident;
pub fn derive_label(
input: syn::DeriveInput,
trait_path: &syn::Path,
attr_name: &str,
) -> TokenStream {
// return true if the variant specified is an `ignore_fields` attribute
fn is_ignore(attr: &syn::Attribute, attr_name: &str) -> bool {
if attr.path.get_ident().as_ref().unwrap() != &attr_name {
return false;
}
syn::custom_keyword!(ignore_fields);
attr.parse_args_with(|input: syn::parse::ParseStream| {
let ignore = input.parse::<Option<ignore_fields>>()?.is_some();
Ok(ignore)
})
.unwrap()
}
let ident = input.ident.clone();
let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
let mut where_clause = where_clause.cloned().unwrap_or_else(|| syn::WhereClause {
@ -123,22 +142,56 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr
.push(syn::parse2(quote! { Self: 'static }).unwrap());
let as_str = match input.data {
syn::Data::Struct(d) => match d.fields {
syn::Fields::Unit => {
syn::Data::Struct(d) => {
// see if the user tried to ignore fields incorrectly
if let Some(attr) = d
.fields
.iter()
.flat_map(|f| &f.attrs)
.find(|a| is_ignore(a, attr_name))
{
let err_msg = format!("`#[{attr_name}(ignore_fields)]` cannot be applied to fields individually: add it to the struct declaration");
return quote_spanned! {
attr.span() => compile_error!(#err_msg);
}
.into();
}
// Structs must either be fieldless, or explicitly ignore the fields.
let ignore_fields = input.attrs.iter().any(|a| is_ignore(a, attr_name));
if matches!(d.fields, syn::Fields::Unit) || ignore_fields {
let lit = ident.to_string();
quote! { #lit }
} else {
let err_msg = format!("Labels cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`");
return quote_spanned! {
d.fields.span() => compile_error!(#err_msg);
}
.into();
}
_ => panic!("Labels cannot contain data."),
},
}
syn::Data::Enum(d) => {
let arms = d.variants.iter().map(|v| match v.fields {
syn::Fields::Unit => {
// check if the user put #[label(ignore_fields)] in the wrong place
if let Some(attr) = input.attrs.iter().find(|a| is_ignore(a, attr_name)) {
let err_msg = format!("`#[{attr_name}(ignore_fields)]` can only be applied to enum variants or struct declarations");
return quote_spanned! {
attr.span() => compile_error!(#err_msg);
}
.into();
}
let arms = d.variants.iter().map(|v| {
// Variants must either be fieldless, or explicitly ignore the fields.
let ignore_fields = v.attrs.iter().any(|a| is_ignore(a, attr_name));
if matches!(v.fields, syn::Fields::Unit) | ignore_fields {
let mut path = syn::Path::from(ident.clone());
path.segments.push(v.ident.clone().into());
let lit = format!("{ident}::{}", v.ident.clone());
quote! { #path => #lit }
quote! { #path { .. } => #lit }
} else {
let err_msg = format!("Label variants cannot contain data, unless explicitly ignored with `#[{attr_name}(ignore_fields)]`");
quote_spanned! {
v.fields.span() => _ => { compile_error!(#err_msg); }
}
}
_ => panic!("Label variants cannot contain data."),
});
quote! {
match self {
@ -146,7 +199,12 @@ pub fn derive_label(input: syn::DeriveInput, trait_path: &syn::Path) -> TokenStr
}
}
}
syn::Data::Union(_) => panic!("Unions cannot be used as labels."),
syn::Data::Union(_) => {
return quote_spanned! {
input.span() => compile_error!("Unions cannot be used as labels.");
}
.into();
}
};
(quote! {