Skip to content

Commit

Permalink
reflection: replace impl_reflect_struct with impl_reflect (#11437)
Browse files Browse the repository at this point in the history
# Objective

- `impl_reflect_struct` doesn't cover tuple structs or enums.
- Problem brought up [on
Discord](https://discord.com/channels/691052431525675048/1002362493634629796/1190623345817960463).

## Solution

- Replaces `impl_reflect_struct` with the new `impl_reflect` which works
for tuple structs and enums too.

---

## Changelog

- Internally in `bevy_reflect_derive`, we have a new `ReflectProvenance`
type which is composed of `ReflectTraitToImpl` and `ReflectSource`.
- `impl_reflect_struct` is gone and totally superseded by
`impl_reflect`.

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
  • Loading branch information
soqb and MrGVSV authored Jan 30, 2024
1 parent 4b7ef44 commit df761af
Show file tree
Hide file tree
Showing 9 changed files with 258 additions and 188 deletions.
24 changes: 10 additions & 14 deletions crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
//! the derive helper attribute for `Reflect`, which looks like:
//! `#[reflect(PartialEq, Default, ...)]` and `#[reflect_value(PartialEq, Default, ...)]`.
use crate::derive_data::ReflectTraitToImpl;
use crate::utility;
use bevy_macro_utils::fq_std::{FQAny, FQOption};
use proc_macro2::{Ident, Span, TokenTree};
use quote::quote_spanned;
use syn::parse::{Parse, ParseStream};
use syn::parse::ParseStream;
use syn::punctuated::Punctuated;
use syn::spanned::Spanned;
use syn::token::Comma;
Expand Down Expand Up @@ -220,10 +221,7 @@ pub(crate) struct ReflectTraits {
}

impl ReflectTraits {
pub fn from_meta_list(
meta: &MetaList,
is_from_reflect_derive: bool,
) -> Result<Self, syn::Error> {
pub fn from_meta_list(meta: &MetaList, trait_: ReflectTraitToImpl) -> Result<Self, syn::Error> {
match meta.tokens.clone().into_iter().next() {
// Handles `#[reflect(where T: Trait, U::Assoc: Trait)]`
Some(TokenTree::Ident(ident)) if ident == "where" => Ok(Self {
Expand All @@ -232,14 +230,14 @@ impl ReflectTraits {
}),
_ => Self::from_metas(
meta.parse_args_with(Punctuated::<Meta, Comma>::parse_terminated)?,
is_from_reflect_derive,
trait_,
),
}
}

fn from_metas(
metas: Punctuated<Meta, Comma>,
is_from_reflect_derive: bool,
trait_: ReflectTraitToImpl,
) -> Result<Self, syn::Error> {
let mut traits = ReflectTraits::default();
for meta in &metas {
Expand Down Expand Up @@ -317,7 +315,7 @@ impl ReflectTraits {
// Override `lit` if this is a `FromReflect` derive.
// This typically means a user is opting out of the default implementation
// from the `Reflect` derive and using the `FromReflect` derive directly instead.
is_from_reflect_derive
(trait_ == ReflectTraitToImpl::FromReflect)
.then(|| LitBool::new(true, Span::call_site()))
.unwrap_or_else(|| lit.clone())
})?);
Expand All @@ -334,6 +332,10 @@ impl ReflectTraits {
Ok(traits)
}

pub fn parse(input: ParseStream, trait_: ReflectTraitToImpl) -> syn::Result<Self> {
ReflectTraits::from_metas(Punctuated::parse_terminated(input)?, trait_)
}

/// Returns true if the given reflected trait name (i.e. `ReflectDefault` for `Default`)
/// is registered for this type.
pub fn contains(&self, name: &str) -> bool {
Expand Down Expand Up @@ -466,12 +468,6 @@ impl ReflectTraits {
}
}

impl Parse for ReflectTraits {
fn parse(input: ParseStream) -> syn::Result<Self> {
ReflectTraits::from_metas(Punctuated::<Meta, Comma>::parse_terminated(input)?, false)
}
}

/// Adds an identifier to a vector of identifiers if it is not already present.
///
/// Returns an error if the identifier already exists in the list.
Expand Down
56 changes: 51 additions & 5 deletions crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use core::fmt;

use crate::container_attributes::{FromReflectAttrs, ReflectTraits, TypePathAttrs};
use crate::field_attributes::{parse_field_attrs, ReflectFieldAttr};
use crate::type_path::parse_path_no_leading_colon;
Expand Down Expand Up @@ -140,10 +142,46 @@ enum ReflectMode {
Value,
}

/// How the macro was invoked.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub(crate) enum ReflectImplSource {
ImplRemoteType,
DeriveLocalType,
}

/// Which trait the macro explicitly implements.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub(crate) enum ReflectTraitToImpl {
Reflect,
FromReflect,
TypePath,
}

/// The provenance of a macro invocation.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub(crate) struct ReflectProvenance {
pub source: ReflectImplSource,
pub trait_: ReflectTraitToImpl,
}

impl fmt::Display for ReflectProvenance {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use self::{ReflectImplSource as S, ReflectTraitToImpl as T};
let str = match (self.source, self.trait_) {
(S::ImplRemoteType, T::Reflect) => "`impl_reflect`",
(S::DeriveLocalType, T::Reflect) => "`#[derive(Reflect)]`",
(S::DeriveLocalType, T::FromReflect) => "`#[derive(FromReflect)]`",
(S::DeriveLocalType, T::TypePath) => "`#[derive(TypePath)]`",
(S::ImplRemoteType, T::FromReflect | T::TypePath) => unreachable!(),
};
f.write_str(str)
}
}

impl<'a> ReflectDerive<'a> {
pub fn from_input(
input: &'a DeriveInput,
is_from_reflect_derive: bool,
provenance: ReflectProvenance,
) -> Result<Self, syn::Error> {
let mut traits = ReflectTraits::default();
// Should indicate whether `#[reflect_value]` was used.
Expand All @@ -167,8 +205,7 @@ impl<'a> ReflectDerive<'a> {
}

reflect_mode = Some(ReflectMode::Normal);
let new_traits =
ReflectTraits::from_meta_list(meta_list, is_from_reflect_derive)?;
let new_traits = ReflectTraits::from_meta_list(meta_list, provenance.trait_)?;
traits.merge(new_traits)?;
}
Meta::List(meta_list) if meta_list.path.is_ident(REFLECT_VALUE_ATTRIBUTE_NAME) => {
Expand All @@ -180,8 +217,7 @@ impl<'a> ReflectDerive<'a> {
}

reflect_mode = Some(ReflectMode::Value);
let new_traits =
ReflectTraits::from_meta_list(meta_list, is_from_reflect_derive)?;
let new_traits = ReflectTraits::from_meta_list(meta_list, provenance.trait_)?;
traits.merge(new_traits)?;
}
Meta::Path(path) if path.is_ident(REFLECT_VALUE_ATTRIBUTE_NAME) => {
Expand Down Expand Up @@ -260,6 +296,16 @@ impl<'a> ReflectDerive<'a> {

let meta = ReflectMeta::new(type_path, traits);

if provenance.source == ReflectImplSource::ImplRemoteType
&& meta.type_path_attrs().should_auto_derive()
&& !meta.type_path().has_custom_path()
{
return Err(syn::Error::new(
meta.type_path().span(),
format!("a #[{TYPE_PATH_ATTRIBUTE_NAME} = \"...\"] attribute must be specified when using {provenance}")
));
}

#[cfg(feature = "documentation")]
let meta = meta.with_docs(doc);

Expand Down
Loading

0 comments on commit df761af

Please sign in to comment.