Skip to content

Switch non-blittable structs to use ManuallyDrop for their ABI #1092

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Aug 26, 2021
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
6 changes: 3 additions & 3 deletions crates/gen/src/bstr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pub fn gen_bstr() -> TokenStream {
quote! {
#[repr(transparent)]
#[derive(::std::cmp::Eq)]
pub struct BSTR(*mut u16);
pub struct BSTR(pub *mut u16);
impl BSTR {
/// Create an empty `BSTR`.
///
Expand Down Expand Up @@ -141,10 +141,10 @@ pub fn gen_bstr() -> TokenStream {
}
}
unsafe impl ::windows::Abi for BSTR {
type Abi = *mut u16;
type Abi = ::std::mem::ManuallyDrop<Self>;
type DefaultType = Self;

fn set_abi(&mut self) -> *mut *mut u16 {
fn set_abi(&mut self) -> *mut Self::Abi {
debug_assert!(self.0.is_null());
&mut self.0 as *mut _ as _
}
Expand Down
9 changes: 7 additions & 2 deletions crates/gen/src/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ pub fn gen_turbo_abi_name(def: &TypeDef, gen: &Gen) -> TokenStream {
format_name(def, gen, to_abi_ident, true)
}

fn to_abi_ident(name: &str) -> TokenStream {
format_token!("{}_abi", name)
}

pub fn gen_name(def: &ElementType, gen: &Gen) -> TokenStream {
match def {
ElementType::Void => quote! { ::std::ffi::c_void },
Expand Down Expand Up @@ -154,10 +158,11 @@ fn gen_abi_type(def: &TypeDef, gen: &Gen) -> TokenStream {
match def.kind() {
TypeKind::Enum => gen_type_name(def, gen),
TypeKind::Struct => {
let tokens = gen_type_name(def, gen);
if def.is_blittable() {
gen_type_name(def, gen)
tokens
} else {
gen_abi_name(def, gen)
quote! { ::std::mem::ManuallyDrop<#tokens> }
}
}
_ => quote! { ::windows::RawPtr },
Expand Down
13 changes: 1 addition & 12 deletions crates/gen/src/struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,20 +147,9 @@ fn gen_struct_with_name(def: &TypeDef, struct_name: &str, gen: &Gen) -> TokenStr
}
}
} else {
let abi_name = gen_abi_name(def, gen);

let fields = fields.iter().map(|(_, signature, name)| {
let kind = gen_abi_sig(signature, gen);
quote! { pub #name: #kind }
});

quote! {
#repr
#[doc(hidden)]
#[derive(::std::clone::Clone, ::std::marker::Copy)]
pub #struct_or_union #abi_name{ #(#fields),* }
unsafe impl ::windows::Abi for #name {
type Abi = #abi_name;
type Abi = ::std::mem::ManuallyDrop<Self>;
type DefaultType = Self;
}
}
Expand Down
4 changes: 0 additions & 4 deletions crates/gen/src/to_ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,3 @@ pub fn to_ident(name: &str) -> TokenStream {
_ => name.into(),
}
}

pub fn to_abi_ident(name: &str) -> TokenStream {
format_token!("{}_abi", name)
}
28 changes: 16 additions & 12 deletions src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2663,7 +2663,7 @@ pub mod Windows {
}
#[repr(transparent)]
#[derive(:: std :: cmp :: Eq)]
pub struct BSTR(*mut u16);
pub struct BSTR(pub *mut u16);
impl BSTR {
#[doc = r" Create an empty `BSTR`."]
#[doc = r""]
Expand Down Expand Up @@ -2782,9 +2782,9 @@ pub mod Windows {
}
}
unsafe impl ::windows::Abi for BSTR {
type Abi = *mut u16;
type Abi = ::std::mem::ManuallyDrop<Self>;
type DefaultType = Self;
fn set_abi(&mut self) -> *mut *mut u16 {
fn set_abi(&mut self) -> *mut Self::Abi {
debug_assert!(self.0.is_null());
&mut self.0 as *mut _ as _
}
Expand Down Expand Up @@ -3033,7 +3033,7 @@ pub mod Windows {
{
#[link(name = "OLEAUT32")]
extern "system" {
fn SysFreeString(bstrstring: BSTR_abi);
fn SysFreeString(bstrstring: ::std::mem::ManuallyDrop<BSTR>);
}
SysFreeString(bstrstring.into_param().abi())
}
Expand All @@ -3045,7 +3045,7 @@ pub mod Windows {
{
#[link(name = "OLEAUT32")]
extern "system" {
fn SysStringLen(pbstr: BSTR_abi) -> u32;
fn SysStringLen(pbstr: ::std::mem::ManuallyDrop<BSTR>) -> u32;
}
SysStringLen(pbstr.into_param().abi())
}
Expand Down Expand Up @@ -3737,15 +3737,17 @@ pub mod Windows {
) -> ::windows::HRESULT,
pub unsafe extern "system" fn(
this: ::windows::RawPtr,
pbstrsource: *mut super::super::Foundation::BSTR_abi,
pbstrsource: *mut ::std::mem::ManuallyDrop<super::super::Foundation::BSTR>,
) -> ::windows::HRESULT,
pub unsafe extern "system" fn(
this: ::windows::RawPtr,
pbstrdescription: *mut super::super::Foundation::BSTR_abi,
pbstrdescription: *mut ::std::mem::ManuallyDrop<
super::super::Foundation::BSTR,
>,
) -> ::windows::HRESULT,
pub unsafe extern "system" fn(
this: ::windows::RawPtr,
pbstrhelpfile: *mut super::super::Foundation::BSTR_abi,
pbstrhelpfile: *mut ::std::mem::ManuallyDrop<super::super::Foundation::BSTR>,
) -> ::windows::HRESULT,
pub unsafe extern "system" fn(
this: ::windows::RawPtr,
Expand Down Expand Up @@ -4201,14 +4203,16 @@ pub mod Windows {
pub unsafe extern "system" fn(this: ::windows::RawPtr) -> u32,
pub unsafe extern "system" fn(
this: ::windows::RawPtr,
description: *mut super::super::Foundation::BSTR_abi,
description: *mut ::std::mem::ManuallyDrop<super::super::Foundation::BSTR>,
error: *mut ::windows::HRESULT,
restricteddescription: *mut super::super::Foundation::BSTR_abi,
capabilitysid: *mut super::super::Foundation::BSTR_abi,
restricteddescription: *mut ::std::mem::ManuallyDrop<
super::super::Foundation::BSTR,
>,
capabilitysid: *mut ::std::mem::ManuallyDrop<super::super::Foundation::BSTR>,
) -> ::windows::HRESULT,
pub unsafe extern "system" fn(
this: ::windows::RawPtr,
reference: *mut super::super::Foundation::BSTR_abi,
reference: *mut ::std::mem::ManuallyDrop<super::super::Foundation::BSTR>,
) -> ::windows::HRESULT,
);
#[repr(transparent)]
Expand Down
2 changes: 1 addition & 1 deletion tests/bstr/tests/bstr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn clone() {
let b = a.clone();
assert_eq!(a, "hello");
assert_eq!(b, "hello");
assert_ne!(a.abi(), b.abi());
assert_ne!(a.0, b.0);

let a = BSTR::default();
let b = a.clone();
Expand Down
94 changes: 75 additions & 19 deletions tests/data_object/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,43 +4,99 @@ use Windows::Win32::Foundation::*;
use Windows::Win32::System::Com::*;

#[implement(Windows::Win32::System::Com::IDataObject)]
struct Test();
#[derive(Default)]
#[allow(non_snake_case)]
struct Test {
GetData: bool,
GetDataHere: bool,
QueryGetData: bool,
GetCanonicalFormatEtc: bool,
SetData: bool,
EnumFormatEtc: bool,
DAdvise: bool,
DUnadvise: bool,
EnumDAdvise: bool,
}

#[allow(non_snake_case)]
impl Test {
fn GetData(&self, _: *const FORMATETC) -> Result<STGMEDIUM> {
panic!()
fn GetData(&mut self, _: *const FORMATETC) -> Result<STGMEDIUM> {
self.GetData = true;
Ok(STGMEDIUM {
tymed: 0,
Anonymous: STGMEDIUM_0 {
pstg: std::ptr::null_mut(),
},
pUnkForRelease: None,
})
}

fn GetDataHere(&mut self, _: *const FORMATETC, _: *mut STGMEDIUM) -> Result<()> {
self.GetDataHere = true;
Ok(())
}

fn GetDataHere(&self, _: *const FORMATETC, _: *mut STGMEDIUM) -> Result<()> {
panic!()
fn QueryGetData(&mut self, _: *const FORMATETC) -> Result<()> {
self.QueryGetData = true;
Ok(())
}

fn QueryGetData(&self, _: *const FORMATETC) -> Result<()> {
panic!()
fn GetCanonicalFormatEtc(&mut self, _: *const FORMATETC) -> Result<FORMATETC> {
self.GetCanonicalFormatEtc = true;
Ok(FORMATETC::default())
}

fn GetCanonicalFormatEtc(&self, _: *const FORMATETC) -> Result<FORMATETC> {
panic!()
fn SetData(&mut self, _: *const FORMATETC, _: *const STGMEDIUM, _: BOOL) -> Result<()> {
self.SetData = true;
Ok(())
}

fn SetData<'a>(&self, _: *const FORMATETC, _: *const STGMEDIUM, _: BOOL) -> Result<()> {
panic!()
fn EnumFormatEtc(&mut self, _: u32) -> Result<IEnumFORMATETC> {
self.EnumFormatEtc = true;
Err(Error::OK)
}

fn EnumFormatEtc(&self, _: u32) -> Result<IEnumFORMATETC> {
panic!()
fn DAdvise(&mut self, _: *const FORMATETC, _: u32, _: &Option<IAdviseSink>) -> Result<u32> {
self.DAdvise = true;
Ok(0)
}

fn DAdvise<'a>(&self, _: *const FORMATETC, _: u32, _: &Option<IAdviseSink>) -> Result<u32> {
panic!()
fn DUnadvise(&mut self, _: u32) -> Result<()> {
self.DUnadvise = true;
Ok(())
}

fn DUnadvise(&self, _: u32) -> Result<()> {
panic!()
fn EnumDAdvise(&mut self) -> Result<IEnumSTATDATA> {
self.EnumDAdvise = true;
Err(Error::OK)
}
}

#[test]
fn test() -> Result<()> {
unsafe {
let d: IDataObject = Test::default().into();
d.GetData(std::ptr::null_mut())?;
d.GetDataHere(std::ptr::null_mut(), std::ptr::null_mut())?;
d.QueryGetData(std::ptr::null_mut())?;
d.GetCanonicalFormatEtc(std::ptr::null_mut())?;
d.SetData(std::ptr::null_mut(), std::ptr::null_mut(), false)?;
let _ = d.EnumFormatEtc(0);
d.DAdvise(std::ptr::null_mut(), 0, None)?;
d.DUnadvise(0)?;
let _ = d.EnumDAdvise();

let i = Test::to_impl(&d);
assert!(i.GetData);
assert!(i.GetDataHere);
assert!(i.QueryGetData);
assert!(i.GetCanonicalFormatEtc);
assert!(i.SetData);
assert!(i.EnumFormatEtc);
assert!(i.DAdvise);
assert!(i.DUnadvise);
assert!(i.EnumDAdvise);

fn EnumDAdvise(&self) -> Result<IEnumSTATDATA> {
panic!()
Ok(())
}
}
3 changes: 1 addition & 2 deletions tests/structs/tests/bstr.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use test_structs::Windows::Win32::System::Diagnostics::Debug::DebugPropertyInfo;
use windows::Abi;

#[test]
fn test() {
Expand All @@ -11,5 +10,5 @@ fn test() {
assert_eq!(a, b);
assert_eq!(a.m_bstrName, "Name");
assert_eq!(b.m_bstrName, "Name");
assert_ne!(a.m_bstrName.abi(), b.m_bstrName.abi());
assert_ne!(a.m_bstrName.0, b.m_bstrName.0);
}