hobofan / ambassador

Delegation of trait implementations via procedural macros
Apache License 2.0
251 stars 13 forks source link

Ambassador ambassador_impl_foo not accessable when in different package #45

Closed potts2020 closed 3 months ago

potts2020 commented 1 year ago

First time submitting an issue, sorry if the formatting is not standard.

Trying to delegate a trait to Foo in a separate package, but this causes a "Not Accessible" Error. See the following Screenshot for more details:

The following screenshot shows the trait Channel that delegates to NodeType (same package). While the use-modifiable Custom enum is in the testing package. Ambassador does work when Custom is instantiated in the same package as Channel, however since the entire point of my use of Ambassador is to allow users to add their own enum types - I must get this working.

Please let me know if this is a current limitation of Ambassador, and if there is anything I can do to bypass this restriction.

image

image

image

dewert99 commented 8 months ago

Sorry it took so long to respond.

Using the current version of ambassador (0.3.5) it should be possible to do this:

mod m1 {

    #[ambassador::delegatable_trait]
    pub trait Channel {
        type Output;

        fn transmit(&self) -> Self::Output;

        fn receive(&mut self, r: Self::Output);
    }
    pub use ambassador_impl_Channel; // This is currently necessary to allow access in other modules
    // This should be done automatically in future versions
}

mod m2 {
    use crate::m1::Channel;
    use crate::m1::ambassador_impl_Channel; // This allows delegating to `Channel` in this module

    struct Local1;

    impl Channel for Local1 {
        type Output = ();
        fn transmit(&self) -> Self::Output {}
        fn receive(&mut self, _: Self::Output) {}
    }

    struct Local2;

    impl Channel for Local2 {
        type Output = ();
        fn transmit(&self) -> Self::Output {}
        fn receive(&mut self, _: Self::Output) {}
    }

    #[derive(ambassador::Delegate)]
    #[delegate(Channel)]
    enum Custom {
        Local1(Local1),
        Local2(Local2)
    }
}
odisseylm commented 3 months ago

Thank you very much for your answer (and for crate at all)! It would be nice to add it to documentation ))

(I also tried to use 'pub use' by myself but probably put it in wrong place and it didn't work :-( )

dewert99 commented 3 months ago

I've been trying to think of how to improve this for future versions, the easiest thing to would be to have #[delegatable_trait] automatically add the pub use ambassador_impl_<Trait>; statement and add documentation about requiring use path::ambassador_impl_<Trait>; when using derive(Delegate). This second use statement is harder to automate since derive(Delegate) doesn't know what module contained the relevant #[delegatable_trait]. One idea I had was to rename the generated macro from ambassador_impl_<Trait> to just <Trait> so it would be imported at the same time as the trait itself. This would work well for #[delegatable_trait] but cause a name collision when trying to create a derive macro for a trait that is also using #[delegatable_trait] . It would also not for #[delegatable_trait_remote] since then the generated macro would not be on the same path. I would probably need to add a macro arg to allow the macro path to be overridden in these cases. Eg:

mod m1 {
    #[ambassador::delegatable_trait_remote]
    pub trait Debug {
        fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error>;
    }
}

mod m2 {
    use core::fmt::Debug;

    #[derive(ambassador::Delegate)]
    #[delegate(Debug, macro="crate::m1::Debug")]
    struct Wrap<X>(X);
}

Do these ideas seem like a good way of handling things?

(@vigna You had similar issue)

dewert99 commented 3 months ago

I'm starting to think that requiring users of delegable traits add use path::ambassador_impl_<Trait>; is a better solution since it works more uniformly.

dewert99 commented 3 months ago

Another advantage of keeping ambassador_impl_<Trait> as the macro name and requiring it is imported is that it allows there to also be a module named ambassador_impl_<Trait> that gets imported at the same time and contains helper macros. This seems necessary in order to implement #54 without exponential blowup.

vigna commented 3 months ago

I'm not against the idea: I'm using it now and it works very well. I'm just saying this should go into thevspecification so there's commitment to the syntax.

Any suggestions for my other issue (delegating traitsv with a specific parameter value)?