From 4e726825e00c5a843734e31381edebd063faaa53 Mon Sep 17 00:00:00 2001
From: sivadeilra <arlie.davis@microsoft.com>
Date: Sat, 25 May 2024 20:26:14 -0700
Subject: [PATCH] Fix handling of COM interfaces with same method name (#3059)

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>
---
 crates/libs/interface/src/lib.rs              |  7 ++-
 crates/tests/implement_core/src/com_object.rs | 50 ++++++++++++++++++-
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/crates/libs/interface/src/lib.rs b/crates/libs/interface/src/lib.rs
index b2f38b310af..090581fd376 100644
--- a/crates/libs/interface/src/lib.rs
+++ b/crates/libs/interface/src/lib.rs
@@ -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 {
diff --git a/crates/tests/implement_core/src/com_object.rs b/crates/tests/implement_core/src/com_object.rs
index de37c49bf99..6959bdcb7b9 100644
--- a/crates/tests/implement_core/src/com_object.rs
+++ b/crates/tests/implement_core/src/com_object.rs
@@ -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],
@@ -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 {
@@ -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)]
@@ -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);
@@ -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);