Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bevy_reflect: Split #[reflect(where)] #11597

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion crates/bevy_asset/src/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ impl std::fmt::Debug for StrongHandle {
/// [`Handle::Strong`] also provides access to useful [`Asset`] metadata, such as the [`AssetPath`] (if it exists).
#[derive(Component, Reflect)]
#[reflect(Component)]
#[reflect(where A: Asset)]
pub enum Handle<A: Asset> {
/// A "strong" reference to a live (or loading) [`Asset`]. If a [`Handle`] is [`Handle::Strong`], the [`Asset`] will be kept
/// alive until the [`Handle`] is dropped. Strong handles also provide access to additional asset metadata.
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_asset/src/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use thiserror::Error;
///
/// For an "untyped" / "generic-less" id, see [`UntypedAssetId`].
#[derive(Reflect)]
#[reflect(where A: Asset)]
pub enum AssetId<A: Asset> {
/// A small / efficient runtime identifier that can be used to efficiently look up an asset stored in [`Assets`]. This is
/// the "default" identifier used for assets. The alternative(s) (ex: [`AssetId::Uuid`]) will only be used if assets are
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ const HASH_ATTR: &str = "Hash";
// but useful to know exist nonetheless
pub(crate) const REFLECT_DEFAULT: &str = "ReflectDefault";

// Attributes for `Reflect` implementation
const NO_FIELD_BOUNDS_ATTR: &str = "no_field_bounds";

// Attributes for `FromReflect` implementation
const FROM_REFLECT_ATTR: &str = "from_reflect";

Expand Down Expand Up @@ -212,6 +215,7 @@ pub(crate) struct ReflectTraits {
from_reflect_attrs: FromReflectAttrs,
type_path_attrs: TypePathAttrs,
custom_where: Option<WhereClause>,
no_field_bounds: bool,
idents: Vec<Ident>,
}

Expand Down Expand Up @@ -260,6 +264,9 @@ impl ReflectTraits {
HASH_ATTR => {
traits.hash.merge(TraitImpl::Implemented(span))?;
}
NO_FIELD_BOUNDS_ATTR => {
traits.no_field_bounds = true;
}
// We only track reflected idents for traits not considered special
_ => {
// Create the reflect ident
Expand Down Expand Up @@ -422,6 +429,10 @@ impl ReflectTraits {
self.custom_where.as_ref()
}

pub fn no_field_bounds(&self) -> bool {
self.no_field_bounds
}

/// Merges the trait implementations of this [`ReflectTraits`] with another one.
///
/// An error is returned if the two [`ReflectTraits`] have conflicting implementations.
Expand All @@ -434,6 +445,8 @@ impl ReflectTraits {

self.merge_custom_where(other.custom_where);

self.no_field_bounds |= other.no_field_bounds;

for ident in other.idents {
add_unique_ident(&mut self.idents, ident)?;
}
Expand Down
34 changes: 22 additions & 12 deletions crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::container_attributes::{FromReflectAttrs, ReflectTraits};
use crate::container_attributes::{FromReflectAttrs, ReflectTraits, TypePathAttrs};
use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr};
use crate::type_path::parse_path_no_leading_colon;
use crate::utility::{StringExpr, WhereClauseOptions};
Expand Down Expand Up @@ -402,6 +402,11 @@ impl<'a> ReflectMeta<'a> {
self.traits.from_reflect_attrs()
}

/// The `TypePath` attributes on this type.
pub fn type_path_attrs(&self) -> &TypePathAttrs {
self.traits.type_path_attrs()
}

/// The path to this type.
pub fn type_path(&self) -> &ReflectTypePath<'a> {
&self.type_path
Expand Down Expand Up @@ -480,7 +485,7 @@ impl<'a> ReflectStruct<'a> {
}

pub fn where_clause_options(&self) -> WhereClauseOptions {
WhereClauseOptions::new(self.meta())
WhereClauseOptions::new_with_fields(self.meta(), self.active_types().into_boxed_slice())
}
}

Expand All @@ -503,28 +508,33 @@ impl<'a> ReflectEnum<'a> {
&self.variants
}

/// Get a collection of types which are exposed to the reflection API
pub fn active_types(&self) -> Vec<Type> {
self.active_fields()
.map(|field| field.data.ty.clone())
.collect()
}

/// Get an iterator of fields which are exposed to the reflection API
pub fn active_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.variants
.iter()
.flat_map(|variant| variant.active_fields())
}

pub fn where_clause_options(&self) -> WhereClauseOptions {
WhereClauseOptions::new(self.meta())
WhereClauseOptions::new_with_fields(self.meta(), self.active_types().into_boxed_slice())
}
}

impl<'a> EnumVariant<'a> {
/// Get an iterator of fields which are exposed to the reflection API
#[allow(dead_code)]
pub fn active_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields()
.iter()
.filter(|field| field.attrs.ignore.is_active())
}

/// Get an iterator of fields which are ignored by the reflection API
#[allow(dead_code)]
pub fn ignored_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields()
.iter()
.filter(|field| field.attrs.ignore.is_ignored())
}

/// The complete set of fields in this variant.
#[allow(dead_code)]
pub fn fields(&self) -> &[StructField<'a>] {
Expand Down
13 changes: 7 additions & 6 deletions crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> proc_macro2::TokenStream {
let type_path = meta.type_path();
let bevy_reflect_path = meta.bevy_reflect_path();
let (impl_generics, ty_generics, where_clause) = type_path.generics().split_for_impl();
let where_from_reflect_clause =
WhereClauseOptions::new_type_path(meta).extend_where_clause(where_clause);
let where_from_reflect_clause = WhereClauseOptions::new(meta).extend_where_clause(where_clause);
quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #type_path #ty_generics #where_from_reflect_clause {
fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQOption<Self> {
Expand All @@ -50,8 +49,9 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream
let (impl_generics, ty_generics, where_clause) = enum_path.generics().split_for_impl();

// Add FromReflect bound for each active field
let where_from_reflect_clause =
WhereClauseOptions::new(reflect_enum.meta()).extend_where_clause(where_clause);
let where_from_reflect_clause = reflect_enum
.where_clause_options()
.extend_where_clause(where_clause);

quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #enum_path #ty_generics #where_from_reflect_clause {
Expand Down Expand Up @@ -130,8 +130,9 @@ fn impl_struct_internal(
.split_for_impl();

// Add FromReflect bound for each active field
let where_from_reflect_clause =
WhereClauseOptions::new(reflect_struct.meta()).extend_where_clause(where_clause);
let where_from_reflect_clause = reflect_struct
.where_clause_options()
.extend_where_clause(where_clause);

quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #struct_path #ty_generics #where_from_reflect_clause {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_reflect/bevy_reflect_derive/src/impls/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub(crate) enum TypedProperty {
}

pub(crate) fn impl_type_path(meta: &ReflectMeta) -> proc_macro2::TokenStream {
let where_clause_options = WhereClauseOptions::new_type_path(meta);
let where_clause_options = WhereClauseOptions::new(meta);

if !meta.traits().type_path_attrs().should_auto_derive() {
return proc_macro2::TokenStream::new();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> proc_macro2::TokenStream {
#[cfg(not(feature = "documentation"))]
let with_docs: Option<proc_macro2::TokenStream> = None;

let where_clause_options = WhereClauseOptions::new_type_path(meta);
let where_clause_options = WhereClauseOptions::new(meta);
let typed_impl = impl_typed(
meta,
&where_clause_options,
Expand Down
87 changes: 59 additions & 28 deletions crates/bevy_reflect/bevy_reflect_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,50 +131,85 @@ pub(crate) static TYPE_NAME_ATTRIBUTE_NAME: &str = "type_name";
/// This is useful for when a type can't or shouldn't implement `TypePath`,
/// or if a manual implementation is desired.
///
/// ## `#[reflect(where T: Trait, U::Assoc: Trait, ...)]`
/// ## `#[reflect(no_field_bounds)]`
///
/// This attribute will opt-out of the default trait bounds added to all field types
/// for the generated reflection trait impls.
///
/// Normally, all fields will have the bounds `TypePath`, and either `FromReflect` or `Reflect`
/// depending on if `#[reflect(from_reflect = false)]` is used.
/// However, this might not always be desirable, and so this attribute may be used to remove those bounds.
///
/// ### Example
///
/// If a type is recursive the default bounds will cause an overflow error when building:
///
/// ```ignore (bevy_reflect is not accessible from this crate)
/// #[derive(Reflect)] // ERROR: overflow evaluating the requirement `Foo: FromReflect`
/// struct Foo {
/// foo: Vec<Foo>,
/// }
///
/// // Generates a where clause like:
/// // impl bevy_reflect::Reflect for Foo
/// // where
/// // Self: Any + Send + Sync,
/// // Vec<Foo>: FromReflect + TypePath,
/// ```
///
/// By default, the derive macro will automatically add certain trait bounds to all generic type parameters
/// in order to make them compatible with reflection without the user needing to add them manually.
/// This includes traits like `Reflect` and `FromReflect`.
/// However, this may not always be desired, and some type paramaters can't or shouldn't require those bounds
/// (i.e. their usages in fields are ignored or they're only used for their associated types).
/// In this case, `Foo` is given the bounds `Vec<Foo>: FromReflect + TypePath`,
/// which requires that `Foo` implements `FromReflect`,
/// which requires that `Vec<Foo>` implements `FromReflect`,
/// and so on, resulting in the error.
///
/// With this attribute, you can specify a custom `where` clause to be used instead of the default.
/// If this attribute is present, none of the type parameters will receive the default bounds.
/// Only the bounds specified by the type itself and by this attribute will be used.
/// The only exceptions to this are the `Any`, `Send`, `Sync`, and `TypePath` bounds,
/// which will always be added regardless of this attribute due to their necessity for reflection
/// in general.
/// To fix this, we can add `#[reflect(no_field_bounds)]` to `Foo` to remove the bounds on `Vec<Foo>`:
///
/// This means that if you want to opt-out of the default bounds for _all_ type parameters,
/// you can add `#[reflect(where)]` to the container item to indicate
/// that an empty `where` clause should be used.
/// ```ignore (bevy_reflect is not accessible from this crate)
/// #[derive(Reflect)]
/// #[reflect(no_field_bounds)]
/// struct Foo {
/// foo: Vec<Foo>,
/// }
///
/// // Generates a where clause like:
/// // impl bevy_reflect::Reflect for Foo
/// // where
/// // Self: Any + Send + Sync,
/// ```
///
/// ## `#[reflect(where T: Trait, U::Assoc: Trait, ...)]`
///
/// This attribute can be used to add additional bounds to the generated reflection trait impls.
///
/// This is useful for when a type needs certain bounds only applied to the reflection impls
/// that are not otherwise automatically added by the derive macro.
///
/// ### Example
///
/// In the example below, we want to enforce that `T::Assoc: List` is required in order for
/// `Foo<T>` to be reflectable, but we don't want it to prevent `Foo<T>` from being used
/// in places where `T::Assoc: List` is not required.
///
/// ```ignore
/// trait Trait {
/// type Assoc;
/// }
///
/// #[derive(Reflect)]
/// #[reflect(where T::Assoc: FromReflect)]
/// #[reflect(where T::Assoc: List)]
/// struct Foo<T: Trait> where T::Assoc: Default {
/// value: T::Assoc,
/// }
///
/// // Generates a where clause like the following
/// // (notice that `T` does not have any `Reflect` or `FromReflect` bounds):
/// // Generates a where clause like:
/// //
/// // impl<T: Trait> bevy_reflect::Reflect for Foo<T>
/// // where
/// // Self: 'static,
/// // Self: Any + Send + Sync,
/// // T::Assoc: Default,
/// // T: bevy_reflect::TypePath
/// // + ::core::any::Any
/// // + ::core::marker::Send
/// // + ::core::marker::Sync,
/// // T::Assoc: FromReflect,
/// // T: TypePath,
/// // T::Assoc: FromReflect + TypePath,
/// // T::Assoc: List,
/// // {/* ... */}
/// ```
///
Expand All @@ -191,10 +226,6 @@ pub(crate) static TYPE_NAME_ATTRIBUTE_NAME: &str = "type_name";
/// which may be useful for maintaining invariants, keeping certain data private,
/// or allowing the use of types that do not implement `Reflect` within the container.
///
/// If the field contains a generic type parameter, you will likely need to add a
/// [`#[reflect(where)]`](#reflectwheret-trait-uassoc-trait-)
/// attribute to the container in order to avoid the default bounds being applied to the type parameter.
///
/// ## `#[reflect(skip_serializing)]`
///
/// This works similar to `#[reflect(ignore)]`, but rather than opting out of _all_ of reflection,
Expand Down
Loading