Simplify dependency tracking (#3460)

microsoft · Jan 22, 2025 · 493d5da · 493d5da
1 parent cbcfa6e
commit 493d5da
Show file tree

Hide file tree

Showing 16 changed files with 249 additions and 241 deletions.
diff --git a/crates/libs/bindgen/src/signature.rs b/crates/libs/bindgen/src/signature.rs
@@ -14,10 +14,6 @@ impl Signature {
             .fold(0, |sum, param| sum + std::cmp::max(4, param.size()))
     }
 
-    pub fn dependencies(&self, dependencies: &mut TypeMap) {
-        self.types().for_each(|ty| ty.dependencies(dependencies));
-    }
-
     pub fn types(&self) -> impl Iterator<Item = &Type> + '_ {
         std::iter::once(&self.return_type)
             .chain(self.params.iter().map(|param| &param.ty))
@@ -44,3 +40,9 @@ impl Signature {
         false
     }
 }
+
+impl Dependencies for Signature {
+    fn combine(&self, dependencies: &mut TypeMap) {
+        self.types().for_each(|ty| ty.combine(dependencies));
+    }
+}
diff --git a/crates/libs/bindgen/src/tables/method_def.rs b/crates/libs/bindgen/src/tables/method_def.rs
@@ -15,6 +15,17 @@ impl MethodDef {
         self.str(3)
     }
 
+    pub fn import_name(&self) -> Option<&'static str> {
+        self.impl_map().and_then(|map| {
+            let import_name = map.import_name();
+            if self.name() != import_name {
+                Some(import_name)
+            } else {
+                None
+            }
+        })
+    }
+
     pub fn params(&self) -> RowIterator<MethodParam> {
         self.list(5)
     }
@@ -29,6 +40,20 @@ impl MethodDef {
         self.impl_map().map_or("", |map| map.scope().name())
     }
 
+    pub fn calling_convention(&self) -> &'static str {
+        self.impl_map().map_or("", |map| {
+            let flags = map.flags();
+
+            if flags.contains(PInvokeAttributes::CallConvPlatformapi) {
+                "system"
+            } else if flags.contains(PInvokeAttributes::CallConvCdecl) {
+                "cdecl"
+            } else {
+                ""
+            }
+        })
+    }
+
     #[track_caller]
     pub fn signature(&self, namespace: &str, generics: &[Type]) -> Signature {
         let mut blob = self.blob(4);

diff --git a/crates/libs/bindgen/src/type_map.rs b/crates/libs/bindgen/src/type_map.rs
@@ -3,6 +3,16 @@ use super::*;
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct TypeMap(HashMap<TypeName, HashSet<Type>>);
 
+pub trait Dependencies {
+    fn combine(&self, dependencies: &mut TypeMap);
+
+    fn dependencies(&self) -> TypeMap {
+        let mut dependencies = TypeMap::new();
+        self.combine(&mut dependencies);
+        dependencies
+    }
+}
+
 impl std::ops::Deref for TypeMap {
     type Target = HashMap<TypeName, HashSet<Type>>;
 
@@ -27,7 +37,7 @@ impl TypeMap {
                         let mut item_dependencies = Self::new();
 
                         for ty in types {
-                            ty.dependencies(&mut item_dependencies);
+                            ty.combine(&mut item_dependencies);
                         }
 
                         if item_dependencies.excluded(filter) {

diff --git a/crates/libs/bindgen/src/types/class.rs b/crates/libs/bindgen/src/types/class.rs
@@ -15,9 +15,7 @@ impl Class {
             return (Cfg::default(), quote! {});
         }
 
-        let mut dependencies = TypeMap::new();
-        self.dependencies(&mut dependencies);
-        let cfg = Cfg::new(self.def, &dependencies);
+        let cfg = Cfg::new(self.def, &self.dependencies());
         let tokens = cfg.write(writer, false);
         (cfg, tokens)
     }
@@ -92,10 +90,7 @@ impl Class {
                         let interface_type = interface.write_name(writer);
 
                         let cfg = if writer.config.package {
-                            let mut dependencies = TypeMap::new();
-                            interface.dependencies(&mut dependencies);
-
-                            class_cfg.difference(interface.def, &dependencies).write(writer, false)
+                            class_cfg.difference(interface.def, &interface.dependencies()).write(writer, false)
                         } else {
                             quote! {}
                         };
@@ -252,12 +247,6 @@ impl Class {
         )
     }
 
-    pub fn dependencies(&self, dependencies: &mut TypeMap) {
-        for interface in self.required_interfaces() {
-            Type::Interface(interface).dependencies(dependencies);
-        }
-    }
-
     fn bases(&self) -> Vec<Self> {
         let mut bases = Vec::new();
         let mut def = self.def;
@@ -356,3 +345,11 @@ impl Class {
             })
     }
 }
+
+impl Dependencies for Class {
+    fn combine(&self, dependencies: &mut TypeMap) {
+        for interface in self.required_interfaces() {
+            Type::Interface(interface).combine(dependencies);
+        }
+    }
+}
diff --git a/crates/libs/bindgen/src/types/cpp_const.rs b/crates/libs/bindgen/src/types/cpp_const.rs
@@ -32,9 +32,7 @@ impl CppConst {
             return quote! {};
         }
 
-        let mut dependencies = TypeMap::new();
-        self.dependencies(&mut dependencies);
-        Cfg::new(self.field, &dependencies).write(writer, false)
+        Cfg::new(self.field, &self.dependencies()).write(writer, false)
     }
 
     pub fn write(&self, writer: &Writer) -> TokenStream {
@@ -137,12 +135,11 @@ impl CppConst {
             panic!()
         }
     }
+}
 
-    pub fn dependencies(&self, dependencies: &mut TypeMap) {
-        self.field
-            .ty(None)
-            .to_const_type()
-            .dependencies(dependencies);
+impl Dependencies for CppConst {
+    fn combine(&self, dependencies: &mut TypeMap) {
+        self.field.ty(None).to_const_type().combine(dependencies);
     }
 }
 

diff --git a/crates/libs/bindgen/src/types/cpp_delegate.rs b/crates/libs/bindgen/src/types/cpp_delegate.rs
@@ -38,9 +38,7 @@ impl CppDelegate {
             return quote! {};
         }
 
-        let mut dependencies = TypeMap::new();
-        self.dependencies(&mut dependencies);
-        Cfg::new(self.def, &dependencies).write(writer, false)
+        Cfg::new(self.def, &self.dependencies()).write(writer, false)
     }
 
     pub fn write(&self, writer: &Writer) -> TokenStream {
@@ -65,11 +63,13 @@ impl CppDelegate {
             pub type #name = Option<unsafe extern "system" fn(#params) #return_sig>;
         }
     }
+}
 
-    pub fn dependencies(&self, dependencies: &mut TypeMap) {
+impl Dependencies for CppDelegate {
+    fn combine(&self, dependencies: &mut TypeMap) {
         self.method()
             .signature(self.def.namespace(), &[])
-            .dependencies(dependencies);
+            .combine(dependencies);
     }
 }
 

diff --git a/crates/libs/bindgen/src/types/cpp_enum.rs b/crates/libs/bindgen/src/types/cpp_enum.rs
@@ -124,22 +124,24 @@ impl CppEnum {
         }
     }
 
-    pub fn dependencies(&self, dependencies: &mut TypeMap) {
+    pub fn size(&self) -> usize {
+        self.def.underlying_type().size()
+    }
+
+    pub fn align(&self) -> usize {
+        self.def.underlying_type().align()
+    }
+}
+
+impl Dependencies for CppEnum {
+    fn combine(&self, dependencies: &mut TypeMap) {
         if let Some(attribute) = self.def.find_attribute("AlsoUsableForAttribute") {
             if let Some((_, Value::Str(type_name))) = attribute.args().first() {
                 self.def
                     .reader()
                     .unwrap_full_name(self.def.namespace(), type_name)
-                    .dependencies(dependencies);
+                    .combine(dependencies);
             }
         }
     }
-
-    pub fn size(&self) -> usize {
-        self.def.underlying_type().size()
-    }
-
-    pub fn align(&self) -> usize {
-        self.def.underlying_type().align()
-    }
 }
diff --git a/crates/libs/bindgen/src/types/cpp_fn.rs b/crates/libs/bindgen/src/types/cpp_fn.rs
@@ -28,26 +28,10 @@ impl CppFn {
     }
 
     pub fn write_link(&self, writer: &Writer, underlying_types: bool) -> TokenStream {
-        let name = self.method.name();
         let library = self.method.module_name().to_lowercase();
-        let impl_map = self.method.impl_map().unwrap();
-        let mut symbol = Some(impl_map.import_name());
-
-        if symbol == Some(name) {
-            symbol = None;
-        }
-
+        let symbol = self.method.import_name();
         let name = to_ident(self.method.name());
-        let impl_flags = impl_map.flags();
-
-        let abi = if impl_flags.contains(PInvokeAttributes::CallConvPlatformapi) {
-            "system"
-        } else if impl_flags.contains(PInvokeAttributes::CallConvCdecl) {
-            "cdecl"
-        } else {
-            panic!()
-        };
-
+        let abi = self.method.calling_convention();
         let signature = self.method.signature(self.namespace, &[]);
 
         let params = signature.params.iter().map(|param| {
@@ -81,19 +65,12 @@ impl CppFn {
             return quote! {};
         }
 
-        let mut dependencies = TypeMap::new();
-        self.dependencies(&mut dependencies);
-        Cfg::new(self.method, &dependencies).write(writer, false)
+        Cfg::new(self.method, &self.dependencies()).write(writer, false)
     }
 
     pub fn write(&self, writer: &Writer) -> TokenStream {
         let name = to_ident(self.method.name());
         let signature = self.method.signature(self.namespace, &[]);
-        let mut dependencies = TypeMap::new();
-
-        if writer.config.package {
-            self.dependencies(&mut dependencies);
-        }
 
         let link = self.write_link(writer, false);
         let arches = write_arches(self.method);
@@ -271,11 +248,13 @@ impl CppFn {
             _ => quote! {},
         }
     }
+}
 
-    pub fn dependencies(&self, dependencies: &mut TypeMap) {
+impl Dependencies for CppFn {
+    fn combine(&self, dependencies: &mut TypeMap) {
         self.method
             .signature(self.namespace, &[])
-            .dependencies(dependencies);
+            .combine(dependencies);
 
         let dependency = match self.method.name() {
             "GetWindowLongPtrA" => Some("GetWindowLongA"),
@@ -289,7 +268,7 @@ impl CppFn {
             self.method
                 .reader()
                 .unwrap_full_name(self.namespace, dependency)
-                .dependencies(dependencies);
+                .combine(dependencies);
         }
     }
 }

diff --git a/crates/libs/bindgen/src/types/cpp_interface.rs b/crates/libs/bindgen/src/types/cpp_interface.rs
@@ -49,9 +49,7 @@ impl CppInterface {
             return (Cfg::default(), quote! {});
         }
 
-        let mut dependencies = TypeMap::new();
-        self.dependencies(&mut dependencies);
-        let cfg = Cfg::new(self.def, &dependencies);
+        let cfg = Cfg::new(self.def, &self.dependencies());
         let tokens = cfg.write(writer, false);
         (cfg, tokens)
     }
@@ -223,21 +221,20 @@ impl CppInterface {
             let impl_name: TokenStream = format!("{}_Impl", self.def.name()).into();
 
             let cfg = if writer.config.package {
-                fn collect(interface: &CppInterface, dependencies: &mut TypeMap, writer: &Writer) {
+                fn combine(interface: &CppInterface, dependencies: &mut TypeMap, writer: &Writer) {
                     for method in interface.get_methods(writer).iter() {
                         if let CppMethodOrName::Method(method) = method {
                             dependencies.combine(&method.dependencies);
                         }
                     }
                 }
 
-                let mut dependencies = TypeMap::new();
-                self.dependencies(&mut dependencies);
+                let mut dependencies = self.dependencies();
+                combine(self, &mut dependencies, writer);
 
-                collect(self, &mut dependencies, writer);
                 base_interfaces.iter().for_each(|interface| {
                     if let Type::CppInterface(ty) = interface {
-                        collect(ty, &mut dependencies, writer);
+                        combine(ty, &mut dependencies, writer);
                     }
                 });
 
@@ -417,23 +414,6 @@ impl CppInterface {
         quote! { #namespace #name }
     }
 
-    #[track_caller]
-    pub fn dependencies(&self, dependencies: &mut TypeMap) {
-        let base_interfaces = self.base_interfaces();
-
-        for interface in &base_interfaces {
-            interface.dependencies(dependencies);
-        }
-
-        for method in self.def.methods() {
-            for ty in method.signature(self.def.namespace(), &[]).types() {
-                if ty.is_core() {
-                    ty.dependencies(dependencies);
-                }
-            }
-        }
-    }
-
     pub fn base_interfaces(&self) -> Vec<Type> {
         let mut bases = vec![];
         let mut def = self.def;
@@ -460,3 +440,21 @@ impl CppInterface {
         bases
     }
 }
+
+impl Dependencies for CppInterface {
+    fn combine(&self, dependencies: &mut TypeMap) {
+        let base_interfaces = self.base_interfaces();
+
+        for interface in &base_interfaces {
+            interface.combine(dependencies);
+        }
+
+        for method in self.def.methods() {
+            for ty in method.signature(self.def.namespace(), &[]).types() {
+                if ty.is_core() {
+                    ty.combine(dependencies);
+                }
+            }
+        }
+    }
+}
diff --git a/crates/libs/bindgen/src/types/cpp_method.rs b/crates/libs/bindgen/src/types/cpp_method.rs
@@ -78,16 +78,13 @@ impl ParamHint {
 impl CppMethod {
     pub fn new(def: MethodDef, namespace: &'static str) -> Self {
         let signature = def.signature(namespace, &[]);
-
+        let dependencies = signature.dependencies();
         let mut param_hints = vec![ParamHint::None; signature.params.len()];
 
         for (position, param) in signature.params.iter().enumerate() {
             param_hints[position] = param.into();
         }
 
-        let mut dependencies = TypeMap::new();
-        signature.dependencies(&mut dependencies);
-
         for position in 0..signature.params.len() {
             // Point len params back to the corresponding ptr params.
             match param_hints[position] {