General pipeline updates

This were the result of me implementing the Python bindings using the
new pipeline system.  There were a handful of things I realized I
needed/wanted there that didn't come up when I was implementing the JS
bindings.

Add `FfiTypeNode`. I didn't originally have this as part of the general
pipeline, since it wasn't needed there.  However, virtually all language
pipelines are going to want to add it so we may as well do it in shared
code.

Determine enum discriminants for the bindings. This uses basically the
same algorithm as from `ComponentInterface`. The one difference is that
for enums without a specified `repr`, this calculates the smallest
integer width needed to store the variants.

I used a new pipeline technique for the enum discriminents: the
old/optional `discr` field gets renamed to `meta_discr` so that we can
create a new/non-optional `discr` field.

Adding the `Node::has_descendant` method. I've noticed I'm implementing
this code again and again, so it feels a good time to generalize.

Several other smaller changes/fixes.

Author: bendk

docs/manual/src/internals/bindings_ir_pipeline.md

@@ -65,6 +65,28 @@ pub struct Node {
 }
 ```
 
+### Field Conversion order
+
+Fields are converted in declaration order.
+If a node contains 2 fields that can come from a single source field, the first one declared will "win".
+
+```rust
+#[derive(Node)]
+pub struct SourceNode {
+    foo: Option<String>
+}
+
+#[derive(Node)]
+pub struct DestNode {
+    /// This field will be set to `SourceNode::foo`
+    #[node(from(foo))]
+    maybe_foo: Option<String>
+    /// This field will be set to an empty string
+    /// A pipeline pass can then populate this field using `maybe_foo`
+    foo: String,
+}
+```
+
 ## Module structure
 
 Each IRs will typically have a module dedicated to them with the following structure:

uniffi_bindgen/src/pipeline/general/callback_interfaces.rs

@@ -5,17 +5,24 @@
 //! FFI info for callback interfaces
 
 use super::*;
+use heck::ToUpperCamelCase;
 use std::collections::HashSet;
 
 pub fn pass(module: &mut Module) -> Result<()> {
     let crate_name = module.crate_name.clone();
+    let module_name = module.name.clone();
     module.try_visit_mut(|cbi: &mut CallbackInterface| {
-        cbi.vtable = vtable(&crate_name, &cbi.name, cbi.methods.clone())?;
+        cbi.vtable = vtable(&module_name, &crate_name, &cbi.name, cbi.methods.clone())?;
         Ok(())
     })?;
     module.try_visit_mut(|int: &mut Interface| {
         if int.imp.has_callback_interface() {
-            int.vtable = Some(vtable(&crate_name, &int.name, int.methods.clone())?);
+            int.vtable = Some(vtable(
+                &module_name,
+                &crate_name,
+                &int.name,
+                int.methods.clone(),
+            )?);
         }
         Ok(())
     })?;
@@ -47,17 +54,27 @@ pub fn pass(module: &mut Module) -> Result<()> {
     Ok(())
 }
 
-fn vtable(crate_name: &str, interface_name: &str, methods: Vec<Method>) -> Result<VTable> {
+fn vtable(
+    module_name: &str,
+    crate_name: &str,
+    interface_name: &str,
+    methods: Vec<Method>,
+) -> Result<VTable> {
     Ok(VTable {
         struct_type: FfiType::Struct(FfiStructName(format!(
             "VTableCallbackInterface{}",
             interface_name
-        ))),
+        )))
+        .into(),
         interface_name: interface_name.to_string(),
         init_fn: RustFfiFunctionName(uniffi_meta::init_callback_vtable_fn_symbol_name(
             crate_name,
             interface_name,
         )),
+        free_fn_type: FfiFunctionTypeName(format!(
+            "CallbackInterfaceFree{}_{interface_name}",
+            module_name.to_upper_camel_case(),
+        )),
         methods: methods
             .into_iter()
             .enumerate()
@@ -67,7 +84,8 @@ fn vtable(crate_name: &str, interface_name: &str, methods: Vec<Method>) -> Resul
                     ffi_type: FfiType::Function(FfiFunctionTypeName(format!(
                         "CallbackInterface{}Method{i}",
                         interface_name
-                    ))),
+                    )))
+                    .into(),
                 })
             })
             .collect::<Result<Vec<_>>>()?,
@@ -119,7 +137,7 @@ fn add_vtable_ffi_definitions(module: &mut Module) -> Result<()> {
                         name: async_info.ffi_foreign_future_result.clone(),
                         fields: match ffi_return_type {
                             Some(return_ffi_type) => vec![
-                                FfiField::new("return_value", return_ffi_type),
+                                FfiField::new("return_value", return_ffi_type.ty),
                                 FfiField::new("call_status", FfiType::RustCallStatus),
                             ],
                             None => vec![
@@ -154,7 +172,8 @@ fn add_vtable_ffi_definitions(module: &mut Module) -> Result<()> {
         ffi_definitions.extend([
             FfiFunctionType {
                 name: FfiFunctionTypeName(format!(
-                    "CallbackInterfaceFree{module_name}_{interface_name}"
+                    "CallbackInterfaceFree{}_{interface_name}",
+                    module_name.to_upper_camel_case(),
                 )),
                 arguments: vec![FfiArgument::new(
                     "handle",
@@ -172,13 +191,15 @@ fn add_vtable_ffi_definitions(module: &mut Module) -> Result<()> {
                 fields: vtable
                     .methods
                     .iter()
-                    .map(|vtable_meth| {
-                        FfiField::new(&vtable_meth.callable.name, vtable_meth.ffi_type.clone())
+                    .map(|vtable_meth| FfiField {
+                        name: vtable_meth.callable.name.clone(),
+                        ty: vtable_meth.ffi_type.clone(),
                     })
                     .chain([FfiField::new(
                         "uniffi_free",
                         FfiType::Function(FfiFunctionTypeName(format!(
-                            "CallbackInterfaceFree{module_name}_{interface_name}"
+                            "CallbackInterfaceFree{}_{interface_name}",
+                            module_name.to_upper_camel_case(),
                         ))),
                     )])
                     .collect(),
@@ -191,7 +212,7 @@ fn add_vtable_ffi_definitions(module: &mut Module) -> Result<()> {
                 name: vtable.init_fn.clone(),
                 arguments: vec![FfiArgument {
                     name: "vtable".into(),
-                    ty: FfiType::Reference(Box::new(vtable.struct_type.clone())),
+                    ty: FfiType::Reference(Box::new(vtable.struct_type.ty.clone())).into(),
                 }],
                 return_type: FfiReturnType { ty: None },
                 async_data: None,
@@ -220,7 +241,8 @@ fn vtable_method(
             ty: FfiType::Handle(HandleKind::CallbackInterface {
                 module_name: module_name.to_string(),
                 interface_name: interface_name.to_string(),
-            }),
+            })
+            .into(),
         })
         .chain(callable.arguments.iter().map(|arg| FfiArgument {
             name: arg.name.clone(),
@@ -229,11 +251,11 @@ fn vtable_method(
         .chain(std::iter::once(match &callable.return_type.ty {
             Some(ty) => FfiArgument {
                 name: "uniffi_out_return".into(),
-                ty: FfiType::MutReference(Box::new(ty.ffi_type.clone())),
+                ty: FfiType::MutReference(Box::new(ty.ffi_type.ty.clone())).into(),
             },
             None => FfiArgument {
                 name: "uniffi_out_return".into(),
-                ty: FfiType::VoidPointer,
+                ty: FfiType::VoidPointer.into(),
             },
         }))
         .collect(),
@@ -257,7 +279,8 @@ fn vtable_method_async(
             ty: FfiType::Handle(HandleKind::CallbackInterface {
                 module_name: module_name.to_string(),
                 interface_name: interface_name.to_string(),
-            }),
+            })
+            .into(),
         })
         .chain(callable.arguments.iter().map(|arg| FfiArgument {
             name: arg.name.clone(),
@@ -266,17 +289,18 @@ fn vtable_method_async(
         .chain([
             FfiArgument {
                 name: "uniffi_future_callback".into(),
-                ty: FfiType::Function(async_data.ffi_foreign_future_complete.clone()),
+                ty: FfiType::Function(async_data.ffi_foreign_future_complete.clone()).into(),
             },
             FfiArgument {
                 name: "uniffi_callback_data".into(),
-                ty: FfiType::Handle(HandleKind::ForeignFutureCallbackData),
+                ty: FfiType::Handle(HandleKind::ForeignFutureCallbackData).into(),
             },
             FfiArgument {
                 name: "uniffi_out_return".into(),
                 ty: FfiType::MutReference(Box::new(FfiType::Struct(FfiStructName(
                     "ForeignFuture".to_owned(),
-                )))),
+                ))))
+                .into(),
             },
         ])
         .collect(),

uniffi_bindgen/src/pipeline/general/checksums.rs

@@ -21,7 +21,7 @@ pub fn pass(module: &mut Module) -> Result<()> {
             async_data: None,
             arguments: vec![],
             return_type: FfiReturnType {
-                ty: Some(FfiType::UInt32),
+                ty: Some(FfiType::UInt32.into()),
             },
             has_rust_call_status_arg: false,
             kind: FfiFunctionKind::UniffiContractVersion,
@@ -33,13 +33,9 @@ pub fn pass(module: &mut Module) -> Result<()> {
     // Checksums, these are used to check that the bindings were built against the same
     // exported interface as the loaded library.
     let mut checksums = vec![];
-    module.try_visit(|callable: &Callable| {
+    // Closure to visit a callable
+    let mut visit_callable = |callable: &Callable| {
         let Some(checksum) = callable.checksum else {
-            if matches!(callable.kind, CallableKind::VTableMethod { .. }) {
-                // UDL-based callbacks don't have checksum functions, ignore those rather than
-                // returning an error.
-                return Ok(());
-            }
             bail!("Checksum not set for {:#?}", callable);
         };
         let fn_name = match &callable.kind {
@@ -66,7 +62,7 @@ pub fn pass(module: &mut Module) -> Result<()> {
                 async_data: None,
                 arguments: vec![],
                 return_type: FfiReturnType {
-                    ty: Some(FfiType::UInt16),
+                    ty: Some(FfiType::UInt16.into()),
                 },
                 has_rust_call_status_arg: false,
                 kind: FfiFunctionKind::Checksum,
@@ -75,7 +71,21 @@ pub fn pass(module: &mut Module) -> Result<()> {
             checksum,
         ));
         Ok(())
+    };
+    // Call `visit_callable` for callables that we have checksums for
+    // (functions/constructors/methods), but not ones where we don't (VTable methods and UniFFI
+    // traits).
+    module.try_visit(|function: &Function| function.try_visit(&mut visit_callable))?;
+    module.try_visit(|int: &Interface| {
+        for cons in int.constructors.iter() {
+            visit_callable(&cons.callable)?;
+        }
+        for meth in int.methods.iter() {
+            visit_callable(&meth.callable)?;
+        }
+        Ok(())
     })?;
+
     for (ffi_func, checksum) in checksums {
         module.checksums.push(Checksum {
             fn_name: ffi_func.name.clone(),

uniffi_bindgen/src/pipeline/general/enums.rs

@@ -3,11 +3,128 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use super::*;
+use std::cmp::{max, min};
 
 pub fn pass(en: &mut Enum) -> Result<()> {
     en.is_flat = match en.shape {
         EnumShape::Error { flat } => flat,
         EnumShape::Enum => en.variants.iter().all(|v| v.fields.is_empty()),
     };
+    for v in en.variants.iter_mut() {
+        v.fields_kind = if v.fields.is_empty() {
+            FieldsKind::Unit
+        } else if v.fields.iter().any(|f| f.name.is_empty()) {
+            FieldsKind::Unnamed
+        } else {
+            FieldsKind::Named
+        };
+    }
+    determine_discriminants(en)?;
+    Ok(())
+}
+
+/// Set the `Enum::discr_type` and `Variant::discr` fields
+///
+/// If we get a value from the metadata, then those will be used.  Otherwise, we will calculate the
+/// discriminants by following Rust's logic.
+pub fn determine_discriminants(en: &mut Enum) -> Result<()> {
+    let signed = match &en.meta_discr_type {
+        Some(type_node) => match &type_node.ty {
+            Type::UInt8 | Type::UInt16 | Type::UInt32 | Type::UInt64 => false,
+            Type::Int8 | Type::Int16 | Type::Int32 | Type::Int64 => true,
+            ty => bail!("Invalid enum discriminant type: {ty:?}"),
+        },
+        // If not specified, then the discriminant type is signed.  We'll calculate the width as we
+        // go through the variant discriminants
+        None => true,
+    };
+
+    // Calculate all variant discriminants.
+    // Use a placeholder value for the type, since we don't necessarily know it yet.
+    let mut max_value: u64 = 0;
+    let mut min_value: i64 = 0;
+    let mut last_variant: Option<&Variant> = None;
+
+    for variant in en.variants.iter_mut() {
+        let discr = match &variant.meta_discr {
+            None => {
+                let lit = match last_variant {
+                    None => {
+                        if signed {
+                            Literal::Int(0, Radix::Decimal, TypeNode::default())
+                        } else {
+                            Literal::UInt(0, Radix::Decimal, TypeNode::default())
+                        }
+                    }
+                    Some(variant) => match &variant.discr.lit {
+                        Literal::UInt(val, _, _) => {
+                            Literal::UInt(val + 1, Radix::Decimal, TypeNode::default())
+                        }
+                        Literal::Int(val, _, _) => {
+                            Literal::Int(val + 1, Radix::Decimal, TypeNode::default())
+                        }
+                        lit => bail!("Invalid enum discriminant literal: {lit:?}"),
+                    },
+                };
+                LiteralNode { lit }
+            }
+            Some(lit_node) => lit_node.clone(),
+        };
+        match &discr.lit {
+            Literal::UInt(val, _, _) => {
+                max_value = max(max_value, *val);
+            }
+            Literal::Int(val, _, _) => {
+                if *val >= 0 {
+                    max_value = max(max_value, *val as u64);
+                } else {
+                    min_value = min(min_value, *val);
+                }
+            }
+            _ => unreachable!(),
+        }
+        variant.discr = discr;
+        last_variant = Some(variant);
+    }
+
+    // Finally, we can figure out the discriminant type
+    en.discr_type = match &en.meta_discr_type {
+        Some(type_node) => type_node.clone(),
+        None => {
+            if min_value >= i8::MIN as i64 && max_value <= i8::MAX as u64 {
+                TypeNode {
+                    ty: Type::Int8,
+                    ..TypeNode::default()
+                }
+            } else if min_value >= i16::MIN as i64 && max_value <= i16::MAX as u64 {
+                TypeNode {
+                    ty: Type::Int16,
+                    ..TypeNode::default()
+                }
+            } else if min_value >= i32::MIN as i64 && max_value <= i32::MAX as u64 {
+                TypeNode {
+                    ty: Type::Int32,
+                    ..TypeNode::default()
+                }
+            } else if max_value <= i64::MAX as u64 {
+                // Note: no need to check `min_value` since that's always in the `i64` bounds.
+                TypeNode {
+                    ty: Type::Int64,
+                    ..TypeNode::default()
+                }
+            } else {
+                bail!("Enum repr not set and magnitude exceeds i64::MAX");
+            }
+        }
+    };
+    for variant in en.variants.iter_mut() {
+        match &mut variant.discr.lit {
+            Literal::UInt(_, _, type_node) | Literal::Int(_, _, type_node) => {
+                *type_node = en.discr_type.clone();
+            }
+            _ => unreachable!(),
+        }
+    }
+
     Ok(())
 }