Simplify signature parameter handling inside windows-bindgen (#3456)

crates/libs/bindgen/src/lib.rs

@@ -12,6 +12,7 @@ mod filter;
 mod guid;
 mod io;
 mod libraries;
+mod param;
 mod references;
 mod signature;
 mod tables;
@@ -30,6 +31,7 @@ use filter::*;
 use guid::*;
 use io::*;
 pub use libraries::*;
+use param::*;
 use references::*;
 use signature::*;
 use std::cmp::Ordering;

crates/libs/bindgen/src/param.rs

@@ -0,0 +1,69 @@
+use super::*;
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct Param {
+    pub def: MethodParam,
+    pub ty: Type,
+}
+
+impl std::ops::Deref for Param {
+    type Target = Type;
+
+    fn deref(&self) -> &Self::Target {
+        &self.ty
+    }
+}
+
+impl Param {
+    pub fn is_convertible(&self) -> bool {
+        self.is_input() && self.ty.is_convertible()
+    }
+
+    pub fn is_input(&self) -> bool {
+        !self.def.flags().contains(ParamAttributes::Out)
+    }
+
+    pub fn is_optional(&self) -> bool {
+        self.def.flags().contains(ParamAttributes::Optional)
+            || self.def.has_attribute("ReservedAttribute")
+    }
+
+    pub fn is_retval(&self) -> bool {
+        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)
+        {
+            return false;
+        }
+
+        for attribute in self.def.attributes() {
+            if matches!(
+                attribute.name(),
+                "NativeArrayInfoAttribute" | "MemorySizeAttribute"
+            ) {
+                return false;
+            }
+        }
+
+        // If it's bigger than 128 bits, best to pass as a reference.
+        if self.ty.deref().size() > 16 {
+            return false;
+        }
+
+        true
+    }
+
+    pub fn write_ident(&self) -> TokenStream {
+        to_ident(&self.def.name().to_lowercase())
+    }
+}

crates/libs/bindgen/src/signature.rs

@@ -3,27 +3,44 @@ use super::*;
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Signature {
     pub call_flags: MethodCallAttributes,
-    pub return_type: (Type, Option<Param>),
-    pub params: Vec<(Type, Param)>,
+    pub return_type: Type,
+    pub params: Vec<Param>,
 }
 
 impl Signature {
     pub fn size(&self) -> usize {
         self.params
             .iter()
-            .fold(0, |sum, param| sum + std::cmp::max(4, param.0.size()))
+            .fold(0, |sum, param| sum + std::cmp::max(4, param.size()))
     }
 
     pub fn dependencies(&self, dependencies: &mut TypeMap) {
-        self.return_type.0.dependencies(dependencies);
-        self.params
-            .iter()
-            .for_each(|(ty, _)| ty.dependencies(dependencies));
+        self.types().for_each(|ty| ty.dependencies(dependencies));
     }
 
     pub fn types(&self) -> impl Iterator<Item = &Type> + '_ {
-        std::iter::once(&self.return_type.0)
-            .chain(self.params.iter().map(|(ty, _)| ty))
+        std::iter::once(&self.return_type)
+            .chain(self.params.iter().map(|param| &param.ty))
             .map(|ty| ty.decay())
     }
+
+    pub fn is_retval(&self) -> bool {
+        // First we check whether there's an explicit retval parameter.
+        if let Some(param) = self.params.last() {
+            if param.def.has_attribute("RetValAttribute") {
+                return true;
+            }
+        }
+
+        // Otherwise we check for heuristically for additional candidates.
+        if let Some(param) = self.params.last() {
+            if param.is_retval() {
+                return self.params[..self.params.len() - 1]
+                    .iter()
+                    .all(|param| param.is_input());
+            }
+        }
+
+        false
+    }
 }

crates/libs/bindgen/src/tables/method_def.rs

@@ -15,7 +15,7 @@ impl MethodDef {
         self.str(3)
     }
 
-    pub fn params(&self) -> RowIterator<Param> {
+    pub fn params(&self) -> RowIterator<MethodParam> {
         self.list(5)
     }
 
@@ -35,7 +35,6 @@ impl MethodDef {
         let call_flags = MethodCallAttributes(blob.read_usize() as u8);
         let _param_count = blob.read_usize();
         let mut return_type = Type::from_blob(&mut blob, None, generics);
-        let mut return_param = None;
 
         let mut params = vec![];
 
@@ -44,21 +43,18 @@ impl MethodDef {
                 if param.has_attribute("ConstAttribute") {
                     return_type = return_type.to_const_type();
                 }
-
-                return_param = Some(param);
             } else {
                 let param_is_const = param.has_attribute("ConstAttribute");
-                let param_is_output = param.flags().contains(ParamAttributes::Out);
+                let param_is_input = !param.flags().contains(ParamAttributes::Out);
                 let mut ty = Type::from_blob(&mut blob, None, generics);
 
-                if param_is_const || !param_is_output {
+                if param_is_const || param_is_input {
                     ty = ty.to_const_type();
                 }
-                if !param_is_output {
+
+                if param_is_input {
                     ty = ty.to_const_ptr();
-                }
 
-                if !param_is_output {
                     if let Some(attribute) = param.find_attribute("AssociatedEnumAttribute") {
                         if let Some((_, Value::Str(name))) = attribute.args().first() {
                             let overload = param.reader().unwrap_full_name(namespace, name);
@@ -68,13 +64,13 @@ impl MethodDef {
                     }
                 }
 
-                params.push((ty, param));
+                params.push(Param { def: param, ty });
             }
         }
 
         Signature {
             call_flags,
-            return_type: (return_type, return_param),
+            return_type,
             params,
         }
     }

crates/libs/bindgen/src/tables/param.rs → crates/libs/bindgen/src/tables/method_param.rs

@@ -1,12 +1,12 @@
 use super::*;
 
-impl std::fmt::Debug for Param {
+impl std::fmt::Debug for MethodParam {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_tuple("Param").field(&self.name()).finish()
+        f.debug_tuple("MethodParam").field(&self.name()).finish()
     }
 }
 
-impl Param {
+impl MethodParam {
     pub fn flags(&self) -> ParamAttributes {
         ParamAttributes(self.usize(0) as u16)
     }

crates/libs/bindgen/src/tables/mod.rs

@@ -9,9 +9,9 @@ mod impl_map;
 mod interface_impl;
 mod member_ref;
 mod method_def;
+mod method_param;
 mod module_ref;
 mod nested_class;
-mod param;
 mod type_def;
 mod type_ref;
 mod type_spec;
@@ -46,7 +46,7 @@ tables! {
     (MethodDef, 6)
     (ModuleRef, 12)
     (NestedClass, 13)
-    (Param, 7)
+    (MethodParam, 7)
     (TypeDef, 8)
     (TypeRef, 9)
     (TypeSpec, 10)

crates/libs/bindgen/src/tables/type_def.rs

@@ -44,7 +44,7 @@ impl TypeDef {
     pub fn generics(&self) -> Vec<Type> {
         self.file()
             .equal_range(2, TypeOrMethodDef::TypeDef(*self).encode())
-            .map(|generic: GenericParam| Type::Param(generic.name()))
+            .map(|generic: GenericParam| Type::Generic(generic.name()))
             .collect()
     }
 

crates/libs/bindgen/src/types/cpp_delegate.rs

@@ -51,8 +51,8 @@ impl CppDelegate {
 
         let mut params = quote! {};
 
-        for (ty, param) in &signature.params {
-            params.combine(write_param(writer, ty, *param));
+        for param in &signature.params {
+            params.combine(write_param(writer, param));
         }
 
         let return_sig = writer.write_return_sig(method, &signature, false);
@@ -73,24 +73,28 @@ impl CppDelegate {
     }
 }
 
-fn write_param(writer: &Writer, ty: &Type, param: Param) -> TokenStream {
-    let name = to_ident(&param.name().to_lowercase());
-    let type_name = ty.write_name(writer);
+fn write_param(writer: &Writer, param: &Param) -> TokenStream {
+    let name = param.write_ident();
+    let type_name = param.write_name(writer);
 
     if writer.config.sys {
         return quote! { #name: #type_name, };
     }
 
-    if param.flags().contains(ParamAttributes::Out) {
-        if ty.deref().is_interface() {
-            let type_name = ty.deref().write_name(writer);
-            quote! { #name: windows_core::OutRef<'_, #type_name>, }
+    if param.is_input() {
+        if param.is_copyable() {
+            return quote! { #name: #type_name, };
         } else {
-            quote! { #name: #type_name, }
+            return quote! { #name: windows_core::Ref<'_, #type_name>, };
         }
-    } else if ty.is_copyable() {
-        quote! { #name: #type_name, }
+    }
+
+    let deref = param.deref();
+
+    if deref.is_interface() {
+        let type_name = deref.write_name(writer);
+        quote! { #name: windows_core::OutRef<'_, #type_name>, }
     } else {
-        quote! { #name: windows_core::Ref<'_, #type_name>, }
+        quote! { #name: #type_name, }
     }
 }

crates/libs/bindgen/src/types/cpp_fn.rs

@@ -50,12 +50,12 @@ impl CppFn {
 
         let signature = self.method.signature(self.namespace, &[]);
 
-        let params = signature.params.iter().map(|(ty, param)| {
-            let name = to_ident(&param.name().to_lowercase());
+        let params = signature.params.iter().map(|param| {
+            let name = param.write_ident();
             let ty = if underlying_types {
-                ty.underlying_type().write_abi(writer)
+                param.underlying_type().write_abi(writer)
             } else {
-                ty.write_abi(writer)
+                param.write_abi(writer)
             };
             quote! { #name: #ty }
         });
@@ -143,7 +143,7 @@ impl CppFn {
             }
             ReturnHint::ResultValue => {
                 let where_clause = method.write_where(writer, false);
-                let return_type = signature.params[signature.params.len() - 1].0.deref();
+                let return_type = signature.params[signature.params.len() - 1].deref();
                 let map = return_type.write_result_map();
                 let return_type = return_type.write_name(writer);
 
@@ -174,9 +174,8 @@ impl CppFn {
             ReturnHint::ReturnValue => {
                 let where_clause = method.write_where(writer, false);
 
-                let return_type = method.signature.params[method.signature.params.len() - 1]
-                    .0
-                    .deref();
+                let return_type =
+                    method.signature.params[method.signature.params.len() - 1].deref();
 
                 if return_type.is_interface() {
                     let return_type = return_type.write_name(writer);
@@ -221,7 +220,7 @@ impl CppFn {
                 let where_clause = method.write_where(writer, false);
 
                 if method.handle_last_error() {
-                    let return_type = signature.return_type.0.write_name(writer);
+                    let return_type = signature.return_type.write_name(writer);
 
                     quote! {
                         #cfg
@@ -302,7 +301,7 @@ impl Writer {
         signature: &Signature,
         underlying_types: bool,
     ) -> TokenStream {
-        match &signature.return_type.0 {
+        match &signature.return_type {
             Type::Void => {
                 if method.has_attribute("DoesNotReturnAttribute") {
                     quote! { -> ! }