Closed sivadeilra closed 1 month ago
More fundamentally, the problem with the existing implementation is that certain functions - like casting/querying for an interface implemented by the implementation - assume that the type that the implement
macro is attached to is already boxed and there's no safe/sound way to panic if and when that's not the case. This PR aims to solve that nicely and it should just be "the way" to implement COM objects in Rust. So,
@ Outer
thing. #[implement(IFoo)]
struct MyApp { ... }
impl IFoo_Impl for MyApp_Impl { ... }
There's no hint that IFoo_Impl
is even a name to be reckoned with. That's more a problem with the interface definition, but MyApp_Impl
is likewise something you'd have to "know" a priori. Even impl IFoo::Impl for MyApp::Impl
would be less mysterious and name-mangily. Maybe something else entirely. I'll play around with it over the next few days and see if I can come up with something a bit more natural. If we are to make a change here, I want to be confident that we are on a good path before we disrupt existing code.
The more I think about it, I think we should just flip to MyApp_Impl
for COM interface implementations and be done with it. Yes, it's a breaking change, but windows-rs
has made breaking changes before, and the major version number is still 0.
Edit: Actually, I just thought of something. What about this?
impl IFoo_Impl for ComObject<MyApp> {
...
}
This actually compiles. I haven't tried wiring up the method dispatch code to it, but this might be a good approach.
Ok, I implemented support for placing interface impls on ComObject<T>
instead of on T
or T_Impl
. It looks nice, especially from the POV of the developer implementing a COM object, because ComObject<MyApp>
gives developers an "go to docs" thing, and it gives them an easy way to discover the methods that are in-scope for self
.
I created a draft PR, #3071 , just for discussion. That PR still has the "inner vs. outer" option on #[implement]
, and I'm leaning toward "this needs to just be a breaking change". So either 1) we go with the opt-in, or 2) we go with the breaking change. In either case, I'll update this PR after the discussion, to reflect the final design, rather than #3071.
I like this!
impl IFoo_Impl for ComObject<MyApp> { ... }
I like this!
Me too! ... Unfortunately, I tried taking that all the way to completion, and ran into an unsolvable problem with generics and trait implementations. It's not going to work. :(
Looks like a new Rust nightly update is failing the merge build workflow. #3096
This PR requires COM objects to implement COM interfaces on the "outer" (
MyApp_Impl
) type, which is generated by the#[implement]
macro, instead of the user-defined (or "inner") (MyApp
) type. This is important because many COM objects need access to the full COM object, not just the contents of the wrapped "inner" type.For example, COM objects may need to cast themselves to a particular
IFoo
that they implement and then return thatIFoo
pointer as an output parameter of a COM method call. This is not possible in the current design because COM objects receive method calls that are typed&self : T
whereT
is the inner type, so they do not have access to the vtables or reference count of the current COM object.This is a breaking change to all code that uses the
#[implement]
macro. All code in this pattern:will need to be changed to this: