Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto Impl trait for struct Newtype(Arc<dyn Trait + Send + Sync + 'static>); etc.? #14

Closed
dpc opened this issue Jun 15, 2022 · 15 comments · Fixed by #26
Closed

Auto Impl trait for struct Newtype(Arc<dyn Trait + Send + Sync + 'static>); etc.? #14

dpc opened this issue Jun 15, 2022 · 15 comments · Fixed by #26

Comments

@dpc
Copy link

dpc commented Jun 15, 2022

I'm researching best approach for writing business app code like typically done in JVM languages etc. I'm not a fan of OOP, and I really dislike SpringBoot style automagic DI. However abstracting away external components behind an interface, and constructor-passing DI are really great for larger apps.

In the past when doing trait as an interface for multiple implementations for DI, I would go for type SharedConnection = Arc<dyn Connection + Send + Sync + 'static>) kind of thing, and I'm happy to finally find a crate that can get rid of some of that boilerplate.

However not long ago, I've read https://jmmv.dev/2022/04/rust-traits-and-dependency-injection.html#good-solution-newtype, and I think a NewType is even better, it's just I never wanted to put an effort writting out all that stuff.

Would supporting this pattern fall under the goals of this crate?

Going even further, I wonder if supporting things like struct Newtype(Arc<RwLock<dyn Trait + Send + Sync + 'static>>) would be possible.

@dhardy
Copy link
Contributor

dhardy commented Jun 18, 2022

Interesting; the new type pattern can indeed be useful. I guess what you're asking for is automatic generation of trait methods in the new type?

The first problem to solve is syntax. The first solution I can think is this:

#[newtype(pub struct Connection(Arc<Db + …>))]
trait dB { … }

This is not great, especially since there's no way to use #[derive] on the struct. Possibly instead using a macro to define the trait from the new type would work, but I'm skeptical (needs more thought).

Would you be interested in sponsoring development? In any case I'm currently on holiday, so it'll be at least two weeks before I can get to this.

@dpc
Copy link
Author

dpc commented Jun 19, 2022

I guess what you're asking for is automatic generation of trait methods in the new type?

I am not even sure what am I asking. In essence I'm just looking for projects and methods that would make writing business software in Rust easier and faster. IMO, good support for constructor-based DI would be an important piece of it.

Would you be interested in sponsoring development? In any case I'm currently on holiday, so it'll be at least two weeks before I can get to this.

Unfortunately I haven nothing to offer. I'm just a cheap-o OSS dev relying on free stuff, and sometimes providing my own free stuff. 🤷 I also have no experience with implementing any more complex Rust macros.

All I wanted is to point out this use-case and see if it would fit under this crate's umbrella.

@dhardy
Copy link
Contributor

dhardy commented Jun 20, 2022

I am not even sure what am I asking.

Can I suggest you think about this further, in that case? I am not a fan of thinking in paradigms. OO, DI, RAII, constraints, etc all have some utility but are insufficient on their own.

@dhardy
Copy link
Contributor

dhardy commented Jun 20, 2022

I think your request may overlap with another feature this library halfway supports: using 'impl_scope + impl_default' to produce and instantiate a new object implementing some traits.

In many applications the type name of this object is irrelevant and only a single instance is constructed. KAS has a macro called make_widget (which in the master branch is built over this library) to do exactly this. Unfortunately it's somewhat specific to widgets, due to certain limitations regarding types in Rust.

See also:

@dpc
Copy link
Author

dpc commented Jul 30, 2022

Can I suggest you think about this further, in that case? I am not a fan of thinking in paradigms. OO, DI, RAII, constraints, etc all have some utility but are insufficient on their own.

I guess I'm looking for something like this:

struct Email;

#[autoimpl(IFoo using self.0)]
struct OwnedEmailService(Box<EmailService>);

#[autoimpl(for<T: trait> Box<T>)]
trait EmailService {
    fn send_email(&mut self, email: &Email);
}

which currently complains error: unsupported trai

and ideally maybe as compact as:

#[autoimpl(for<T: trait + Send + Sync> struct SharedEmailService(Arc<dyn T>))]
trait EmailService {
    fn send_email(&self, email: &Email);
}

or

#[autoimpl(for<T: trait> struct OwnedmailService(Arc<dyn T>))]
trait EmailService {
    fn send_email(&mut self, email: &Email);
}

dependening on who is responsible for thread-safety.

If take to extreme, since this is supposed to be used commonly in bussiness code to support D.I.:

#[di(OwnedmailService, Box)]
trait EmailService {
    fn send_email(&mut self, email: &Email);
}

even, though I know I'm now stretching it, just to save some keystrokes.

This is specifically to abstract away the implementation of EmailService and allow things like dependency injection, etc. The newtype idea is inspired by https://jmmv.dev/2022/04/rust-traits-and-dependency-injection.html#good-solution-newtype

but other than this I used this a lot in my own code, by making type OwnedSomething = Box<dyn Something>; and making the Box<dyn T> implementation manually (which I think I can now do with #[autoimpl]

@dhardy
Copy link
Contributor

dhardy commented Aug 2, 2022

So, from your first code, I presume you put the wrong trait name:

#[autoimpl(EmailService using self.0)]
struct OwnedEmailService(Box<EmailService>);

This is very close to #[autoimpl(Deref using self.0)] — but with a crucial difference: Deref is a known trait. There's a fundamental problem supporting this syntax for unknown traits: the macro has no way of knowing what methods to generate.

#[autoimpl(for<T: trait + Send + Sync> struct SharedEmailService(Arc<dyn T>))]
trait EmailService {
    fn send_email(&self, email: &Email);
}

This works around the problem: the trait is now part of the macro input, hence this could be supported. It's clunky syntax (for<..> struct Foo(..)), and it's not obvious how well it would work with non-tuple structs or structs with template parameters. Maybe we can find a nicer syntax?

#[di(OwnedmailService, Box)]
trait EmailService {
    fn send_email(&mut self, email: &Email);
}

I'm not sure how to read this one — that di knows that an identifier starting Owned should generate a struct like OwnedFoo(Rc<dyn T>), and that Box expands to Box<dyn T>?

Certainly this is neater syntax, but very "magical": hard to understand how it works without a manual (although to some extend this is true of the above macros too).

Conclusion

Something like this is possible, but we should think about syntax some more.

The middle option is the most viable, but leaves a few questions unanswered:

  • Do we support pub qualifiers in the macro parameters? Or perhaps simply copy this from the trait.
  • Do we support custom doc or simply auto-generate documentation with a link back to the trait?
  • Do we support custom methods on the struct? Do we auto-generate any methods?
  • What about trait impls for the struct: do we just implement a few like Deref, Debug, Eq and From automatically and leave it at that? This is easier in the templated case since the trait impls will be valid code regardless of whether the type supports the trait (e.g. impl<T: Eq> Eq for ...).
  • What about template parameters?
#[autoimpl(for<T: trait> &T, &mut T, Box<T>)]
#[autoimpl(struct SharedEmailService<T: trait + Send + Sync>(pub Arc<T>))]
trait EmailService {
    fn send_email(&self, email: &Email);
}

// or just a non-parameterised version:
#[autoimpl(struct SharedEmailService(pub Arc<dyn trait>))]
trait EmailService { .. }

@dpc
Copy link
Author

dpc commented Aug 2, 2022

I'm not sure how to read this one — that di knows that an identifier starting Owned should generate a struct like OwnedFoo(Rc<dyn T>), and that Box expands to Box<dyn T>?

Rc? di knows to generated struct OwnedService(...) as a newtype wrapper and the inside put Box<dyn Trait>. The type to use in the newtype is given as an argument, the name is not interpreted.

Other than this agreed on everything, AFAICT.

@dhardy
Copy link
Contributor

dhardy commented Aug 2, 2022

Oh, then I completely mis-read di. Sorry, not a fan of that particular proposal at all.

@dhardy
Copy link
Contributor

dhardy commented Oct 6, 2022

Here's another couple of possibilities. Yes, they aren't completely minimal. Thoughts?

Autoimpl on trait definitions with using syntax

#[autoimpl(for<T: trait> &T, &mut T, Box<T>)]
// Here, support `using self.foo` syntax:
#[autoimpl(for<T: trait> SharedEmailService<T> using self.0)]
pub trait EmailService {
    fn send_email(&self, email: &Email);
}

pub struct SharedEmailService<T: EmailService + Send + Sync>(pub Arc<T>);

Autoimpl on type definitions with custom generic parameters

// Since SharedEmailService<T> supports Deref to some T: trait, we can implement just like for Box<T>
#[autoimpl(for<T: trait> &T, &mut T, Box<T>, SharedEmailService<T>)]
pub trait EmailService {
    fn send_email(&self, email: &Email);
}

// Here, we support optional parameter `Deref<Target = ...>`
// (if not specified, this defaults to the field type, which would be Arc<T> here)
#[autoimpl(Deref<Target = T> using self.0)]
pub struct SharedEmailService<T: EmailService + Send + Sync>(pub Arc<T>);

@dpc
Copy link
Author

dpc commented Oct 6, 2022

BTW. In a project I have an opportunity to try this pattern (which we call "dyn newtype"), we went with a macro_rules macro, example usage.

This is what I wish I could achieve without having to maintain my own macro.

@dhardy
Copy link
Contributor

dhardy commented Oct 13, 2022

I had a look at your macro: basically, implement Deref and From. Using #[autoimpl(Deref<Target = ...> using self.0)] could work (if I add support for custom Target), but is repetitive. How to support From<I> is less obvious. In theory something like this could be made to work...

#[autoimpl(for<I: IDatabase + Send + Sync + 'static> From<I> with |i| Self(Arc::new(i)))]

... but it's hardly better than writing the impl by hand:

impl<I: IDatabase + Send + Sync + 'static> From<I> for Database {
    fn from(i: I) -> Self {
        Self(Arc::new(i))
    }
}

The impl Self syntax from impl_scope! is supposed to help avoiding repetition, but IDatabase + Send + Sync + 'static is a dyn type as declared, not generic. I don't see a way of support that.


I guess what you're asking for is something more like this:

#[derive(Clone)]
#[impl_newtype(Deref, DerefMut, From)]
pub struct Database(Arc<dyn IDatabase + Send + Sync + 'static>);

The catch is it would be quite specific, supporting only structs with a single field like Foo<T>. (Handling the dyn correctly and supporting generics should be fine. From would have to assume that Foo::new is the correct constructor.)

... but e.g. it should also support struct Newtype(Arc<RwLock<dyn Trait + Send + Sync + 'static>>) as mentioned above. That requires a slightly different approach; perhaps searching for dyn A + ... within the type.

I could write such a thing. Please send an email if you wish to motivate me.

@dpc
Copy link
Author

dpc commented Oct 13, 2022

I had a look at your macro: basically, implement Deref and From.

I also have bunch of other macros for other things (clone, eq and some more custom stuff), but this is the core one that a type starts with.

I guess what you're asking for is something more like this:

I'm not convinced myself if this is something this library has to support. I just think it's worth considering.

As things are right now, working with dyn Trait is a bit of a PITA. Not only object-safety introduces a lot of limitations, but also a lot boilerplate is involved to make it ergonomic. I wish there was a good solution to standardize on, but maybe it should be a crate built entirely for that purpose. Donno.

@dhardy
Copy link
Contributor

dhardy commented Oct 14, 2022

As things are right now, working with dyn Trait is a bit of a PITA.

Yes it is. Sometimes restricting methods with where Self: Sized is enough; sometimes you end up needing separate "sized" and object-safe variants of a trait (object-safe methods cannot use associated types to let the impl define method return types). Clone doesn't work without specific machinery (your macro makes an interesting assumption, which might cause name collisions). Dynamic down-cast requires 'static. Neither upcast to a supertrait nor downcast to a more specific trait works (yet) without specific methods for those operations. impl Trait on methods is not (currently) supported.

Not only object-safety introduces a lot of limitations, but also a lot boilerplate is involved to make it ergonomic. I wish there was a good solution to standardize on, but maybe it should be a crate built entirely for that purpose.

My approach is usually to skip the newtype (maybe use a typedef instead). Box<Trait> does not get Clone, but you do get Deref and From. The only real benefits of newtype are (1) hiding the internal type from your API and (2) supporting a custom API which differs from that of the object-safe trait (e.g. DrawIface is a glorified newtype over the hidden trait DrawSharedImpl). The latter case requires custom implementations anyway since it differs from the trait, hence perhaps why I don't see much need for extra machinery here.

@dpc
Copy link
Author

dpc commented Nov 17, 2022

Nice. I'll give it a try in some short/mid term and report back. Thanks!

@dhardy
Copy link
Contributor

dhardy commented Nov 17, 2022

Sure. For the most part it's not implementing the trait directly on the newtype, but it still works (maybe unless you want to pass the newtype into a function with a bound on the trait?). Do let me know if there are issues.

It is possible to directly implement the trait on the newtype using this, but I think only for traits with a type parameter (like Newtype<T: Foo>(T)). In theory there could be another macro which implements the trait over any type which can dereference to that type, but it would have to be a different macro, so invent some new syntax I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants