COM interface impls move from MyApp to MyApp_Impl ("outer" object)

[sivadeilra]

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 that IFoo 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 where T 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:

#[implement(IFoo)]
struct MyApp { ... }

impl IFoo_Impl for MyApp { ... }

will need to be changed to this:

#[implement(IFoo)]
struct MyApp { ... }

impl IFoo_Impl for MyApp_Impl { ... }

[kennykerr]

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,

1. I don't think its sufficient to offer an optional alternative.
2. There should just be one way to do this and that should be simplest and most streamlined default rather than something you need to opt in to.
3. Even if both approaches were sound, having two non-complimentary ways to do essentially the same thing seems very confusing.
4. I don't like the ergonomics of the sample in the PR description even if you remove the @ 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.

[sivadeilra]

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.

[sivadeilra]

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.

[kennykerr]

I like this!

impl IFoo_Impl for ComObject<MyApp> {
   ...
}

[sivadeilra]

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. :(

[kennykerr]

Is this just formatting?

[sivadeilra]

Yes. These files weren't being run through rustfmt before.

[kennykerr]

I wonder, can we implement Into/From here as a shorthand for the simple case of returning the interface as before - then it would be similar to before: Ok(StockIterator {...}.into()) ?

[sivadeilra]

The into() thing still totally works. I just like the ComObject::new(...).into_interface() more because it's more explicit.

[kennykerr]

Much nicer.

[kennykerr]

As above, be nice if this simple case had a shorthand.

[sivadeilra]

It still works; just being explicit. I can change this back, but I like the explicitness.

[kennykerr]

Looks like a new Rust nightly update is failing the merge build workflow. #3096