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

Improvements to the API #147

Closed
rylev opened this issue Jun 25, 2020 · 3 comments
Closed

Improvements to the API #147

rylev opened this issue Jun 25, 2020 · 3 comments
Labels
question Further information is requested

Comments

@rylev
Copy link
Contributor

rylev commented Jun 25, 2020

Progress on this crate has been fairly quite as we've been focusing on getting winrt-rs in working order. Now that that crate has solidified somewhat, it's time to return to this crate with the lessons learned from winrt. The following is a suggestion on the way we will proceed forward based on conversations I've had with @kennykerr. Please let us know if you have any feedback.

Traits vs Structs

Currently, COM interfaces are modeled as traits that must be used in conjunction with a ComPtr (for non-reference counting) or ComRc (for reference counting). This has certain advantages and disadvantages:

Pros

  • a familiar syntax for defining COM interfaces
  • built in support for ensuring that all methods are implemented for a given interface when building a COM server

Both Pro and Con

  • requires use of dyn keyword when dealing with interfaces. This is good in that it's a gentle reminder the user is performing dynamic dispatch, but it is extra syntax when presumably the user knows COM is dynamic dispatch.

Cons

  • extra syntax when taking a normal COM pointer as an argument: ComPtr<dyn IAnimal> vs IAnimal
  • strengths are for the less common case of producing COM interfaces rather than consuming
  • Not how COM works in other languages and thus might be slightly confusing to learn.

While switching to structs would not only be wins, there are enough here that it seems like the correct thing to do.

This would require a new syntax for defining COM interfaces. Something like the following:

#[com_interface("00000000-0000-0000-C000-000000000046")]
interface IUnknown {
    fn query_interface(
        &self,
        riid: *const IID,
        ppv: *mut *mut c_void
    ) -> HRESULT;
    fn add_ref(&self) -> u32;
    fn release(&self) -> u32;
}

and for implementing those interfaces:

#[com_interface_impl]
impl IDomesticAnimal for BritishShortHairCat {
    unsafe fn train(&self) -> HRESULT {
        println!("Training...");
        NOERROR
    }
}

Consumption of those interfaces would then look like this:

let factory = get_class_object(&CLSID_CAT_CLASS).unwrap();
println!("Got cat class object");

let unknown = factory
  .query_interface::<IUnknown>()
   .unwrap()
println!("Got IUnknown");

let animal = unknown
  .query_interface::<IAnimal>()
  .unwrap();
println!("Got IAnimal");

unsafe { animal.eat() };

Nullability

Currently ComPtr is used only for non-null pointers.

Pros

  • no null checks required for the user when the user has a ComPtr meaning they can never make a mistake and use a ComPtr that has not actually been initialized. This is the very reason Option<T> exists in Rust.

Cons

  • null COM pointers are used in many common API patterns including out params. Currently this requires the user to first initialize a raw pointer, pass that pointer as the out param and then initialize the ComPtr struct with the raw pointer which can be awkward.

This is being discussed already in #141

Method names

Currently there is a safe variant of query_interface that is called get_interface. The name difference is due to query_interface already existing and there being no method overloads. This is perhaps confusing, but the only two choices are to give the safe variant a different name (as is the case now), or give the unsafe variant a different name.

@rylev rylev added the question Further information is requested label Jun 25, 2020
@rylev
Copy link
Contributor Author

rylev commented Aug 5, 2020

Closing this after #156 since it touches on a lot of this. We can open new issues for remaining questions.

@rylev rylev closed this as completed Aug 5, 2020
@gentoo90
Copy link

Not sure if this has anything to do with COM VTables but you might want to take a look at rust-lang/rfcs#2967

@rylev
Copy link
Contributor Author

rylev commented Aug 24, 2020

That would certainly make a difference in the design of this library. We'll have to keep an eye out for that.

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

No branches or pull requests

2 participants