gdnative icon indicating copy to clipboard operation
gdnative copied to clipboard

Can't Derive ToVariant on generic type with constraint

Open astrale-sharp opened this issue 3 years ago • 10 comments

Hi ! I'm trying to get this to compile :

#[derive(Debug, Clone, ToVariant)]
pub enum WorldChange<T: Entity> {
    EntityMoved {
        id: EntityId,
        from: Position,
        to: Position,
    },
    EntitySentMessage {
        from: EntityId,
        to: EntityId,
        msg: T::Message,
    },
    EntityStateChanged {
        id: EntityId,
        change: T::EntityChange,
    },
    EntityPlaced(EntityId, Position),
    EntityUnplaced(EntityId),
}

but it complains this enum takes 1 generic argument but 0 generic arguments were supplied expected 1 generic argument.

Note that it doesn't complain for MyStruct<T>, but it complains for any constraint MyStruct<T : Debug> or MyStruct<T : ToVariant> it doesn't matter as well if T::Message impl ToVariant or not (which it does)

It would be nice to be able to constrain the type because my only other option is to impl ToVariant by myself which is not ideal.

astrale-sharp avatar Oct 09 '22 10:10 astrale-sharp

Thanks for reporting. I'm not 100% sure how good the support for generics is in the derives.

Did you already have a look with cargo expand to see the code generated by the derive-proc-macro? That often helps pinpoint where exactly the error message occurs.

Bromeon avatar Oct 09 '22 21:10 Bromeon

Okay, I learned a bit I installed nightly and i'm running through that code to try and understand what's up : this :

#[derive(Debug, Clone, ToVariant)]
pub enum WorldChange<T: Entity> {
    EntityMoved {
        id: EntityId,
        from: Position,
        to: Position,
        cost: f32,
    },
    EntitySentMessage {
        from: EntityId,
        to: EntityId,
        msg: T::Message,
    },
    EntityStateChanged {
        id: EntityId,
        change: T::EntityChange,
    },
    EntityPlaced(EntityId, Position),
    EntityUnplaced(EntityId),
    EntityPassedTurn(EntityId),
}

expands the bloc of code at the end of this comment:

It was incredibly useful to pinpoint the bug ! notice that if you replace the first line impl<T: Entity> ::gdnative::core_types::ToVariant for WorldChange<T: Entity> by impl<T: Entity> ::gdnative::core_types::ToVariant for WorldChange<T> There is no more problems ! I've never messed with proc macros yet but that doesn't look so hard (the error was really confusing tho, maybe we should report it to rust-lang cause this enum takes 1 generic argument but 0 generic arguments were supplied expected 1 generic argument is really not the problem here ...

expansion

 impl<T: Entity> ::gdnative::core_types::ToVariant for WorldChange<T: Entity>
    where
        T::Message: ::gdnative::core_types::ToVariant,
        T::EntityChange: ::gdnative::core_types::ToVariant,
    {
        fn to_variant(&self) -> ::gdnative::core_types::Variant {
            use ::gdnative::core_types::ToVariant;
            use ::gdnative::core_types::FromVariant;
            match &self {
                WorldChange::EntityMoved { id, from, to, cost } => {
                    let __dict = ::gdnative::core_types::Dictionary::new();
                    let __key = ::gdnative::core_types::ToVariant::to_variant(
                        &::gdnative::core_types::GodotString::from("EntityMoved"),
                    );
                    let __value = {
                        let __dict = ::gdnative::core_types::Dictionary::new();
                        {
                            let __key = ::gdnative::core_types::GodotString::from("id")
                                .to_variant();
                            __dict.insert(&__key, &(id).to_variant());
                        }
                        {
                            let __key = ::gdnative::core_types::GodotString::from("from")
                                .to_variant();
                            __dict.insert(&__key, &(from).to_variant());
                        }
                        {
                            let __key = ::gdnative::core_types::GodotString::from("to")
                                .to_variant();
                            __dict.insert(&__key, &(to).to_variant());
                        }
                        {
                            let __key = ::gdnative::core_types::GodotString::from("cost")
                                .to_variant();
                            __dict.insert(&__key, &(cost).to_variant());
                        }
                        __dict.into_shared().to_variant()
                    };
                    __dict.insert(&__key, &__value);
                    ::gdnative::core_types::ToVariant::to_variant(&__dict.into_shared())
                }
                WorldChange::EntitySentMessage { from, to, msg } => {
                    let __dict = ::gdnative::core_types::Dictionary::new();
                    let __key = ::gdnative::core_types::ToVariant::to_variant(
                        &::gdnative::core_types::GodotString::from("EntitySentMessage"),
                    );
                    let __value = {
                        let __dict = ::gdnative::core_types::Dictionary::new();
                        {
                            let __key = ::gdnative::core_types::GodotString::from("from")
                                .to_variant();
                            __dict.insert(&__key, &(from).to_variant());
                        }
                        {
                            let __key = ::gdnative::core_types::GodotString::from("to")
                                .to_variant();
                            __dict.insert(&__key, &(to).to_variant());
                        }
                        {
                            let __key = ::gdnative::core_types::GodotString::from("msg")
                                .to_variant();
                            __dict.insert(&__key, &(msg).to_variant());
                        }
                        __dict.into_shared().to_variant()
                    };
                    __dict.insert(&__key, &__value);
                    ::gdnative::core_types::ToVariant::to_variant(&__dict.into_shared())
                }
                WorldChange::EntityStateChanged { id, change } => {
                    let __dict = ::gdnative::core_types::Dictionary::new();
                    let __key = ::gdnative::core_types::ToVariant::to_variant(
                        &::gdnative::core_types::GodotString::from("EntityStateChanged"),
                    );
                    let __value = {
                        let __dict = ::gdnative::core_types::Dictionary::new();
                        {
                            let __key = ::gdnative::core_types::GodotString::from("id")
                                .to_variant();
                            __dict.insert(&__key, &(id).to_variant());
                        }
                        {
                            let __key = ::gdnative::core_types::GodotString::from(
                                    "change",
                                )
                                .to_variant();
                            __dict.insert(&__key, &(change).to_variant());
                        }
                        __dict.into_shared().to_variant()
                    };
                    __dict.insert(&__key, &__value);
                    ::gdnative::core_types::ToVariant::to_variant(&__dict.into_shared())
                }
                WorldChange::EntityPlaced(__field_0, __field_1) => {
                    let __dict = ::gdnative::core_types::Dictionary::new();
                    let __key = ::gdnative::core_types::ToVariant::to_variant(
                        &::gdnative::core_types::GodotString::from("EntityPlaced"),
                    );
                    let __value = {
                        let __array = ::gdnative::core_types::VariantArray::new();
                        __array.push(&(__field_0).to_variant());
                        __array.push(&(__field_1).to_variant());
                        __array.into_shared().to_variant()
                    };
                    __dict.insert(&__key, &__value);
                    ::gdnative::core_types::ToVariant::to_variant(&__dict.into_shared())
                }
                WorldChange::EntityUnplaced(__field_0) => {
                    let __dict = ::gdnative::core_types::Dictionary::new();
                    let __key = ::gdnative::core_types::ToVariant::to_variant(
                        &::gdnative::core_types::GodotString::from("EntityUnplaced"),
                    );
                    let __value = (__field_0).to_variant();
                    __dict.insert(&__key, &__value);
                    ::gdnative::core_types::ToVariant::to_variant(&__dict.into_shared())
                }
                WorldChange::EntityPassedTurn(__field_0) => {
                    let __dict = ::gdnative::core_types::Dictionary::new();
                    let __key = ::gdnative::core_types::ToVariant::to_variant(
                        &::gdnative::core_types::GodotString::from("EntityPassedTurn"),
                    );
                    let __value = (__field_0).to_variant();
                    __dict.insert(&__key, &__value);
                    ::gdnative::core_types::ToVariant::to_variant(&__dict.into_shared())
                }
            }
        }
    }

astrale-sharp avatar Oct 12 '22 11:10 astrale-sharp

Thanks for expanding, that helped me narrow down the problem!

A fix is available in #961, could you see if it works for you? You can add a Git dependency in Cargo.toml with a branch name.

Bromeon avatar Oct 12 '22 21:10 Bromeon

Amazing thanks ! No it doesn't work at all ! But the errors i get are very very weird !

cargo expand generate code that impl ToVariant Correctly but #[method] now complains that all the struct deriving ToVariant don't satisfy the trait bound , I'm extremly puzzled to be honest ... (The implementation of the trait is there !)

What's extremely odd is that your test seems to be passing and i dont do anything differently, maybe i messed it up in Cargo.toml ? I used : gdnative = {git = "https://github.com/Bromeon/godot-rust.git", branch = "bugfix/to-variant-bounds"} EDIT: or maybe it's not compatible with 0.11.0 which i was using until now but that would be surprising EDIT 2 : I've been investigating and the bug(?) seems to be there before the fix (which is reassuring because the fix only slightly modified the derive macro), let me check some more stuff before reporting

astrale-sharp avatar Oct 16 '22 06:10 astrale-sharp

I found it ! Oh my it was subtle (for me)!! I temporarily had to add #![feature(associated_type_bounds)] but it is not needed once i fixed the impl

Okay so a struct like this

#[derive(Debug, Clone,ToVariant)]
pub struct Intent<T: Entity> {
    pub emitter: EntityId,
    pub action: Action<T>,
    pub priority: i32,
}

has a generated cargo expand like this

impl<T: Entity> ::gdnative::core_types::ToVariant for Intent<T>
 where
     T: ::gdnative::core_types::ToVariant,
{
    fn to_variant(&self) -> ::gdnative::core_types::Variant {
        use ::gdnative::core_types::FromVariant;
        use ::gdnative::core_types::ToVariant;
        {
            let Intent {
                emitter,
                action,
                priority,
            } = self;
            {
                let __dict = ::gdnative::core_types::Dictionary::new();
                {
                    let __key = ::gdnative::core_types::GodotString::from("emitter").to_variant();
                    __dict.insert(
                        &__key,
                        &(emitter).to_variant(),
                    );
                }
                {
                    let __key = ::gdnative::core_types::GodotString::from("action").to_variant();
                    __dict.insert(
                        &__key,
                        &(action).to_variant(),
                    );
                }
                {
                    let __key = ::gdnative::core_types::GodotString::from("priority").to_variant();
                    __dict.insert(
                        &__key,
                        &(priority).to_variant(),
                    );
                }
                __dict.into_shared().to_variant()
            }
        }
    }
}

Note that where T: ::gdnative::core_types::ToVariant, is not needed (and not implemented so that's why it wouldn't work with me) commenting out these line suffice as a fix for me, so i'm not stuck Fixing the macros probably won't be easy tho because we would need to know if we need T to impl ToVariant and that doesn't seem trivial to me?

Edit : maybe it wouldnt be so hard, the user would just have to write

#[derive(ToVariant)]
MyStruct<T : ToVariant> /* ... */

if needed, but i guess we'd have to see if the error msg is nice, maybe leave that to me i'd like to help and try to improve the macros (plus i have a code base to test it right after so) Is that okay if I try to help?

astrale-sharp avatar Oct 16 '22 07:10 astrale-sharp

Yeah, gladly! 😊

Maybe I'll merge the existing PR but keep this issue open; then you can use it as a base to address the where problem (in a separate PR). Would be good if you could cover it in tests as well!

Bromeon avatar Oct 16 '22 08:10 Bromeon

Hey, I'm having trouble launching the tests in test_derive.rs with cargo test and other cargo test stuff i tried (-p, etc) A little help? ^^ I found ./check.sh itest but it fails with > cp target/debug/gdnative_test* test/project/lib/ cp: impossible d'évaluer 'target/debug/gdnative_test*': Aucun fichier ou dossier de ce type

astrale-sharp avatar Oct 16 '22 18:10 astrale-sharp

Did you run sh check.sh itest from the repository root?

It should create some dynamic library files (for example .dll or .so) in target/debug/, can you check they are there?

Otherwise feel free to use the CI for testing, just amend your commits and keep force-pushing (git push --force-with-lease) 🙂

Bromeon avatar Oct 17 '22 17:10 Bromeon

Hey, I'm having trouble launching the tests in test_derive.rs with cargo test and other cargo test stuff i tried (-p, etc) A little help? ^^ I found ./check.sh itest but it fails with > cp target/debug/gdnative_test* test/project/lib/ cp: impossible d'évaluer 'target/debug/gdnative_test*': Aucun fichier ou dossier de ce type

Maybe related to #965

Bogay avatar Oct 18 '22 17:10 Bogay

Fixing the macros probably won't be easy tho because we would need to know if we need T to impl ToVariant and that doesn't seem trivial to me?

This is correct. Macros operate only at the token level, so any automatic bounds are at most guesses. serde handles this problem by allowing users to override any automatic bounds with a item-level attribute parameter.

Now that we have ItemAttr we can probably do this as well, perhaps with a similar syntax.

chitoyuu avatar Jan 09 '23 10:01 chitoyuu