Repository: Ralith/hecs (rust)

Issue:
Add support for generic types to `#[derive(Bundle)]`
At current, the `Bundle` derive macro doesn't support generics and it seems to me like it should be able to support them without too much trouble.

Pull request:
Support generics in bundle derive macro
Fixes #87

This still does the duplication check, unlike tuples which don't do that. Should that stay or not?

Edit: Maybe the dupe check can be made cheaper by making it lazy and only execute once via AtomicBool and closures?

Patch:
diff --git a/macros/src/lib.rs b/macros/src/lib.rs
index f4dfec4..483aa1f 100644
--- a/macros/src/lib.rs
+++ b/macros/src/lib.rs
@@ -41,12 +41,6 @@ pub fn derive_bundle(input: TokenStream) -> TokenStream {
 
 fn derive_bundle_(input: DeriveInput) -> Result<TokenStream2> {
     let ident = input.ident;
-    if !input.generics.params.is_empty() {
-        return Err(Error::new_spanned(
-            input.generics,
-            "derive(Bundle) does not support generics",
-        ));
-    }
     let data = match input.data {
         syn::Data::Struct(s) => s,
         _ => {
@@ -58,13 +52,13 @@ fn derive_bundle_(input: DeriveInput) -> Result<TokenStream2> {
     };
     let (tys, field_members) = struct_fields(&data.fields);
     let field_idents = member_as_idents(&field_members);
+    let generics = add_additional_bounds_to_generic_params(input.generics);
 
-    let dyn_bundle_code = gen_dynamic_bundle_impl(&ident, &field_members, &tys);
-    let num_tys = tys.len();
-    let bundle_code = if num_tys == 0 {
-        gen_unit_struct_bundle_impl(ident)
+    let dyn_bundle_code = gen_dynamic_bundle_impl(&ident, &generics, &field_members, &tys);
+    let bundle_code = if tys.is_empty() {
+        gen_unit_struct_bundle_impl(ident, &generics)
     } else {
-        gen_bundle_impl(&ident, &field_members, &field_idents, &tys)
+        gen_bundle_impl(&ident, &generics, &field_members, &field_idents, &tys)
     };
     let mut ts = dyn_bundle_code;
     ts.extend(bundle_code);
@@ -73,12 +67,14 @@ fn derive_bundle_(input: DeriveInput) -> Result<TokenStream2> {
 
 fn gen_dynamic_bundle_impl(
     ident: &syn::Ident,
+    generics: &syn::Generics,
     field_members: &[syn::Member],
     tys: &[&syn::Type],
 ) -> TokenStream2 {
+    let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
     quote! {
-        impl ::hecs::DynamicBundle for #ident {
-            fn with_ids<T>(&self, f: impl ::std::ops::FnOnce(&[::std::any::TypeId]) -> T) -> T {
+        impl #impl_generics ::hecs::DynamicBundle for #ident #ty_generics #where_clause {
+            fn with_ids<__hecs__T>(&self, f: impl ::std::ops::FnOnce(&[::std::any::TypeId]) -> __hecs__T) -> __hecs__T {
                 <Self as ::hecs::Bundle>::with_static_ids(f)
             }
 
@@ -99,38 +95,47 @@ fn gen_dynamic_bundle_impl(
 
 fn gen_bundle_impl(
     ident: &syn::Ident,
+    generics: &syn::Generics,
     field_members: &[syn::Member],
     field_idents: &[Cow<syn::Ident>],
     tys: &[&syn::Type],
 ) -> TokenStream2 {
     let num_tys = tys.len();
+    let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
+    let with_static_ids_inner = quote! {
+        {
+            let mut tys = [#((::std::mem::align_of::<#tys>(), ::std::any::TypeId::of::<#tys>())),*];
+            tys.sort_unstable_by(|x, y| {
+                ::std::cmp::Ord::cmp(&x.0, &y.0)
+                    .reverse()
+                    .then(::std::cmp::Ord::cmp(&x.1, &y.1))
+            });
+            let mut ids = [::std::any::TypeId::of::<()>(); #num_tys];
+            for (id, info) in ::std::iter::Iterator::zip(ids.iter_mut(), tys.iter()) {
+                *id = info.1;
+            }
+            ids
+        }
+    };
+    let with_static_ids_body = if generics.params.is_empty() {
+        quote! {
+            ::hecs::lazy_static::lazy_static! {
+                static ref ELEMENTS: [::std::any::TypeId; #num_tys] = {
+                    #with_static_ids_inner
+                };
+            }
+            f(&*ELEMENTS)
+        }
+    } else {
+        quote! {
+            f(&#with_static_ids_inner)
+        }
+    };
     quote! {
-        impl ::hecs::Bundle for #ident {
-            fn with_static_ids<T>(f: impl ::std::ops::FnOnce(&[::std::any::TypeId]) -> T) -> T {
-                ::hecs::lazy_static::lazy_static! {
-                    static ref ELEMENTS: [::std::any::TypeId; #num_tys] = {
-                        let mut dedup = ::std::collections::HashSet::new();
-                        for &(ty, name) in [#((::std::any::TypeId::of::<#tys>(), ::std::any::type_name::<#tys>())),*].iter() {
-                            if !dedup.insert(ty) {
-                                ::std::panic!("{} has multiple {} fields; each type must occur at most once!", stringify!(#ident), name);
-                            }
-                        }
-
-                        let mut tys = [#((::std::mem::align_of::<#tys>(), ::std::any::TypeId::of::<#tys>())),*];
-                        tys.sort_unstable_by(|x, y| {
-                            ::std::cmp::Ord::cmp(&x.0, &y.0)
-                                .reverse()
-                                .then(::std::cmp::Ord::cmp(&x.1, &y.1))
-                        });
-                        let mut ids = [::std::any::TypeId::of::<()>(); #num_tys];
-                        for (id, info) in ::std::iter::Iterator::zip(ids.iter_mut(), tys.iter()) {
-                            *id = info.1;
-                        }
-                        ids
-                    };
-                }
-
-                f(&*ELEMENTS)
+        impl #impl_generics ::hecs::Bundle for #ident #ty_generics #where_clause {
+            #[allow(non_camel_case_types)]
+            fn with_static_ids<__hecs__T>(f: impl ::std::ops::FnOnce(&[::std::any::TypeId]) -> __hecs__T) -> __hecs__T {
+                #with_static_ids_body
             }
 
             fn static_type_info() -> ::std::vec::Vec<::hecs::TypeInfo> {
@@ -155,10 +160,12 @@ fn gen_bundle_impl(
 }
 
 // no reason to generate a static for unit structs
-fn gen_unit_struct_bundle_impl(ident: syn::Ident) -> TokenStream2 {
+fn gen_unit_struct_bundle_impl(ident: syn::Ident, generics: &syn::Generics) -> TokenStream2 {
+    let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
     quote! {
-        impl ::hecs::Bundle for #ident {
-            fn with_static_ids<T>(f: impl ::std::ops::FnOnce(&[::std::any::TypeId]) -> T) -> T { f(&[]) }
+        impl #impl_generics ::hecs::Bundle for #ident #ty_generics #where_clause {
+            #[allow(non_camel_case_types)]
+            fn with_static_ids<__hecs__T>(f: impl ::std::ops::FnOnce(&[::std::any::TypeId]) -> __hecs__T) -> __hecs__T { f(&[]) }
             fn static_type_info() -> ::std::vec::Vec<::hecs::TypeInfo> { ::std::vec::Vec::new() }
 
             unsafe fn get(
@@ -170,6 +177,23 @@ fn gen_unit_struct_bundle_impl(ident: syn::Ident) -> TokenStream2 {
     }
 }
 
+fn make_component_trait_bound() -> syn::TraitBound {
+    syn::TraitBound {
+        paren_token: None,
+        modifier: syn::TraitBoundModifier::None,
+        lifetimes: None,
+        path: syn::parse_quote!(::hecs::Component),
+    }
+}
+
+fn add_additional_bounds_to_generic_params(mut generics: syn::Generics) -> syn::Generics {
+    generics.type_params_mut().for_each(|tp| {
+        tp.bounds
+            .push(syn::TypeParamBound::Trait(make_component_trait_bound()))
+    });
+    generics
+}
+
 fn struct_fields(fields: &syn::Fields) -> (Vec<&syn::Type>, Vec<syn::Member>) {
     match fields {
         syn::Fields::Named(ref fields) => fields
diff --git a/src/archetype.rs b/src/archetype.rs
index ccbdff2..50db000 100644
--- a/src/archetype.rs
+++ b/src/archetype.rs
@@ -43,11 +43,26 @@ pub struct Archetype {
 }
 
 impl Archetype {
+    fn assert_type_info(types: &[TypeInfo]) {
+        types.windows(2).for_each(|x| match x[0].cmp(&x[1]) {
+            core::cmp::Ordering::Less => (),
+            #[cfg(debug_assertions)]
+            core::cmp::Ordering::Equal => panic!(
+                "attempted to create archetype with duplicate components; \
+                    each type must occur at most once! duplicate component type: {}",
+                x[0].type_name
+            ),
+            #[cfg(not(debug_assertions))]
+            core::cmp::Ordering::Equal => panic!(
+                "attempted to create archetype with duplicate components; \
+            each type must occur at most once!"
+            ),
+            core::cmp::Ordering::Greater => panic!("type info is unsorted"),
+        });
+    }
+
     pub(crate) fn new(types: Vec<TypeInfo>) -> Self {
-        debug_assert!(
-            types.windows(2).all(|x| x[0] < x[1]),
-            "type info unsorted or contains duplicates"
-        );
+        Self::assert_type_info(&types);
         Self {
             types,
             state: HashMap::default(),
@@ -405,6 +420,8 @@ pub struct TypeInfo {
     id: TypeId,
     layout: Layout,
     drop: unsafe fn(*mut u8),
+    #[cfg(debug_assertions)]
+    type_name: &'static str,
 }
 
 impl TypeInfo {
@@ -418,6 +435,8 @@ impl TypeInfo {
             id: TypeId::of::<T>(),
             layout: Layout::new::<T>(),
             drop: drop_ptr::<T>,
+            #[cfg(debug_assertions)]
+            type_name: core::any::type_name::<T>(),
         }
     }
 


Test patch:
diff --git a/tests/derive.rs b/tests/derive.rs
index c843459..ca570ce 100644
--- a/tests/derive.rs
+++ b/tests/derive.rs
@@ -12,6 +12,7 @@ fn derive() {
         "tuple_structs.rs",
         "named_structs.rs",
         "no_prelude.rs",
+        "generics.rs",
     ];
     for &passing_test in successes {
         t.pass(&format!("{}/{}", TEST_DIR, passing_test));
diff --git a/tests/derive/generics.rs b/tests/derive/generics.rs
new file mode 100644
index 0000000..f15eccb
--- /dev/null
+++ b/tests/derive/generics.rs
@@ -0,0 +1,21 @@
+use hecs::Bundle;
+
+#[derive(Bundle)]
+struct Foo<T> {
+    foo: T,
+}
+
+#[derive(Bundle)]
+struct Bar<T> {
+    foo: i32,
+    bar: T,
+}
+
+#[derive(Bundle)]
+struct Baz<T, U, V> {
+    foo: T,
+    bar: U,
+    baz: V,
+}
+
+fn main() {}
diff --git a/tests/tests.rs b/tests/tests.rs
index 2660155..c8c615f 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -243,7 +243,18 @@ fn derived_bundle() {
 
 #[test]
 #[cfg(feature = "macros")]
-#[should_panic(expected = "each type must occur at most once")]
+#[cfg_attr(
+    debug_assertions,
+    should_panic(
+        expected = "attempted to create archetype with duplicate components; each type must occur at most once! duplicate component type: i32"
+    )
+)]
+#[cfg_attr(
+    not(debug_assertions),
+    should_panic(
+        expected = r#"attempted to create archetype with duplicate components; each type must occur at most once!"#
+    )
+)]
 fn bad_bundle_derive() {
     #[derive(Bundle)]
     struct Foo {
@@ -368,3 +379,21 @@ fn query_one() {
     world.despawn(a).unwrap();
     assert!(world.query_one::<&i32>(a).is_err());
 }
+
+#[test]
+#[cfg_attr(
+    debug_assertions,
+    should_panic(
+        expected = r#"attempted to create archetype with duplicate components; each type must occur at most once! duplicate component type: f32"#
+    )
+)]
+#[cfg_attr(
+    not(debug_assertions),
+    should_panic(
+        expected = r#"attempted to create archetype with duplicate components; each type must occur at most once!"#
+    )
+)]
+fn duplicate_components_panic() {
+    let mut world = World::new();
+    world.reserve::<(f32, i64, f32)>(1);
+}