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

Small bindgen refactor and tools refresh #2695

Merged
merged 6 commits into from
Nov 6, 2023
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
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