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

Support fixed-size array parameters #1941

Merged
merged 4 commits into from Jul 28, 2022
Merged

Support fixed-size array parameters #1941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2bbea7d
Add support for fixed arrays
kennykerr Jul 27, 2022
c503531
Add and update samples and tests
kennykerr Jul 27, 2022
b337a8a
code gen
kennykerr Jul 27, 2022
cc3e618
build validation
kennykerr Jul 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Expand Up @@ -136,6 +136,7 @@ jobs:
cargo clippy -p test_alternate_success_code &&
cargo clippy -p test_arch &&
cargo clippy -p test_arch_feature &&
cargo clippy -p test_bcrypt &&
cargo clippy -p test_bstr &&
cargo clippy -p test_cfg_generic &&
cargo clippy -p test_class_factory &&
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Expand Up @@ -119,15 +119,16 @@ jobs:
cargo test --target ${{ matrix.target }} -p test_alternate_success_code &&
cargo test --target ${{ matrix.target }} -p test_arch &&
cargo test --target ${{ matrix.target }} -p test_arch_feature &&
cargo test --target ${{ matrix.target }} -p test_bcrypt &&
cargo test --target ${{ matrix.target }} -p test_bstr &&
cargo test --target ${{ matrix.target }} -p test_cfg_generic &&
cargo test --target ${{ matrix.target }} -p test_class_factory &&
cargo test --target ${{ matrix.target }} -p test_component &&
cargo test --target ${{ matrix.target }} -p test_component_client &&
cargo test --target ${{ matrix.target }} -p test_const_fields &&
cargo test --target ${{ matrix.target }} -p test_core &&
cargo test --target ${{ matrix.target }} -p test_data_object &&
cargo clean &&
cargo test --target ${{ matrix.target }} -p test_data_object &&
cargo test --target ${{ matrix.target }} -p test_debug &&
cargo test --target ${{ matrix.target }} -p test_deprecated &&
cargo test --target ${{ matrix.target }} -p test_dispatch &&
Expand Down
55 changes: 37 additions & 18 deletions crates/libs/bindgen/src/gen.rs
View file
Open in desktop
Expand Up @@ -922,23 +922,23 @@ impl<'a> Gen<'a> {
_ => {
let name = self.param_name(param.def);
match param.array_info {
ArrayInfo::Fixed(fixed) if fixed > 0 && self.reader.param_free_with(param.def).is_none() => {
if self.reader.param_flags(param.def).output() {
quote! { ::core::mem::transmute(::windows::core::as_mut_ptr_or_null(#name)), }
ArrayInfo::Fixed(_) | ArrayInfo::RelativeLen(_) | ArrayInfo::RelativeByteLen(_) => {
let flags = self.reader.param_flags(param.def);
let map = if flags.optional() {
quote! { #name.as_deref().map_or(::core::ptr::null(), |slice|slice.as_ptr()) }
} else {
quote! { ::core::mem::transmute(::windows::core::as_ptr_or_null(#name)), }
}
}
ArrayInfo::RelativeLen(_) => {
if self.reader.param_flags(param.def).output() {
quote! { ::core::mem::transmute(::windows::core::as_mut_ptr_or_null(#name)), }
} else {
quote! { ::core::mem::transmute(::windows::core::as_ptr_or_null(#name)), }
}
quote! { #name.as_ptr() }
};
quote! { ::core::mem::transmute(#map), }
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we know this is a pointer does as _ work instead? Would be nice to avoid another use of transmute.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not just yet - it still requires a bit more untangling before that will work. There are a few cases where the underlying types aren't simply the pointer equivalent of the slice. But I'd love to get there.

}
ArrayInfo::RelativePtr(relative) => {
let name = self.param_name(params[relative].def);
quote! { #name.len() as _, }
let flags = self.reader.param_flags(params[relative].def);
if flags.optional() {
quote! { #name.as_deref().map_or(0, |slice|slice.len() as _), }
} else {
quote! { #name.len() as _, }
}
}
_ => {
if self.reader.signature_param_input_value(param) {
Expand Down Expand Up @@ -986,14 +986,16 @@ impl<'a> Gen<'a> {
let ty = param.ty.deref();
let ty = self.type_default_name(&ty);
let len = Literal::u32_unsuffixed(fixed as _);

let ty = if self.reader.param_flags(param.def).output() {
quote! { &mut [#ty; #len] }
} else {
quote! { &[#ty; #len] }
};

tokens.combine(&quote! { #name: #ty, });
if self.reader.param_flags(param.def).optional() {
tokens.combine(&quote! { #name: ::core::option::Option<#ty>, });
} else {
tokens.combine(&quote! { #name: #ty, });
}
continue;
}
}
Expand All @@ -1006,8 +1008,25 @@ impl<'a> Gen<'a> {
} else {
quote! { &[#ty] }
};
if self.reader.param_flags(param.def).optional() {
tokens.combine(&quote! { #name: ::core::option::Option<#ty>, });
} else {
tokens.combine(&quote! { #name: #ty, });
}
continue;
}

tokens.combine(&quote! { #name: #ty, });
if let ArrayInfo::RelativeByteLen(_) = param.array_info {
let ty = if self.reader.param_flags(param.def).output() {
quote! { &mut [u8] }
} else {
quote! { &[u8] }
};
if self.reader.param_flags(param.def).optional() {
tokens.combine(&quote! { #name: ::core::option::Option<#ty>, });
} else {
tokens.combine(&quote! { #name: #ty, });
}
continue;
}

Expand All @@ -1022,7 +1041,7 @@ impl<'a> Gen<'a> {
continue;
}

if param.ty.is_pointer() && !param.ty.is_void() {
if param.ty.is_pointer() && !param.ty.is_void() && param.array_info != ArrayInfo::Removed {
let param_flags = self.reader.param_flags(param.def);
let kind = self.type_default_name(&param.ty.deref());
let kind = if param_flags.output() {
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/bindgen/src/replacements/bstr.rs
View file
Open in desktop
Expand Up @@ -26,7 +26,7 @@ pub fn gen() -> TokenStream {
return Self(::core::ptr::null_mut());
}

unsafe { SysAllocStringLen(value) }
unsafe { SysAllocStringLen(Some(value)) }
}

pub fn as_wide(&self) -> &[u16] {
Expand Down
47 changes: 34 additions & 13 deletions crates/libs/metadata/src/reader/mod.rs
View file
Open in desktop
Expand Up @@ -45,6 +45,7 @@ tables! {
pub enum ArrayInfo {
Fixed(usize),
RelativeLen(usize),
RelativeByteLen(usize),
RelativePtr(usize),
None,
Removed,
Expand Down Expand Up @@ -543,22 +544,32 @@ impl<'a> Reader<'a> {

for position in 0..params.len() {
// Point len params back to the corresponding ptr params.
if let ArrayInfo::RelativeLen(relative) = params[position].array_info {
// The len params must be input only.
if !self.param_flags(params[relative].def).output() && position != relative {
params[relative].array_info = ArrayInfo::RelativePtr(position);
} else {
match params[position].array_info {
ArrayInfo::RelativeLen(relative) | ArrayInfo::RelativeByteLen(relative) => {
// The len params must be input only.
if !self.param_flags(params[relative].def).output() && position != relative && !params[relative].ty.is_pointer() {
params[relative].array_info = ArrayInfo::RelativePtr(position);
} else {
params[position].array_info = ArrayInfo::Removed;
}
}
// TODO: workaround for https://github.com/microsoft/win32metadata/issues/1014
ArrayInfo::Fixed(fixed) if fixed == 0 || self.param_free_with(params[position].def).is_some() => {
params[position].array_info = ArrayInfo::Removed;
}
_ => {}
}
}

let mut sets = BTreeMap::<usize, Vec<usize>>::new();

// Finds sets of ptr params pointing at the same len param.
for (position, param) in params.iter().enumerate() {
if let ArrayInfo::RelativeLen(relative) = param.array_info {
sets.entry(relative).or_default().push(position);
match param.array_info {
ArrayInfo::RelativeLen(relative) | ArrayInfo::RelativeByteLen(relative) => {
sets.entry(relative).or_default().push(position);
}
_ => {}
}
}

Expand Down Expand Up @@ -629,14 +640,24 @@ impl<'a> Reader<'a> {
}
pub fn param_array_info(&self, row: Param) -> ArrayInfo {
for attribute in self.param_attributes(row) {
if self.attribute_name(attribute) == "NativeArrayInfoAttribute" {
for (_, value) in self.attribute_args(attribute) {
match value {
Value::I16(value) => return ArrayInfo::RelativeLen(value as _),
Value::I32(value) => return ArrayInfo::Fixed(value as _),
_ => {}
match self.attribute_name(attribute) {
"NativeArrayInfoAttribute" => {
for (_, value) in self.attribute_args(attribute) {
match value {
Value::I16(value) => return ArrayInfo::RelativeLen(value as _),
Value::I32(value) => return ArrayInfo::Fixed(value as _),
_ => {}
}
}
}
"MemorySizeAttribute" => {
for (_, value) in self.attribute_args(attribute) {
if let Value::I16(value) = value {
return ArrayInfo::RelativeByteLen(value as _);
}
}
}
_ => {}
}
}
ArrayInfo::None
Expand Down