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

bevy_reflect: Fix trailing comma breaking derives (#8014)

Browse Source 
# Objective

Fixes #7989

Based on #7991 by @CoffeeVampir3

## Solution

There were three parts to this issue:
1. `extend_where_clause` did not account for the optionality of a where
clause's trailing comma
    ```rust
    // OKAY
    struct Foo<T> where T: Asset, {/* ... */}
    // ERROR
    struct Foo<T> where T: Asset {/* ... */}
    ```
2. `FromReflect` derive logic was not actively using
`extend_where_clause` which led to some inconsistencies (enums weren't
adding _any_ additional bounds even)
3. Using `extend_where_clause` in the `FromReflect` derive logic meant
we had to optionally add `Default` bounds to ignored fields iff the
entire item itself was not already `Default` (otherwise the definition
for `Handle<T>` wouldn't compile since `HandleType` doesn't impl
`Default` but `Handle<T>` itself does)

---

## Changelog

- Fixed issue where a missing trailing comma could break the reflection
derives
This commit is contained in:
Gino Valente 2023-03-27 14:47:33 -07:00 committed by GitHub
parent 846b748c6f
commit 5e5a305d43
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 314 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs
View File

@ -3,7 +3,7 @@ use crate::derive_data::ReflectEnum;
use crate::enum_utility::{get_variant_constructors, EnumVariantConstructors};
use crate::field_attributes::DefaultBehavior;
use crate::fq_std::{FQAny, FQClone, FQDefault, FQOption};
use crate::utility::ident_or_index;
use crate::utility::{extend_where_clause, ident_or_index, WhereClauseOptions};
use crate::{ReflectMeta, ReflectStruct};
use proc_macro::TokenStream;
use proc_macro2::Span;
@ -49,8 +49,20 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
let (impl_generics, ty_generics, where_clause) =
reflect_enum.meta().generics().split_for_impl();
// Add FromReflect bound for each active field
let where_from_reflect_clause = extend_where_clause(
where_clause,
&WhereClauseOptions {
active_types: reflect_enum.active_types().into_boxed_slice(),
ignored_types: reflect_enum.ignored_types().into_boxed_slice(),
active_trait_bounds: quote!(#bevy_reflect_path::FromReflect),
ignored_trait_bounds: quote!(#FQDefault),
},
);
TokenStream::from(quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_clause {
impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_from_reflect_clause {
fn from_reflect(#ref_value: &dyn #bevy_reflect_path::Reflect) -> #FQOption<Self> {
if let #bevy_reflect_path::ReflectRef::Enum(#ref_value) = #bevy_reflect_path::Reflect::reflect_ref(#ref_value) {
match #bevy_reflect_path::Enum::variant_name(#ref_value) {
@ -89,11 +101,11 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
Ident::new("Struct", Span::call_site())
};
let field_types = reflect_struct.active_types();
let MemberValuePair(active_members, active_values) =
get_active_fields(reflect_struct, &ref_struct, &ref_struct_type, is_tuple);
let constructor = if reflect_struct.meta().traits().contains(REFLECT_DEFAULT) {
let is_defaultable = reflect_struct.meta().traits().contains(REFLECT_DEFAULT);
let constructor = if is_defaultable {
quote!(
let mut __this: Self = #FQDefault::default();
#(
@ -120,16 +132,19 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
// Add FromReflect bound for each active field
let mut where_from_reflect_clause = if where_clause.is_some() {
quote! {#where_clause}
} else if !active_members.is_empty() {
quote! {where}
let where_from_reflect_clause = extend_where_clause(
where_clause,
&WhereClauseOptions {
active_types: reflect_struct.active_types().into_boxed_slice(),
ignored_types: reflect_struct.ignored_types().into_boxed_slice(),
active_trait_bounds: quote!(#bevy_reflect_path::FromReflect),
ignored_trait_bounds: if is_defaultable {
quote!()
} else {
quote! {}
};
where_from_reflect_clause.extend(quote! {
#(#field_types: #bevy_reflect_path::FromReflect,)*
});
quote!(#FQDefault)
},
},
);
TokenStream::from(quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #struct_name #ty_generics #where_from_reflect_clause

5
crates/bevy_reflect/bevy_reflect_derive/src/utility.rs
View File

@ -122,8 +122,9 @@ pub(crate) fn extend_where_clause(
let active_trait_bounds = &where_clause_options.active_trait_bounds;
let ignored_trait_bounds = &where_clause_options.ignored_trait_bounds;
let mut generic_where_clause = if where_clause.is_some() {
quote! {#where_clause}
let mut generic_where_clause = if let Some(where_clause) = where_clause {
let predicates = where_clause.predicates.iter();
quote! {where #(#predicates,)*}
} else if !(active_types.is_empty() && ignored_types.is_empty()) {
quote! {where}
} else {

282
crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/bounds.pass.rs Normal file
View File

@ -0,0 +1,282 @@
use bevy_reflect::prelude::*;
fn main() {}
#[derive(Default)]
struct NonReflect;
struct NonReflectNonDefault;
mod structs {
use super::*;
#[derive(Reflect)]
struct ReflectGeneric<T> {
foo: T,
#[reflect(ignore)]
_ignored: NonReflect,
}
#[derive(Reflect, FromReflect)]
struct FromReflectGeneric<T> {
foo: T,
#[reflect(ignore)]
_ignored: NonReflect,
}
#[derive(Reflect, FromReflect)]
#[reflect(Default)]
struct DefaultGeneric<T> {
foo: Option<T>,
#[reflect(ignore)]
_ignored: NonReflectNonDefault,
}
impl<T> Default for DefaultGeneric<T> {
fn default() -> Self {
Self {
foo: None,
_ignored: NonReflectNonDefault,
}
}
}
#[derive(Reflect)]
struct ReflectBoundGeneric<T: Clone> {
foo: T,
#[reflect(ignore)]
_ignored: NonReflect,
}
#[derive(Reflect, FromReflect)]
struct FromReflectBoundGeneric<T: Clone> {
foo: T,
#[reflect(ignore)]
_ignored: NonReflect,
}
#[derive(Reflect, FromReflect)]
#[reflect(Default)]
struct DefaultBoundGeneric<T: Clone> {
foo: Option<T>,
#[reflect(ignore)]
_ignored: NonReflectNonDefault,
}
impl<T: Clone> Default for DefaultBoundGeneric<T> {
fn default() -> Self {
Self {
foo: None,
_ignored: NonReflectNonDefault,
}
}
}
#[derive(Reflect)]
struct ReflectGenericWithWhere<T>
where
T: Clone,
{
foo: T,
#[reflect(ignore)]
_ignored: NonReflect,
}
#[derive(Reflect, FromReflect)]
struct FromReflectGenericWithWhere<T>
where
T: Clone,
{
foo: T,
#[reflect(ignore)]
_ignored: NonReflect,
}
#[derive(Reflect, FromReflect)]
#[reflect(Default)]
struct DefaultGenericWithWhere<T>
where
T: Clone,
{
foo: Option<T>,
#[reflect(ignore)]
_ignored: NonReflectNonDefault,
}
impl<T> Default for DefaultGenericWithWhere<T>
where
T: Clone,
{
fn default() -> Self {
Self {
foo: None,
_ignored: NonReflectNonDefault,
}
}
}
#[derive(Reflect)]
#[rustfmt::skip]
struct ReflectGenericWithWhereNoTrailingComma<T>
where
T: Clone
{
foo: T,
#[reflect(ignore)]
_ignored: NonReflect,
}
#[derive(Reflect, FromReflect)]
#[rustfmt::skip]
struct FromReflectGenericWithWhereNoTrailingComma<T>
where
T: Clone
{
foo: T,
#[reflect(ignore)]
_ignored: NonReflect,
}
#[derive(Reflect, FromReflect)]
#[reflect(Default)]
#[rustfmt::skip]
struct DefaultGenericWithWhereNoTrailingComma<T>
where
T: Clone
{
foo: Option<T>,
#[reflect(ignore)]
_ignored: NonReflectNonDefault,
}
impl<T> Default for DefaultGenericWithWhereNoTrailingComma<T>
where
T: Clone,
{
fn default() -> Self {
Self {
foo: None,
_ignored: NonReflectNonDefault,
}
}
}
}
mod tuple_structs {
use super::*;
#[derive(Reflect)]
struct ReflectGeneric<T>(T, #[reflect(ignore)] NonReflect);
#[derive(Reflect, FromReflect)]
struct FromReflectGeneric<T>(T, #[reflect(ignore)] NonReflect);
#[derive(Reflect, FromReflect)]
#[reflect(Default)]
struct DefaultGeneric<T>(Option<T>, #[reflect(ignore)] NonReflectNonDefault);
impl<T> Default for DefaultGeneric<T> {
fn default() -> Self {
Self(None, NonReflectNonDefault)
}
}
#[derive(Reflect)]
struct ReflectBoundGeneric<T: Clone>(T, #[reflect(ignore)] NonReflect);
#[derive(Reflect, FromReflect)]
struct FromReflectBoundGeneric<T: Clone>(T, #[reflect(ignore)] NonReflect);
#[derive(Reflect, FromReflect)]
#[reflect(Default)]
struct DefaultBoundGeneric<T: Clone>(Option<T>, #[reflect(ignore)] NonReflectNonDefault);
impl<T: Clone> Default for DefaultBoundGeneric<T> {
fn default() -> Self {
Self(None, NonReflectNonDefault)
}
}
#[derive(Reflect)]
struct ReflectGenericWithWhere<T>(T, #[reflect(ignore)] NonReflect)
where
T: Clone;
#[derive(Reflect, FromReflect)]
struct FromReflectGenericWithWhere<T>(T, #[reflect(ignore)] NonReflect)
where
T: Clone;
#[derive(Reflect, FromReflect)]
#[reflect(Default)]
struct DefaultGenericWithWhere<T>(Option<T>, #[reflect(ignore)] NonReflectNonDefault)
where
T: Clone;
impl<T> Default for DefaultGenericWithWhere<T>
where
T: Clone,
{
fn default() -> Self {
Self(None, NonReflectNonDefault)
}
}
}
mod enums {
use super::*;
#[derive(Reflect)]
enum ReflectGeneric<T> {
Foo(T, #[reflect(ignore)] NonReflect),
}
#[derive(Reflect, FromReflect)]
enum FromReflectGeneric<T> {
Foo(T, #[reflect(ignore)] NonReflect),
}
#[derive(Reflect)]
enum ReflectBoundGeneric<T: Clone> {
Foo(T, #[reflect(ignore)] NonReflect),
}
#[derive(Reflect, FromReflect)]
enum FromReflectBoundGeneric<T: Clone> {
Foo(T, #[reflect(ignore)] NonReflect),
}
#[derive(Reflect)]
enum ReflectGenericWithWhere<T>
where
T: Clone,
{
Foo(T, #[reflect(ignore)] NonReflect),
}
#[derive(Reflect, FromReflect)]
enum FromReflectGenericWithWhere<T>
where
T: Clone,
{
Foo(T, #[reflect(ignore)] NonReflect),
}
#[derive(Reflect)]
#[rustfmt::skip]
enum ReflectGenericWithWhereNoTrailingComma<T>
where
T: Clone
{
Foo(T, #[reflect(ignore)] NonReflect),
}
#[derive(Reflect, FromReflect)]
#[rustfmt::skip]
enum FromReflectGenericWithWhereNoTrailingComma<T>
where
T: Clone
{
Foo(T, #[reflect(ignore)] NonReflect),
}
}