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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 3 commits into from
Jun 13, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0dcf9e2
Change #[implement] so that interfaces go on Foo_Impl
Jun 2, 2024
bc86afc
update generated bindings
Jun 12, 2024
23affe2
fix debugger_visualizer
Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
35 changes: 19 additions & 16 deletions crates/libs/bindgen/src/rust/extensions/impl/Foundation/Collections/Iterable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Comment on lines +16 to +21
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}

Expand All @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much nicer.

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);
Comment on lines +63 to +64
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just formatting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

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)
}
}
Expand All @@ -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())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
51 changes: 36 additions & 15 deletions crates/libs/bindgen/src/rust/extensions/impl/Foundation/Collections/MapView.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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(())
Expand All @@ -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,
Expand All @@ -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))
}
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -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())
}
Expand Down
52 changes: 27 additions & 25 deletions crates/libs/bindgen/src/rust/extensions/impl/Foundation/Collections/VectorView.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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())
}
}
24 changes: 18 additions & 6 deletions crates/libs/bindgen/src/rust/implements.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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>(), });
}
Expand All @@ -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>, });
}
Expand All @@ -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
Expand Down
Loading