COM interface impls move to outer implementation (microsoft#3065)

crates/libs/bindgen/src/rust/extensions/impl/Foundation/Collections/Iterable.rs

@@ -7,17 +7,18 @@ where
     values: Vec<T::Default>,
 }
 
-impl<T> IIterable_Impl<T> for StockIterable<T>
+impl<T> IIterable_Impl<T> for StockIterable_Impl<T>
 where
     T: windows_core::RuntimeType,
     T::Default: Clone,
 {
     fn First(&self) -> windows_core::Result<IIterator<T>> {
-        unsafe {
-            // TODO: ideally we can do an AddRef rather than a QI here (via cast)...
-            // and then we can get rid of the unsafe as well.
-            Ok(StockIterator { owner: self.cast()?, current: 0.into() }.into())
-        }
+        use windows_core::IUnknownImpl;
+        Ok(windows_core::ComObject::new(StockIterator {
+            owner: self.to_object(),
+            current: 0.into(),
+        })
+        .into_interface())
     }
 }
 
@@ -27,52 +28,54 @@ where
     T: windows_core::RuntimeType + 'static,
     T::Default: Clone,
 {
-    owner: IIterable<T>,
+    owner: windows_core::ComObject<StockIterable<T>>,
     current: std::sync::atomic::AtomicUsize,
 }
 
-impl<T> IIterator_Impl<T> for StockIterator<T>
+impl<T> IIterator_Impl<T> for StockIterator_Impl<T>
 where
     T: windows_core::RuntimeType,
     T::Default: Clone,
 {
     fn Current(&self) -> windows_core::Result<T> {
-        let owner: &StockIterable<T> = unsafe { windows_core::AsImpl::as_impl(&self.owner) };
+        let owner: &StockIterable<T> = &self.owner;
         let current = self.current.load(std::sync::atomic::Ordering::Relaxed);
 
-        if owner.values.len() > current {
+        if self.owner.values.len() > current {
             T::from_default(&owner.values[current])
         } else {
             Err(windows_core::Error::from(windows_core::imp::E_BOUNDS))
         }
     }
 
     fn HasCurrent(&self) -> windows_core::Result<bool> {
-        let owner: &StockIterable<T> = unsafe { windows_core::AsImpl::as_impl(&self.owner) };
+        let owner: &StockIterable<T> = &self.owner;
         let current = self.current.load(std::sync::atomic::Ordering::Relaxed);
 
         Ok(owner.values.len() > current)
     }
 
     fn MoveNext(&self) -> windows_core::Result<bool> {
-        let owner: &StockIterable<T> = unsafe { windows_core::AsImpl::as_impl(&self.owner) };
+        let owner: &StockIterable<T> = &self.owner;
         let current = self.current.load(std::sync::atomic::Ordering::Relaxed);
 
         if current < owner.values.len() {
-            self.current.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+            self.current
+                .fetch_add(1, std::sync::atomic::Ordering::Relaxed);
         }
 
         Ok(owner.values.len() > current + 1)
     }
 
     fn GetMany(&self, values: &mut [T::Default]) -> windows_core::Result<u32> {
-        let owner: &StockIterable<T> = unsafe { windows_core::AsImpl::as_impl(&self.owner) };
+        let owner: &StockIterable<T> = &self.owner;
         let current = self.current.load(std::sync::atomic::Ordering::Relaxed);
 
         let actual = std::cmp::min(owner.values.len() - current, values.len());
         let (values, _) = values.split_at_mut(actual);
         values.clone_from_slice(&owner.values[current..current + actual]);
-        self.current.fetch_add(actual, std::sync::atomic::Ordering::Relaxed);
+        self.current
+            .fetch_add(actual, std::sync::atomic::Ordering::Relaxed);
         Ok(actual as u32)
     }
 }
@@ -85,6 +88,6 @@ where
     type Error = windows_core::Error;
     fn try_from(values: Vec<T::Default>) -> windows_core::Result<Self> {
         // TODO: should provide a fallible try_into or more explicit allocator
-        Ok(StockIterable { values }.into())
+        Ok(windows_core::ComObject::new(StockIterable { values }).into_interface())
     }
 }

crates/libs/bindgen/src/rust/extensions/impl/Foundation/Collections/MapView.rs

@@ -9,31 +9,36 @@ where
     map: std::collections::BTreeMap<K::Default, V::Default>,
 }
 
-impl<K, V> IIterable_Impl<IKeyValuePair<K, V>> for StockMapView<K, V>
+impl<K, V> IIterable_Impl<IKeyValuePair<K, V>> for StockMapView_Impl<K, V>
 where
     K: windows_core::RuntimeType,
     V: windows_core::RuntimeType,
     K::Default: Clone + Ord,
     V::Default: Clone,
 {
     fn First(&self) -> windows_core::Result<IIterator<IKeyValuePair<K, V>>> {
-        unsafe {
-            // TODO: ideally we can do an AddRef rather than a QI here (via cast)...
-            // and then we can get rid of the unsafe as well.
-            Ok(StockMapViewIterator::<K, V> { _owner: self.cast()?, current: std::sync::RwLock::new(self.map.iter()) }.into())
-        }
+        use windows_core::IUnknownImpl;
+
+        Ok(windows_core::ComObject::new(StockMapViewIterator::<K, V> {
+            _owner: self.to_object(),
+            current: std::sync::RwLock::new(self.map.iter()),
+        })
+        .into_interface())
     }
 }
 
-impl<K, V> IMapView_Impl<K, V> for StockMapView<K, V>
+impl<K, V> IMapView_Impl<K, V> for StockMapView_Impl<K, V>
 where
     K: windows_core::RuntimeType,
     V: windows_core::RuntimeType,
     K::Default: Clone + Ord,
     V::Default: Clone,
 {
     fn Lookup(&self, key: &K::Default) -> windows_core::Result<V> {
-        let value = self.map.get(key).ok_or_else(|| windows_core::Error::from(windows_core::imp::E_BOUNDS))?;
+        let value = self
+            .map
+            .get(key)
+            .ok_or_else(|| windows_core::Error::from(windows_core::imp::E_BOUNDS))?;
         V::from_default(value)
     }
     fn Size(&self) -> windows_core::Result<u32> {
@@ -42,7 +47,11 @@ where
     fn HasKey(&self, key: &K::Default) -> windows_core::Result<bool> {
         Ok(self.map.contains_key(key))
     }
-    fn Split(&self, first: &mut Option<IMapView<K, V>>, second: &mut Option<IMapView<K, V>>) -> windows_core::Result<()> {
+    fn Split(
+        &self,
+        first: &mut Option<IMapView<K, V>>,
+        second: &mut Option<IMapView<K, V>>,
+    ) -> windows_core::Result<()> {
         *first = None;
         *second = None;
         Ok(())
@@ -57,11 +66,11 @@ where
     K::Default: Clone + Ord,
     V::Default: Clone,
 {
-    _owner: IIterable<IKeyValuePair<K, V>>,
+    _owner: windows_core::ComObject<StockMapView<K, V>>,
     current: std::sync::RwLock<std::collections::btree_map::Iter<'a, K::Default, V::Default>>,
 }
 
-impl<'a, K, V> IIterator_Impl<IKeyValuePair<K, V>> for StockMapViewIterator<'a, K, V>
+impl<'a, K, V> IIterator_Impl<IKeyValuePair<K, V>> for StockMapViewIterator_Impl<'a, K, V>
 where
     K: windows_core::RuntimeType,
     V: windows_core::RuntimeType,
@@ -72,7 +81,11 @@ where
         let mut current = self.current.read().unwrap().clone().peekable();
 
         if let Some((key, value)) = current.peek() {
-            Ok(StockKeyValuePair { key: (*key).clone(), value: (*value).clone() }.into())
+            Ok(windows_core::ComObject::new(StockKeyValuePair {
+                key: (*key).clone(),
+                value: (*value).clone(),
+            })
+            .into_interface())
         } else {
             Err(windows_core::Error::from(windows_core::imp::E_BOUNDS))
         }
@@ -97,7 +110,13 @@ where
 
         for pair in pairs {
             if let Some((key, value)) = current.next() {
-                *pair = Some(StockKeyValuePair { key: (*key).clone(), value: (*value).clone() }.into());
+                *pair = Some(
+                    windows_core::ComObject::new(StockKeyValuePair {
+                        key: (*key).clone(),
+                        value: (*value).clone(),
+                    })
+                    .into_interface(),
+                );
                 actual += 1;
             } else {
                 break;
@@ -120,7 +139,7 @@ where
     value: V::Default,
 }
 
-impl<K, V> IKeyValuePair_Impl<K, V> for StockKeyValuePair<K, V>
+impl<K, V> IKeyValuePair_Impl<K, V> for StockKeyValuePair_Impl<K, V>
 where
     K: windows_core::RuntimeType,
     V: windows_core::RuntimeType,
@@ -143,7 +162,9 @@ where
     V::Default: Clone,
 {
     type Error = windows_core::Error;
-    fn try_from(map: std::collections::BTreeMap<K::Default, V::Default>) -> windows_core::Result<Self> {
+    fn try_from(
+        map: std::collections::BTreeMap<K::Default, V::Default>,
+    ) -> windows_core::Result<Self> {
         // TODO: should provide a fallible try_into or more explicit allocator
         Ok(StockMapView { map }.into())
     }

crates/libs/bindgen/src/rust/extensions/impl/Foundation/Collections/VectorView.rs

@@ -7,27 +7,32 @@ where
     values: Vec<T::Default>,
 }
 
-impl<T> IIterable_Impl<T> for StockVectorView<T>
+impl<T> IIterable_Impl<T> for StockVectorView_Impl<T>
 where
     T: windows_core::RuntimeType,
     T::Default: Clone + PartialEq,
 {
     fn First(&self) -> windows_core::Result<IIterator<T>> {
-        unsafe {
-            // TODO: ideally we can do an AddRef rather than a QI here (via cast)...
-            // and then we can get rid of the unsafe as well.
-            Ok(StockVectorViewIterator { owner: self.cast()?, current: 0.into() }.into())
-        }
+        use windows_core::IUnknownImpl;
+
+        Ok(windows_core::ComObject::new(StockVectorViewIterator {
+            owner: self.to_object(),
+            current: 0.into(),
+        })
+        .into_interface())
     }
 }
 
-impl<T> IVectorView_Impl<T> for StockVectorView<T>
+impl<T> IVectorView_Impl<T> for StockVectorView_Impl<T>
 where
     T: windows_core::RuntimeType,
     T::Default: Clone + PartialEq,
 {
     fn GetAt(&self, index: u32) -> windows_core::Result<T> {
-        let item = self.values.get(index as usize).ok_or_else(|| windows_core::Error::from(windows_core::imp::E_BOUNDS))?;
+        let item = self
+            .values
+            .get(index as usize)
+            .ok_or_else(|| windows_core::Error::from(windows_core::imp::E_BOUNDS))?;
         T::from_default(item)
     }
     fn Size(&self) -> windows_core::Result<u32> {
@@ -60,52 +65,49 @@ where
     T: windows_core::RuntimeType + 'static,
     T::Default: Clone + PartialEq,
 {
-    owner: IIterable<T>,
+    owner: windows_core::ComObject<StockVectorView<T>>,
     current: std::sync::atomic::AtomicUsize,
 }
 
-impl<T> IIterator_Impl<T> for StockVectorViewIterator<T>
+impl<T> IIterator_Impl<T> for StockVectorViewIterator_Impl<T>
 where
     T: windows_core::RuntimeType,
     T::Default: Clone + PartialEq,
 {
     fn Current(&self) -> windows_core::Result<T> {
-        let owner: &StockVectorView<T> = unsafe { windows_core::AsImpl::as_impl(&self.owner) };
         let current = self.current.load(std::sync::atomic::Ordering::Relaxed);
 
-        if owner.values.len() > current {
-            T::from_default(&owner.values[current])
+        if let Some(item) = self.owner.values.get(current) {
+            T::from_default(item)
         } else {
             Err(windows_core::Error::from(windows_core::imp::E_BOUNDS))
         }
     }
 
     fn HasCurrent(&self) -> windows_core::Result<bool> {
-        let owner: &StockVectorView<T> = unsafe { windows_core::AsImpl::as_impl(&self.owner) };
         let current = self.current.load(std::sync::atomic::Ordering::Relaxed);
-
-        Ok(owner.values.len() > current)
+        Ok(self.owner.values.len() > current)
     }
 
     fn MoveNext(&self) -> windows_core::Result<bool> {
-        let owner: &StockVectorView<T> = unsafe { windows_core::AsImpl::as_impl(&self.owner) };
         let current = self.current.load(std::sync::atomic::Ordering::Relaxed);
 
-        if current < owner.values.len() {
-            self.current.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
+        if current < self.owner.values.len() {
+            self.current
+                .fetch_add(1, std::sync::atomic::Ordering::Relaxed);
         }
 
-        Ok(owner.values.len() > current + 1)
+        Ok(self.owner.values.len() > current + 1)
     }
 
     fn GetMany(&self, values: &mut [T::Default]) -> windows_core::Result<u32> {
-        let owner: &StockVectorView<T> = unsafe { windows_core::AsImpl::as_impl(&self.owner) };
         let current = self.current.load(std::sync::atomic::Ordering::Relaxed);
 
-        let actual = std::cmp::min(owner.values.len() - current, values.len());
+        let actual = std::cmp::min(self.owner.values.len() - current, values.len());
         let (values, _) = values.split_at_mut(actual);
-        values.clone_from_slice(&owner.values[current..current + actual]);
-        self.current.fetch_add(actual, std::sync::atomic::Ordering::Relaxed);
+        values.clone_from_slice(&self.owner.values[current..current + actual]);
+        self.current
+            .fetch_add(actual, std::sync::atomic::Ordering::Relaxed);
         Ok(actual as u32)
     }
 }
@@ -118,6 +120,6 @@ where
     type Error = windows_core::Error;
     fn try_from(values: Vec<T::Default>) -> windows_core::Result<Self> {
         // TODO: should provide a fallible try_into or more explicit allocator
-        Ok(StockVectorView { values }.into())
+        Ok(windows_core::ComObject::new(StockVectorView { values }).into_interface())
     }
 }

crates/libs/bindgen/src/rust/implements.rs

@@ -97,10 +97,16 @@ pub fn writer(writer: &Writer, def: metadata::TypeDef) -> TokenStream {
 
         if has_unknown_base {
             quote! {
-                unsafe extern "system" fn #name<#constraints Identity: windows_core::IUnknownImpl<Impl = Impl>, Impl: #impl_ident<#generic_names>, const OFFSET: isize> #vtbl_signature {
+                unsafe extern "system" fn #name<
+                    #constraints
+                    Identity: windows_core::IUnknownImpl,
+                    const OFFSET: isize
+                > #vtbl_signature
+                where
+                    Identity: #impl_ident<#generic_names>
+                {
                     // offset the `this` pointer by `OFFSET` times the size of a pointer and cast it as an IUnknown implementation
-                    let this = (this as *const *const ()).offset(OFFSET) as *const Identity;
-                    let this = (*this).get_impl();
+                    let this: &Identity = &*((this as *const *const ()).offset(OFFSET) as *const Identity);
                     #invoke_upcall
                 }
             }
@@ -123,7 +129,7 @@ pub fn writer(writer: &Writer, def: metadata::TypeDef) -> TokenStream {
         Some(metadata::Type::TypeDef(def, generics)) => {
             let name = writer.type_def_name_imp(*def, generics, "_Vtbl");
             if has_unknown_base {
-                methods.combine(&quote! { base__: #name::new::<Identity, Impl, OFFSET>(), });
+                methods.combine(&quote! { base__: #name::new::<Identity, OFFSET>(), });
             } else {
                 methods.combine(&quote! { base__: #name::new::<Impl>(), });
             }
@@ -136,7 +142,7 @@ pub fn writer(writer: &Writer, def: metadata::TypeDef) -> TokenStream {
     for method in def.methods() {
         let name = method_names.add(method);
         if has_unknown_base {
-            methods.combine(&quote! { #name: #name::<#generic_names Identity, Impl, OFFSET>, });
+            methods.combine(&quote! { #name: #name::<#generic_names Identity, OFFSET>, });
         } else {
             methods.combine(&quote! { #name: #name::<Impl>, });
         }
@@ -151,7 +157,13 @@ pub fn writer(writer: &Writer, def: metadata::TypeDef) -> TokenStream {
             #runtime_name
             #features
             impl<#constraints> #vtbl_ident<#generic_names> {
-                pub const fn new<Identity: windows_core::IUnknownImpl<Impl = Impl>, Impl: #impl_ident<#generic_names>, const OFFSET: isize>() -> #vtbl_ident<#generic_names> {
+                pub const fn new<
+                    Identity: windows_core::IUnknownImpl,
+                    const OFFSET: isize
+                >() -> #vtbl_ident<#generic_names>
+                where
+                    Identity : #impl_ident<#generic_names>
+                {
                     #(#method_impls)*
                     Self{
                         #methods