feat(schema): support generic bounds in structs & enums

[nelson137]

Changes

• Support generic bounds in struct & enum definitions that use HasSchema derive macro
  • Required for jumpy#942
• Move the static S: OnceLock<parking_lot::RwLock<HashMap<TypeId, &'static Schema>>> for generics into bones_schema so that user crates don't have to take a dependency on parking_lot

Given the following rust code:

use bones_schema::HasSchema;

#[derive(HasSchema, Clone, Default)]
#[repr(C)]
struct HasGenericWithBounds<T: Default>(T);

The HasSchema derive macro used to generate:

#[repr(C)]
#[allow(dead_code)]
struct HasGenericWithBounds<T: Default>(T);

unsafe impl<T: HasSchema + Clone> bones_schema::HasSchema
for HasGenericWithBounds<T: Default> {
//                       ^^^^^^^^^^ error[E0229]: associated item constraints are not allowed here
    fn schema() -> &'static bones_schema::Schema {
        use ::std::sync::OnceLock;
        use ::std::any::TypeId;
        use bones_utils::HashMap;
        use parking_lot::RwLock; // <~~ requires user crate to depend on `parking_lot`
        static S: OnceLock<RwLock<HashMap<TypeId, &'static Schema>>> = OnceLock::new();
//                                                         ^^^^^^ error[E0412]: cannot find type `Schema` in this scope
        let schema = {
            S.get_or_init(Default::default).read().get(&TypeId::of::<Self>()).copied()
        };
        schema
            .unwrap_or_else(|| {
                let schema = bones_schema::registry::SCHEMA_REGISTRY
                    .register(/* ... */);
                S.get_or_init(Default::default)
                    .write()
                    .insert(TypeId::of::<Self>(), schema);
                schema
            })
    }
}

With errors:

error[E0412]: cannot find type `Schema` in this scope
 --> framework_crates/bones_schema/tests/generic_bounds.rs:3:10
  |
3 | #[derive(HasSchema, Clone, Default)]
  |          ^^^^^^^^^ not found in this scope
  |
  = note: this error originates in the derive macro `HasSchema` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0229]: associated item constraints are not allowed here
 --> framework_crates/bones_schema/tests/generic_bounds.rs:6:29
  |
6 | struct HasGenericWithBounds<T: Default>(T);
  |                             ^^^^^^^^^^ associated item constraint not allowed here

Now it generates:

#[repr(C)]
#[allow(dead_code)]
struct HasGenericWithBounds<T: Default>(T);

unsafe impl<T: HasSchema + Clone + Default> bones_schema::HasSchema
//          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ correct bounds
for HasGenericWithBounds<T> {
//                       ^ correct parameters
    fn schema() -> &'static bones_schema::Schema {
        use ::std::any::TypeId;
        let schema = {
            bones_schema::registry::GENERIC_SCHEMA_CACHE // <~~ use shared `HashMap` in `bones_schema` so user crate doesn't have to depend on `parking_lot`
                .read()
                .get(&TypeId::of::<Self>())
                .copied()
        };
        schema
            .unwrap_or_else(|| {
                let schema = bones_schema::registry::SCHEMA_REGISTRY
                    .register(/* ... */);
                bones_schema::registry::GENERIC_SCHEMA_CACHE
                    .write()
                    .insert(TypeId::of::<Self>(), schema);
                schema
            })
    }
}

[zicklag]

Sorry I don't have time to do a proper review, I just want to throw a warning in here because I've messed this up when dealing with generics in the has schema derive: be very careful to make sure that each different instantiation of the generic type's schema returns it's own unique schema.

There was a really tricky to debug issue once where generic enums like Maybe<T> was accidentally returning the schema of the first registered T for the inner types, even though it needed to make sure each different Maybe<T> used a different schema internally.

Sorry that is really hard to say in a way that makes sense.

This is the PR that fixed the error I'm talking about: #456

---

Anyway, warning aside, big thanks for this!

[nelson137]

@zicklag thanks for weighing in. I think I ran into that issue once already 😆

My first push had some failing tests and it looked like it was because of the new shared GENERIC_SCHEMA_CACHE that replaced each variant having its own static HashMap<TypeId, &'static Schema> -- the type ID for Maybe::Set and Maybe::Unset are the same so their schemas were overlapping in the cache. To fix it I added the variant name to the cache key ((TypeId, &'static str)) which I think is ok, since the type ID accounts for generic params and the name differentiates enum variants.

That's the only thing I'm not 100% on, since the rest of the changes are essentially just fixing syntax errors in the derive macro code gen.

[MaxCWhitehead]

lmk if I'm missing anything - but I think we may have dropped the #[schema(no_clone] (i.e. no_clone bool) support here, if this attribute is present, we should not be requiring clone. If reading this right - a no_clone type with generic params will now require clone on its generic types, which previously did not - might not compile with the right instantiation. (we may not have any test coverage or usage that would break in code I'm guessing).

[nelson137]

Really good catch. Fixed, and added a test that will fail to compile if the code gen is broken again.