Skip to content

Commit

Permalink
Make proc macros hygienic in bevy_reflect_derive (#6752)
Browse files Browse the repository at this point in the history
# Objective

- Fixes #3004 

## Solution

- Replaced all the types with their fully quallified names
- Replaced all trait methods and inherent methods on dyn traits with their fully qualified names
- Made a new file `fq_std.rs` that contains structs corresponding to commonly used Structs and Traits from `std`. These structs are replaced by their respective fully qualified names when used inside `quote!`
  • Loading branch information
elbertronnie committed Dec 5, 2022
1 parent 9f0c41f commit f9c52f9
Show file tree
Hide file tree
Showing 14 changed files with 260 additions and 165 deletions.
35 changes: 18 additions & 17 deletions crates/bevy_reflect/bevy_reflect_derive/src/container_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
//! the derive helper attribute for `Reflect`, which looks like:
//! `#[reflect(PartialEq, Default, ...)]` and `#[reflect_value(PartialEq, Default, ...)]`.

use crate::fq_std::{FQAny, FQDefault, FQOption};
use crate::utility;
use proc_macro2::{Ident, Span};
use quote::quote_spanned;
Expand Down Expand Up @@ -222,17 +223,17 @@ impl ReflectTraits {
pub fn get_hash_impl(&self, bevy_reflect_path: &Path) -> Option<proc_macro2::TokenStream> {
match &self.hash {
&TraitImpl::Implemented(span) => Some(quote_spanned! {span=>
fn reflect_hash(&self) -> Option<u64> {
use std::hash::{Hash, Hasher};
let mut hasher = #bevy_reflect_path::ReflectHasher::default();
Hash::hash(&std::any::Any::type_id(self), &mut hasher);
fn reflect_hash(&self) -> #FQOption<u64> {
use ::core::hash::{Hash, Hasher};
let mut hasher: #bevy_reflect_path::ReflectHasher = #FQDefault::default();
Hash::hash(&#FQAny::type_id(self), &mut hasher);
Hash::hash(self, &mut hasher);
Some(hasher.finish())
#FQOption::Some(Hasher::finish(&hasher))
}
}),
&TraitImpl::Custom(ref impl_fn, span) => Some(quote_spanned! {span=>
fn reflect_hash(&self) -> Option<u64> {
Some(#impl_fn(self))
fn reflect_hash(&self) -> #FQOption<u64> {
#FQOption::Some(#impl_fn(self))
}
}),
TraitImpl::NotImplemented => None,
Expand All @@ -248,18 +249,18 @@ impl ReflectTraits {
) -> Option<proc_macro2::TokenStream> {
match &self.partial_eq {
&TraitImpl::Implemented(span) => Some(quote_spanned! {span=>
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
let value = value.as_any();
if let Some(value) = value.downcast_ref::<Self>() {
Some(std::cmp::PartialEq::eq(self, value))
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> #FQOption<bool> {
let value = <dyn #bevy_reflect_path::Reflect>::as_any(value);
if let #FQOption::Some(value) = <dyn #FQAny>::downcast_ref::<Self>(value) {
#FQOption::Some(::core::cmp::PartialEq::eq(self, value))
} else {
Some(false)
#FQOption::Some(false)
}
}
}),
&TraitImpl::Custom(ref impl_fn, span) => Some(quote_spanned! {span=>
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> Option<bool> {
Some(#impl_fn(self, value))
fn reflect_partial_eq(&self, value: &dyn #bevy_reflect_path::Reflect) -> #FQOption<bool> {
#FQOption::Some(#impl_fn(self, value))
}
}),
TraitImpl::NotImplemented => None,
Expand All @@ -272,12 +273,12 @@ impl ReflectTraits {
pub fn get_debug_impl(&self) -> Option<proc_macro2::TokenStream> {
match &self.debug {
&TraitImpl::Implemented(span) => Some(quote_spanned! {span=>
fn debug(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Debug::fmt(self, f)
fn debug(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
::core::fmt::Debug::fmt(self, f)
}
}),
&TraitImpl::Custom(ref impl_fn, span) => Some(quote_spanned! {span=>
fn debug(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn debug(&self, f: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
#impl_fn(self, f)
}
}),
Expand Down
5 changes: 3 additions & 2 deletions crates/bevy_reflect/bevy_reflect_derive/src/documentation.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Contains code related to documentation reflection (requires the `documentation` feature).

use crate::fq_std::FQOption;
use proc_macro2::TokenStream;
use quote::{quote, ToTokens};
use syn::{Attribute, Lit, Meta};
Expand Down Expand Up @@ -69,9 +70,9 @@ impl Documentation {
impl ToTokens for Documentation {
fn to_tokens(&self, tokens: &mut TokenStream) {
if let Some(doc) = self.doc_string() {
quote!(Some(#doc)).to_tokens(tokens);
quote!(#FQOption::Some(#doc)).to_tokens(tokens);
} else {
quote!(None).to_tokens(tokens);
quote!(#FQOption::None).to_tokens(tokens);
}
}
}
3 changes: 2 additions & 1 deletion crates/bevy_reflect/bevy_reflect_derive/src/enum_utility.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::fq_std::FQDefault;
use crate::{
derive_data::{EnumVariantFields, ReflectEnum},
utility::ident_or_index,
Expand Down Expand Up @@ -39,7 +40,7 @@ pub(crate) fn get_variant_constructors(
let constructor_fields = fields.iter().enumerate().map(|(declar_index, field)| {
let field_ident = ident_or_index(field.data.ident.as_ref(), declar_index);
let field_value = if field.attrs.ignore.is_ignored() {
quote! { Default::default() }
quote! { #FQDefault::default() }
} else {
let error_repr = field.data.ident.as_ref().map_or_else(
|| format!("at index {reflect_index}"),
Expand Down
77 changes: 77 additions & 0 deletions crates/bevy_reflect/bevy_reflect_derive/src/fq_std.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//! This module contains unit structs that should be used inside `quote!` and `spanned_quote!` using the variable interpolation syntax in place of their equivalent structs and traits present in `std`.
//
//! To create hygienic proc macros, all the names must be its fully qualified form. These unit structs help us to not specify the fully qualified name every single time.
//!
//! # Example
//! Instead of writing this:
//! ```ignore
//! quote!(
//! fn get_id() -> Option<i32> {
//! Some(0)
//! }
//! )
//! ```
//! Or this:
//! ```ignore
//! quote!(
//! fn get_id() -> ::core::option::Option<i32> {
//! ::core::option::Option::Some(0)
//! }
//! )
//! ```
//! We should write this:
//! ```ignore
//! use crate::fq_std::FQOption;
//!
//! quote!(
//! fn get_id() -> #FQOption<i32> {
//! #FQOption::Some(0)
//! }
//! )
//! ```

use proc_macro2::TokenStream;
use quote::{quote, ToTokens};

pub(crate) struct FQAny;
pub(crate) struct FQBox;
pub(crate) struct FQClone;
pub(crate) struct FQDefault;
pub(crate) struct FQOption;
pub(crate) struct FQResult;

impl ToTokens for FQAny {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::core::any::Any).to_tokens(tokens);
}
}

impl ToTokens for FQBox {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::std::boxed::Box).to_tokens(tokens);
}
}

impl ToTokens for FQClone {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::core::clone::Clone).to_tokens(tokens);
}
}

impl ToTokens for FQDefault {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::core::default::Default).to_tokens(tokens);
}
}

impl ToTokens for FQOption {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::core::option::Option).to_tokens(tokens);
}
}

impl ToTokens for FQResult {
fn to_tokens(&self, tokens: &mut TokenStream) {
quote!(::core::result::Result).to_tokens(tokens);
}
}
47 changes: 26 additions & 21 deletions crates/bevy_reflect/bevy_reflect_derive/src/from_reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ use crate::container_attributes::REFLECT_DEFAULT;
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::{ReflectMeta, ReflectStruct};
use proc_macro::TokenStream;
use proc_macro2::Span;
use quote::quote;
use quote::{quote, ToTokens};
use syn::{Field, Ident, Index, Lit, LitInt, LitStr, Member};

/// Implements `FromReflect` for the given struct
Expand All @@ -25,15 +26,17 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream {
let (impl_generics, ty_generics, where_clause) = meta.generics().split_for_impl();
TokenStream::from(quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_clause {
fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> Option<Self> {
Some(reflect.as_any().downcast_ref::<#type_name #ty_generics>()?.clone())
fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQOption<Self> {
#FQOption::Some(#FQClone::clone(<dyn #FQAny>::downcast_ref::<#type_name #ty_generics>(<dyn #bevy_reflect_path::Reflect>::as_any(reflect))?))
}
}
})
}

/// Implements `FromReflect` for the given enum type
pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
let fqoption = FQOption.into_token_stream();

let type_name = reflect_enum.meta().type_name();
let bevy_reflect_path = reflect_enum.meta().bevy_reflect_path();

Expand All @@ -47,14 +50,14 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream {
reflect_enum.meta().generics().split_for_impl();
TokenStream::from(quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #type_name #ty_generics #where_clause {
fn from_reflect(#ref_value: &dyn #bevy_reflect_path::Reflect) -> Option<Self> {
if let #bevy_reflect_path::ReflectRef::Enum(#ref_value) = #ref_value.reflect_ref() {
match #ref_value.variant_name() {
#(#variant_names => Some(#variant_constructors),)*
name => panic!("variant with name `{}` does not exist on enum `{}`", name, std::any::type_name::<Self>()),
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) {
#(#variant_names => #fqoption::Some(#variant_constructors),)*
name => panic!("variant with name `{}` does not exist on enum `{}`", name, ::core::any::type_name::<Self>()),
}
} else {
None
#FQOption::None
}
}
}
Expand All @@ -72,6 +75,8 @@ impl MemberValuePair {
}

fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> TokenStream {
let fqoption = FQOption.into_token_stream();

let struct_name = reflect_struct.meta().type_name();
let generics = reflect_struct.meta().generics();
let bevy_reflect_path = reflect_struct.meta().bevy_reflect_path();
Expand All @@ -89,21 +94,21 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token

let constructor = if reflect_struct.meta().traits().contains(REFLECT_DEFAULT) {
quote!(
let mut __this = Self::default();
let mut __this: Self = #FQDefault::default();
#(
if let Some(__field) = #active_values() {
if let #fqoption::Some(__field) = #active_values() {
// Iff field exists -> use its value
__this.#active_members = __field;
}
)*
Some(__this)
#FQOption::Some(__this)
)
} else {
let MemberValuePair(ignored_members, ignored_values) =
get_ignored_fields(reflect_struct, is_tuple);

quote!(
Some(
#FQOption::Some(
Self {
#(#active_members: #active_values()?,)*
#(#ignored_members: #ignored_values,)*
Expand All @@ -129,11 +134,11 @@ fn impl_struct_internal(reflect_struct: &ReflectStruct, is_tuple: bool) -> Token
TokenStream::from(quote! {
impl #impl_generics #bevy_reflect_path::FromReflect for #struct_name #ty_generics #where_from_reflect_clause
{
fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> Option<Self> {
if let #bevy_reflect_path::ReflectRef::#ref_struct_type(#ref_struct) = reflect.reflect_ref() {
fn from_reflect(reflect: &dyn #bevy_reflect_path::Reflect) -> #FQOption<Self> {
if let #bevy_reflect_path::ReflectRef::#ref_struct_type(#ref_struct) = #bevy_reflect_path::Reflect::reflect_ref(reflect) {
#constructor
} else {
None
#FQOption::None
}
}
}
Expand All @@ -153,7 +158,7 @@ fn get_ignored_fields(reflect_struct: &ReflectStruct, is_tuple: bool) -> MemberV

let value = match &field.attrs.default {
DefaultBehavior::Func(path) => quote! {#path()},
_ => quote! {Default::default()},
_ => quote! {#FQDefault::default()},
};

(member, value)
Expand Down Expand Up @@ -189,19 +194,19 @@ fn get_active_fields(
let value = match &field.attrs.default {
DefaultBehavior::Func(path) => quote! {
(||
if let Some(field) = #get_field {
if let #FQOption::Some(field) = #get_field {
<#ty as #bevy_reflect_path::FromReflect>::from_reflect(field)
} else {
Some(#path())
#FQOption::Some(#path())
}
)
},
DefaultBehavior::Default => quote! {
(||
if let Some(field) = #get_field {
if let #FQOption::Some(field) = #get_field {
<#ty as #bevy_reflect_path::FromReflect>::from_reflect(field)
} else {
Some(Default::default())
#FQOption::Some(#FQDefault::default())
}
)
},
Expand Down
Loading

0 comments on commit f9c52f9

Please sign in to comment.