Skip to content

Commit

Permalink
Fix handling of COM interfaces with same method name (microsoft#3059)
Browse files Browse the repository at this point in the history
It is common in COM interfaces for different interfaces to have methods
that have the same name, especially for interface "versions" that
extend the semantics of the underlying interfaces.  For example,
look at many of the inheritance relationships in COM interfaces in
DirectWrite (IDWriteTextFormat, IDWriteTextFormat2, IDWriteTextFormat3,
etc.)

This fixes the handling of this situation. All that is necessary is to
use the syntax which explicitly selects a specific trait, when invoking
IFoo_Impl methods. This PR adds unit test coverage for this situation.

Co-authored-by: Arlie Davis <ardavis@microsoft.com>
  • Loading branch information
2 people authored and mati865 committed Jun 15, 2024
1 parent ad33bbf commit 4e72682
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 3 deletions.
7 changes: 5 additions & 2 deletions crates/libs/interface/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,11 @@ impl Interface {
quote! {
unsafe extern "system" fn #name<Identity: ::windows_core::IUnknownImpl<Impl = Impl>, Impl: #trait_name, const OFFSET: isize>(this: *mut ::core::ffi::c_void, #(#args),*) #ret {
let this = (this as *const *const ()).offset(OFFSET) as *const Identity;
let this = (*this).get_impl();
this.#name(#(#params),*).into()
let this_impl: &Impl = (*this).get_impl();
// We use explicit <Impl as IFoo_Impl> so that we can select the correct method
// for situations where IFoo3 derives from IFoo2 and both declare a method with
// the same name.
<Impl as #trait_name>::#name(this_impl, #(#params),*).into()
}
}
} else {
Expand Down
50 changes: 49 additions & 1 deletion crates/tests/implement_core/src/com_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,25 @@ unsafe trait IFoo: IUnknown {
fn get_x(&self) -> u32;

fn get_self_as_bar(&self) -> IBar;

fn common(&self) -> u32;
}

#[interface("687eb4b2-6df6-41a3-86c7-4b04b94ad2d8")]
unsafe trait IBar: IUnknown {
fn say_hello(&self);

fn common(&self) -> u64;
}

#[interface("4351c285-97ad-450a-b445-8795632d2fb9")]
unsafe trait IBar2: IBar {
fn common(&self) -> f32;
}

const APP_SIGNATURE: [u8; 8] = *b"cafef00d";

#[implement(IFoo, IBar)]
#[implement(IFoo, IBar, IBar2)]
struct MyApp {
// We use signature to verify field offsets for dynamic casts
signature: [u8; 8],
Expand All @@ -38,12 +47,26 @@ impl IFoo_Impl for MyApp {
let outer = MyApp_Impl::from_inner_ref(self);
outer.to_interface()
}

unsafe fn common(&self) -> u32 {
100
}
}

impl IBar_Impl for MyApp {
unsafe fn say_hello(&self) {
println!("Hello!");
}

unsafe fn common(&self) -> u64 {
1_000_000_000_000
}
}

impl IBar2_Impl for MyApp {
unsafe fn common(&self) -> f32 {
std::f32::consts::PI
}
}

impl Borrow<u32> for MyApp {
Expand Down Expand Up @@ -376,6 +399,23 @@ fn dynamic_cast() {
assert_eq!(dyn_app_owned_2.signature, APP_SIGNATURE);
}

// Test that we can invoke the correct method in situations where two different
// interfaces declare a method with the same name, including situations where
// one of the interfaces inherits from the other.
#[test]
fn common_method_name() {
let app = MyApp::new(42);

let ifoo: IFoo = app.to_interface();
assert_eq!(unsafe { ifoo.common() }, 100);

let ibar: IBar = app.to_interface();
assert_eq!(unsafe { ibar.common() }, 1_000_000_000_000);

let ibar2: IBar2 = app.to_interface();
assert_eq!(unsafe { ibar2.common() }, std::f32::consts::PI);
}

// This tests that we can place a type that is not Send in a ComObject.
// Compilation is sufficient to test.
#[implement(IBar)]
Expand All @@ -387,6 +427,10 @@ impl IBar_Impl for UnsendableThing {
unsafe fn say_hello(&self) {
println!("{}", self.cell.get());
}

unsafe fn common(&self) -> u64 {
0
}
}

static_assertions::assert_not_impl_all!(UnsendableThing: Send, Sync);
Expand All @@ -401,6 +445,10 @@ impl IBar_Impl for SendableThing {
unsafe fn say_hello(&self) {
println!("{}", *self.arc);
}

unsafe fn common(&self) -> u64 {
0
}
}

static_assertions::assert_impl_all!(SendableThing: Send, Sync);
Expand Down

0 comments on commit 4e72682

Please sign in to comment.