Skip to content

Commit

Permalink
Small bindgen refactor and tools refresh (#2695)
Browse files Browse the repository at this point in the history
  • Loading branch information
kennykerr authored Nov 6, 2023
1 parent f6fe766 commit ab61477
Show file tree
Hide file tree
Showing 16 changed files with 109 additions and 109 deletions.
167 changes: 86 additions & 81 deletions crates/libs/bindgen/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,6 @@ pub enum SignatureParamKind {
Other,
}

impl SignatureParamKind {
fn is_array(&self) -> bool {
matches!(self, Self::ArrayFixed(_) | Self::ArrayRelativeLen(_) | Self::ArrayRelativeByteLen(_) | Self::ArrayRelativePtr(_))
}
}

pub struct Signature {
pub def: MethodDef,
pub params: Vec<SignatureParam>,
Expand Down Expand Up @@ -113,6 +107,89 @@ impl std::fmt::Debug for Guid {
}
}

impl SignatureParamKind {
fn is_array(&self) -> bool {
matches!(self, Self::ArrayFixed(_) | Self::ArrayRelativeLen(_) | Self::ArrayRelativeByteLen(_) | Self::ArrayRelativePtr(_))
}
}

impl SignatureParam {
pub fn is_convertible(&self) -> bool {
!self.def.flags().contains(ParamAttributes::Out) && !self.ty.is_winrt_array() && !self.ty.is_pointer() && !self.kind.is_array() && (type_is_borrowed(&self.ty) || type_is_non_exclusive_winrt_interface(&self.ty) || type_is_trivially_convertible(&self.ty))
}

fn is_retval(&self) -> bool {
// The Win32 metadata uses `RetValAttribute` to call out retval methods but it is employed
// very sparingly, so this heuristic is used to apply the transformation more uniformly.
if self.def.has_attribute("RetValAttribute") {
return true;
}
if !self.ty.is_pointer() {
return false;
}
if self.ty.is_void() {
return false;
}
let flags = self.def.flags();
if flags.contains(ParamAttributes::In) || !flags.contains(ParamAttributes::Out) || flags.contains(ParamAttributes::Optional) || self.kind.is_array() {
return false;
}
if param_kind(self.def).is_array() {
return false;
}
// If it's bigger than 128 bits, best to pass as a reference.
if self.ty.deref().size() > 16 {
return false;
}
// Win32 callbacks are defined as `Option<T>` so we don't include them here to avoid
// producing the `Result<Option<T>>` anti-pattern.
match self.ty.deref() {
Type::TypeDef(def, _) => !type_def_is_callback(def),
_ => true,
}
}
}

impl Signature {
pub fn kind(&self) -> SignatureKind {
if self.def.has_attribute("CanReturnMultipleSuccessValuesAttribute") {
return SignatureKind::PreserveSig;
}
match &self.return_type {
Type::Void if self.is_retval() => SignatureKind::ReturnValue,
Type::Void => SignatureKind::ReturnVoid,
Type::HRESULT => {
if self.params.len() >= 2 {
if let Some((guid, object)) = signature_param_is_query(&self.params) {
if self.params[object].def.flags().contains(ParamAttributes::Optional) {
return SignatureKind::QueryOptional(QueryPosition { object, guid });
} else {
return SignatureKind::Query(QueryPosition { object, guid });
}
}
}
if self.is_retval() {
SignatureKind::ResultValue
} else {
SignatureKind::ResultVoid
}
}
Type::TypeDef(def, _) if def.type_name() == TypeName::WIN32_ERROR => SignatureKind::ResultVoid,
Type::TypeDef(def, _) if def.type_name() == TypeName::BOOL && method_def_last_error(self.def) => SignatureKind::ResultVoid,
_ if type_is_struct(&self.return_type) => SignatureKind::ReturnStruct,
_ => SignatureKind::PreserveSig,
}
}

fn is_retval(&self) -> bool {
self.params.last().map_or(false, |param| param.is_retval())
&& self.params[..self.params.len() - 1].iter().all(|param| {
let flags = param.def.flags();
!flags.contains(ParamAttributes::Out)
})
}
}

pub fn type_def_invoke_method(row: TypeDef) -> MethodDef {
row.methods().find(|method| method.name() == "Invoke").expect("`Invoke` method not found")
}
Expand Down Expand Up @@ -214,7 +291,7 @@ pub fn method_def_signature(namespace: &str, row: MethodDef, generics: &[Type])

for param in &mut params {
if param.kind == SignatureParamKind::Other {
if signature_param_is_convertible(param) {
if param.is_convertible() {
if type_is_non_exclusive_winrt_interface(&param.ty) {
param.kind = SignatureParamKind::TryInto;
} else {
Expand Down Expand Up @@ -273,79 +350,6 @@ fn param_or_enum(row: Param) -> Option<String> {
})
}

pub fn signature_param_is_convertible(param: &SignatureParam) -> bool {
!param.def.flags().contains(ParamAttributes::Out) && !param.ty.is_winrt_array() && !param.ty.is_pointer() && !param.kind.is_array() && (type_is_borrowed(&param.ty) || type_is_non_exclusive_winrt_interface(&param.ty) || type_is_trivially_convertible(&param.ty))
}

fn signature_param_is_retval(param: &SignatureParam) -> bool {
// The Win32 metadata uses `RetValAttribute` to call out retval methods but it is employed
// very sparingly, so this heuristic is used to apply the transformation more uniformly.
if param.def.has_attribute("RetValAttribute") {
return true;
}
if !param.ty.is_pointer() {
return false;
}
if param.ty.is_void() {
return false;
}
let flags = param.def.flags();
if flags.contains(ParamAttributes::In) || !flags.contains(ParamAttributes::Out) || flags.contains(ParamAttributes::Optional) || param.kind.is_array() {
return false;
}
if param_kind(param.def).is_array() {
return false;
}
// If it's bigger than 128 bits, best to pass as a reference.
if param.ty.deref().size() > 16 {
return false;
}
// Win32 callbacks are defined as `Option<T>` so we don't include them here to avoid
// producing the `Result<Option<T>>` anti-pattern.
match param.ty.deref() {
Type::TypeDef(def, _) => !type_def_is_callback(def),
_ => true,
}
}

pub fn signature_kind(signature: &Signature) -> SignatureKind {
if signature.def.has_attribute("CanReturnMultipleSuccessValuesAttribute") {
return SignatureKind::PreserveSig;
}
match &signature.return_type {
Type::Void if signature_is_retval(signature) => SignatureKind::ReturnValue,
Type::Void => SignatureKind::ReturnVoid,
Type::HRESULT => {
if signature.params.len() >= 2 {
if let Some((guid, object)) = signature_param_is_query(&signature.params) {
if signature.params[object].def.flags().contains(ParamAttributes::Optional) {
return SignatureKind::QueryOptional(QueryPosition { object, guid });
} else {
return SignatureKind::Query(QueryPosition { object, guid });
}
}
}
if signature_is_retval(signature) {
SignatureKind::ResultValue
} else {
SignatureKind::ResultVoid
}
}
Type::TypeDef(def, _) if def.type_name() == TypeName::WIN32_ERROR => SignatureKind::ResultVoid,
Type::TypeDef(def, _) if def.type_name() == TypeName::BOOL && method_def_last_error(signature.def) => SignatureKind::ResultVoid,
_ if type_is_struct(&signature.return_type) => SignatureKind::ReturnStruct,
_ => SignatureKind::PreserveSig,
}
}

fn signature_is_retval(signature: &Signature) -> bool {
signature.params.last().map_or(false, signature_param_is_retval)
&& signature.params[..signature.params.len() - 1].iter().all(|param| {
let flags = param.def.flags();
!flags.contains(ParamAttributes::Out)
})
}

fn signature_param_is_query(params: &[SignatureParam]) -> Option<(usize, usize)> {
if let Some(guid) = params.iter().rposition(|param| param.ty == Type::ConstPtr(Box::new(Type::GUID), 1) && !param.def.flags().contains(ParamAttributes::Out)) {
if let Some(object) = params.iter().rposition(|param| param.ty == Type::MutPtr(Box::new(Type::Void), 2) && param.def.has_attribute("ComOutPtrAttribute")) {
Expand Down Expand Up @@ -411,6 +415,7 @@ pub fn type_has_callback(ty: &Type) -> bool {
_ => false,
}
}

pub fn type_def_has_callback(row: TypeDef) -> bool {
if type_def_is_callback(row) {
return true;
Expand Down Expand Up @@ -761,7 +766,7 @@ pub fn type_def_invalid_values(row: TypeDef) -> Vec<i64> {
let mut values = Vec::new();
for attribute in row.attributes() {
if attribute.name() == "InvalidHandleValueAttribute" {
if let Some((_, Value::I64(value))) = attribute.args().get(0) {
if let Some((_, Value::I64(value))) = attribute.args().first() {
values.push(*value);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/bindgen/src/rust/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn cfg_add_attributes<R: AsRow + Into<HasAttribute>>(cfg: &mut Cfg, row: R) {
for attribute in row.attributes() {
match attribute.name() {
"SupportedArchitectureAttribute" => {
if let Some((_, Value::EnumDef(_, value))) = attribute.args().get(0) {
if let Some((_, Value::EnumDef(_, value))) = attribute.args().first() {
if let Value::I32(value) = **value {
if value & 1 == 1 {
cfg.arches.insert("x86");
Expand Down
4 changes: 2 additions & 2 deletions crates/libs/bindgen/src/rust/com_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub fn writer(writer: &Writer, def: TypeDef, kind: InterfaceKind, method: Method
bases.combine(&quote! { .base__ });
}

let kind = signature_kind(&signature);
let kind = signature.kind();
match kind {
SignatureKind::Query(_) => {
let args = writer.win32_args(&signature.params, kind);
Expand Down Expand Up @@ -153,7 +153,7 @@ pub fn writer(writer: &Writer, def: TypeDef, kind: InterfaceKind, method: Method
}

pub fn gen_upcall(writer: &Writer, sig: &Signature, inner: TokenStream) -> TokenStream {
match signature_kind(sig) {
match sig.kind() {
SignatureKind::ResultValue => {
let invoke_args = sig.params[..sig.params.len() - 1].iter().map(|param| gen_win32_invoke_arg(writer, param));

Expand Down
2 changes: 1 addition & 1 deletion crates/libs/bindgen/src/rust/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ fn field_guid(row: Field) -> Option<Guid> {
}

fn field_is_ansi(row: Field) -> bool {
row.find_attribute("NativeEncodingAttribute").is_some_and(|attribute| matches!(attribute.args().get(0), Some((_, Value::String(encoding))) if encoding == "ansi"))
row.find_attribute("NativeEncodingAttribute").is_some_and(|attribute| matches!(attribute.args().first(), Some((_, Value::String(encoding))) if encoding == "ansi"))
}

fn type_has_replacement(ty: &Type) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/bindgen/src/rust/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn gen_win_function(writer: &Writer, namespace: &str, def: MethodDef) -> TokenSt
let features = writer.cfg_features(&cfg);
let link = gen_link(writer, namespace, &signature, &cfg);

let kind = signature_kind(&signature);
let kind = signature.kind();
match kind {
SignatureKind::Query(_) => {
let args = writer.win32_args(&signature.params, kind);
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/bindgen/src/rust/handles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub fn gen_win_handle(writer: &Writer, def: TypeDef) -> TokenStream {

fn type_def_usable_for(row: TypeDef) -> Option<TypeDef> {
if let Some(attribute) = row.find_attribute("AlsoUsableForAttribute") {
if let Some((_, Value::String(name))) = attribute.args().get(0) {
if let Some((_, Value::String(name))) = attribute.args().first() {
return row.reader().get_type_def(row.namespace(), name.as_str()).next();
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/bindgen/src/rust/winrt_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ fn gen_winrt_params(writer: &Writer, params: &[SignatureParam]) -> TokenStream {
if param.def.flags().contains(ParamAttributes::In) {
if param.ty.is_winrt_array() {
result.combine(&quote! { #name: &[#default_type], });
} else if signature_param_is_convertible(param) {
} else if param.is_convertible() {
let (position, _) = generic_params.next().unwrap();
let kind: TokenStream = format!("P{position}").into();
result.combine(&quote! { #name: #kind, });
Expand Down
6 changes: 3 additions & 3 deletions crates/libs/bindgen/src/rust/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ impl Writer {
}
/// The signature params which are generic (along with their relative index)
pub fn generic_params<'b>(&'b self, params: &'b [SignatureParam]) -> impl Iterator<Item = (usize, &SignatureParam)> + 'b {
params.iter().filter(move |param| signature_param_is_convertible(param)).enumerate()
params.iter().filter(move |param| param.is_convertible()).enumerate()
}
/// The generic param names (i.e., `T` in `fn foo<T>()`)
pub fn constraint_generics(&self, params: &[SignatureParam]) -> TokenStream {
Expand Down Expand Up @@ -1070,7 +1070,7 @@ impl Writer {

quote! { (#this #(#params),*) -> ::windows_core::Result<#return_type> }
} else {
let signature_kind = signature_kind(signature);
let signature_kind = signature.kind();
let mut params = quote! {};

if signature_kind == SignatureKind::ResultValue {
Expand Down Expand Up @@ -1207,7 +1207,7 @@ fn type_def_is_agile(row: TypeDef) -> bool {
match attribute.name() {
"AgileAttribute" => return true,
"MarshalingBehaviorAttribute" => {
if let Some((_, Value::EnumDef(_, value))) = attribute.args().get(0) {
if let Some((_, Value::EnumDef(_, value))) = attribute.args().first() {
if let Value::I32(2) = **value {
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/bindgen/src/winmd/writer/type.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(dead_code, clippy::upper_case_acronyms)]
#![allow(dead_code, clippy::upper_case_acronyms, clippy::enum_variant_names)]

#[derive(Clone, Debug, Hash, PartialEq, Eq)]
pub struct TypeName {
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/core/src/imp/factory_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ mod tests {
results.push(unsafe { library.to_string().unwrap() });
crate::Result::<()>::Err(crate::Error::OK)
});
assert!(matches!(end_result, None));
assert!(end_result.is_none());
assert_eq!(results, vec!["A.B.dll", "A.dll"]);
}
}
4 changes: 0 additions & 4 deletions crates/libs/core/src/strings/literals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,6 @@ macro_rules! h {
}};
}

pub use h;
pub use s;
pub use w;

#[doc(hidden)]
pub const fn decode_utf8_char(bytes: &[u8], mut pos: usize) -> Option<(u32, usize)> {
if bytes.len() == pos {
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/core/src/strings/pcstr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ mod tests {
#[test]
fn can_display() {
// 💖 followed by an invalid byte sequence and then an incomplete one
let s = vec![240, 159, 146, 150, 255, 240, 159, 0];
let s = [240, 159, 146, 150, 255, 240, 159, 0];
let s = PCSTR::from_raw(s.as_ptr());
assert_eq!("💖�", format!("{}", unsafe { s.display() }));
}
Expand Down
2 changes: 1 addition & 1 deletion crates/libs/implement/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pub fn implement(attributes: proc_macro::TokenStream, original_type: proc_macro:
let attributes = syn::parse_macro_input!(attributes as ImplementAttributes);
let interfaces_len = proc_macro2::Literal::usize_unsuffixed(attributes.implement.len());

let identity_type = if let Some(first) = attributes.implement.get(0) {
let identity_type = if let Some(first) = attributes.implement.first() {
first.to_ident()
} else {
quote! { ::windows::core::IInspectable }
Expand Down
4 changes: 2 additions & 2 deletions crates/libs/sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ Learn more about Rust for Windows here: <https://github.com/microsoft/windows-rs
#![cfg_attr(not(feature = "docs"), doc(hidden))]

extern crate self as windows_sys;
mod Windows;
pub mod core;
pub use Windows::*;

include!("Windows/mod.rs");
4 changes: 2 additions & 2 deletions crates/libs/windows/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ Learn more about Rust for Windows here: <https://github.com/microsoft/windows-rs
#![cfg_attr(not(feature = "docs"), doc(hidden))]

extern crate self as windows;
pub use Windows::*;
mod Windows;

pub mod core {
pub use windows_core::*;
Expand All @@ -23,3 +21,5 @@ pub mod core {
#[cfg(feature = "implement")]
pub use windows_interface::interface;
}

include!("Windows/mod.rs");
Loading

0 comments on commit ab61477

Please sign in to comment.