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

windows-bindgen now uses Ref and OutRef for COM interface traits #3386

Merged
merged 5 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/clippy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ jobs:
run: cargo clippy -p test_query_signature
- name: Clippy test_readme
run: cargo clippy -p test_readme
- name: Clippy test_ref_params
run: cargo clippy -p test_ref_params
- name: Clippy test_reference
run: cargo clippy -p test_reference
- name: Clippy test_reference_client
Expand Down
8 changes: 6 additions & 2 deletions .github/workflows/raw-dylib.yml
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,12 @@ jobs:
run: cargo test -p test_query_signature --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_readme
run: cargo test -p test_readme --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_reference
run: cargo test -p test_reference --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_ref_params
run: cargo test -p test_ref_params --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Clean
run: cargo clean
- name: Test test_reference
run: cargo test -p test_reference --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_reference_client
run: cargo test -p test_reference_client --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_reference_float
Expand Down Expand Up @@ -362,6 +364,8 @@ jobs:
run: cargo test -p windows_x86_64_gnu --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test windows_x86_64_gnullvm
run: cargo test -p windows_x86_64_gnullvm --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Clean
run: cargo clean
- name: Test windows_x86_64_msvc
run: cargo test -p windows_x86_64_msvc --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Check diff
Expand Down
8 changes: 6 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,12 @@ jobs:
run: cargo test -p test_query_signature --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_readme
run: cargo test -p test_readme --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_reference
run: cargo test -p test_reference --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_ref_params
run: cargo test -p test_ref_params --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Clean
run: cargo clean
- name: Test test_reference
run: cargo test -p test_reference --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_reference_client
run: cargo test -p test_reference_client --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test test_reference_float
Expand Down Expand Up @@ -359,6 +361,8 @@ jobs:
run: cargo test -p windows_x86_64_gnu --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Test windows_x86_64_gnullvm
run: cargo test -p windows_x86_64_gnullvm --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Clean
run: cargo clean
- name: Test windows_x86_64_msvc
run: cargo test -p windows_x86_64_msvc --target ${{ matrix.target }} ${{ matrix.etc }}
- name: Check diff
Expand Down
4 changes: 2 additions & 2 deletions crates/libs/bindgen/src/types/cpp_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl CppFn {
let where_clause = method.write_where(writer, false);
let return_type = signature.params[signature.params.len() - 1].0.deref();

let map = if !return_type.is_nullable() {
let map = if !return_type.is_interface() {
quote! { map(||core::mem::transmute(result__)) }
} else {
quote! { and_then(||windows_core::Type::from_abi(result__)) }
Expand Down Expand Up @@ -167,7 +167,7 @@ impl CppFn {
.0
.deref();

if return_type.is_nullable() {
if return_type.is_interface() {
let return_type = return_type.write_name(writer);

quote! {
Expand Down
21 changes: 12 additions & 9 deletions crates/libs/bindgen/src/types/cpp_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ impl CppMethod {

let map = if return_type.is_copyable() {
quote! { map(||result__) }
} else if return_type.is_nullable() {
} else if return_type.is_interface() {
quote! { and_then(||windows_core::Type::from_abi(result__)) }
} else {
quote! { map(||core::mem::transmute(result__)) }
Expand Down Expand Up @@ -337,7 +337,7 @@ impl CppMethod {
.0
.deref();

if return_type.is_nullable() {
if return_type.is_interface() {
let return_type = return_type.write_name(writer);

quote! {
Expand Down Expand Up @@ -743,12 +743,15 @@ fn write_produce_type(writer: &Writer, ty: &Type, param: Param) -> TokenStream {
let name = to_ident(&param.name().to_lowercase());
let kind = ty.write_default(writer);

if param.flags().contains(ParamAttributes::In) {
if !param.flags().contains(ParamAttributes::Out) && ty.is_interface() {
let type_name = ty.write_name(writer);
quote! { #name: windows_core::Ref<'_, #type_name>, }
} else if param.flags().contains(ParamAttributes::Out) && ty.deref().is_interface() {
let type_name = ty.deref().write_name(writer);
quote! { #name: windows_core::OutRef<'_, #type_name>, }
} else if param.flags().contains(ParamAttributes::In) {
if ty.is_primitive() {
quote! { #name: #kind, }
} else if ty.is_nullable() {
let kind = ty.write_name(writer);
quote! { #name: Option<&#kind>, }
} else {
quote! { #name: &#kind, }
}
Expand All @@ -760,9 +763,9 @@ fn write_produce_type(writer: &Writer, ty: &Type, param: Param) -> TokenStream {
fn write_invoke_arg(ty: &Type, param: Param, _hint: ParamHint) -> TokenStream {
let name = to_ident(&param.name().to_lowercase());

if param.flags().contains(ParamAttributes::In) && ty.is_nullable() {
quote! { windows_core::from_raw_borrowed(&#name) }
} else if (!ty.is_pointer() && ty.is_nullable())
if !param.flags().contains(ParamAttributes::Out) && ty.is_interface() {
quote! { core::mem::transmute_copy(&#name) }
} else if (!ty.is_pointer() && ty.is_interface())
|| (param.flags().contains(ParamAttributes::In) && !ty.is_primitive())
{
quote! { core::mem::transmute(&#name) }
Expand Down
15 changes: 8 additions & 7 deletions crates/libs/bindgen/src/types/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ impl Method {
quote! { core::slice::from_raw_parts(core::mem::transmute_copy(&#name), #abi_size_name as usize) }
} else if param.0.is_primitive() {
quote! { #name }
} else if param.0.is_const_ref() {
} else if param.0.is_const_ref() || param.0.is_interface() {
quote! { core::mem::transmute_copy(&#name) }
} else if param.0.is_nullable() {
quote! { windows_core::from_raw_borrowed(&#name) }
} else {
quote! { core::mem::transmute(&#name) }
}
Expand Down Expand Up @@ -144,9 +142,9 @@ impl Method {
quote! { &[#default_type] }
} else if p.0.is_primitive() {
quote! { #default_type }
} else if p.0.is_nullable() {
} else if p.0.is_interface() {
let type_name = p.0.write_name(writer);
quote! { Option<&#type_name> }
quote! { windows_core::Ref<'_, #type_name> }
} else {
quote! { &#default_type }
}
Expand All @@ -155,6 +153,9 @@ impl Method {
} else if p.0.is_winrt_array_ref() {
let kind = p.0.write_name(writer);
quote! { &mut windows_core::Array<#kind> }
} else if p.0.is_interface() {
let type_name = p.0.write_name(writer);
quote! { windows_core::OutRef<'_, #type_name> }
} else {
quote! { &mut #default_type }
};
Expand All @@ -180,7 +181,7 @@ impl Method {
};

let return_type_tokens = if noexcept {
if self.signature.return_type.0.is_nullable() {
if self.signature.return_type.0.is_interface() {
quote! { -> Option<#return_type_tokens> }
} else if self.signature.return_type.0 == Type::Void {
quote! {}
Expand Down Expand Up @@ -431,7 +432,7 @@ impl Method {
let noexcept = self.def.has_attribute("NoExceptionAttribute");

let return_type = if noexcept {
if self.signature.return_type.0.is_nullable() {
if self.signature.return_type.0.is_interface() {
quote! { -> Option<#return_type> }
} else if self.signature.return_type.0 == Type::Void {
quote! {}
Expand Down
6 changes: 3 additions & 3 deletions crates/libs/bindgen/src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,11 @@ impl Type {
Self::PtrMut(ty, pointers) => Self::PtrMut(ty.clone(), pointers - 1),
Self::PSTR | Self::PCSTR => Self::U8,
Self::PWSTR | Self::PCWSTR => Self::U16,
rest => panic!("{rest:?}"),
_ => self.clone(),
}
}

pub fn is_nullable(&self) -> bool {
pub fn is_interface(&self) -> bool {
matches!(
self,
Self::Class(_)
Expand Down Expand Up @@ -472,7 +472,7 @@ impl Type {

if matches!(self, Self::Param(_)) {
quote! { <#tokens as windows_core::Type<#tokens>>::Default }
} else if self.is_nullable() && !writer.config.sys {
} else if self.is_interface() && !writer.config.sys {
quote! { Option<#tokens> }
} else {
tokens
Expand Down
6 changes: 0 additions & 6 deletions crates/libs/core/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,6 @@ pub unsafe trait Interface: Sized + Clone {
}
}

/// # Safety
#[doc(hidden)]
pub unsafe fn from_raw_borrowed<T: Interface>(raw: &*mut c_void) -> Option<&T> {
T::from_raw_borrowed(raw)
}

/// This has the same memory representation as `IFoo`, but represents a borrowed interface pointer.
///
/// This type has no `Drop` impl; it does not AddRef/Release the given interface. However, because
Expand Down
33 changes: 32 additions & 1 deletion crates/libs/core/src/ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,45 @@ use core::mem::transmute;
pub struct Ref<'a, T: Type<T>>(T::Abi, PhantomData<&'a T>);

impl<T: Type<T, Default = Option<T>, Abi = *mut c_void>> Ref<'_, T> {
/// Returns `true` if the argument is null.
pub fn is_null(&self) -> bool {
self.0.is_null()
}

/// Converts the argument to a [Result<&T>] reference.
pub fn ok(&self) -> Result<&T> {
if self.0.is_null() {
Err(Error::from_hresult(imp::E_POINTER))
} else {
unsafe { Ok(transmute::<&*mut c_void, &T>(&self.0)) }
unsafe { Ok(self.assume_init()) }
}
}

/// Converts the argument to a `&T` reference.
///
/// # Panics
///
/// Panics if the argument is null.
pub fn unwrap(&self) -> &T {
if self.0.is_null() {
panic!("called `Ref::unwrap` on a null value")
} else {
unsafe { self.assume_init() }
}
}

/// Converts the argument to an [Option<T>] by cloning the reference.
pub fn cloned(&self) -> Option<T> {
if self.0.is_null() {
None
} else {
unsafe { Some(self.assume_init().clone()) }
}
}

unsafe fn assume_init(&self) -> &T {
transmute::<&*mut c_void, &T>(&self.0)
}
}

impl<T: Type<T>> core::ops::Deref for Ref<'_, T> {
Expand Down
Loading
Loading