Convert function argument and return types

Fixes #639.

Author: madsmtm

crates/header-translator/src/fn_utils.rs

@@ -0,0 +1,55 @@
+use std::sync::LazyLock;
+
+use regex::Regex;
+
+/// Algorithm described in:
+/// <https://clang.llvm.org/docs/AutomaticReferenceCounting.html#auditing-of-c-retainable-pointer-interfaces>
+///
+/// > A function obeys the create/copy naming convention if its name
+/// > contains as a substring:
+/// > - either “Create” or “Copy” not followed by a lowercase letter, or
+/// > - either “create” or “copy” not followed by a lowercase letter and
+/// >   not preceded by any letter, whether uppercase or lowercase.
+///
+/// See also Clang's implementation:
+/// <https://github.com/llvm/llvm-project/blob/llvmorg-19.1.6/clang/lib/Analysis/CocoaConventions.cpp#L97-L145>
+/// <https://github.com/llvm/llvm-project/blob/llvmorg-19.1.6/clang/lib/Analysis/RetainSummaryManager.cpp>
+pub(crate) fn follows_create_rule(name: &str) -> bool {
+    static RE: LazyLock<Regex> = LazyLock::new(|| {
+        Regex::new(r"(Create|Copy)([^a-z]|$)|([^a-zA-Z]|^)(create|copy)([^a-z]|$)").unwrap()
+    });
+
+    RE.is_match(name)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_follows_create_rule() {
+        assert!(follows_create_rule("ThingCreate"));
+        assert!(follows_create_rule("CreateThing"));
+        assert!(follows_create_rule("CopyCreateThing"));
+        assert!(follows_create_rule("create_thing"));
+
+        assert!(!follows_create_rule("Created"));
+        assert!(!follows_create_rule("created"));
+        assert!(!follows_create_rule("GetAbc"));
+        assert!(!follows_create_rule("recreate"));
+
+        assert!(follows_create_rule("CreatedCopy"));
+
+        // A few real-world examples
+        assert!(follows_create_rule("dispatch_data_create"));
+        assert!(follows_create_rule("dispatch_data_create_map"));
+        assert!(!follows_create_rule("dispatch_data_get_size"));
+        assert!(follows_create_rule("MTLCreateSystemDefaultDevice"));
+        assert!(follows_create_rule("MTLCopyAllDevices"));
+        assert!(!follows_create_rule("MTLRemoveDeviceObserver"));
+        assert!(follows_create_rule("CFArrayCreate"));
+        assert!(follows_create_rule("CFArrayCreateCopy"));
+        assert!(!follows_create_rule("CFArrayGetCount"));
+        assert!(!follows_create_rule("CFArrayGetValues"));
+    }
+}

crates/header-translator/src/lib.rs

@@ -16,6 +16,7 @@ mod config;
 mod context;
 mod display_helper;
 mod expr;
+mod fn_utils;
 mod global_analysis;
 mod id;
 mod library;

crates/header-translator/src/rust_type.rs

@@ -1367,7 +1367,11 @@ impl Ty {
     }
 
     pub(crate) fn is_objc_bool(&self) -> bool {
-        matches!(self, Self::Primitive(Primitive::ObjcBool))
+        match self {
+            Self::Primitive(Primitive::ObjcBool) => true,
+            Self::TypeDef { to, .. } => to.is_objc_bool(),
+            _ => false,
+        }
     }
 
     fn plain(&self) -> impl fmt::Display + '_ {
@@ -1678,6 +1682,59 @@ impl Ty {
         })
     }
 
+    pub(crate) fn fn_return_converter(
+        &self,
+        returns_retained: bool,
+    ) -> Option<(
+        impl fmt::Display + '_,
+        impl fmt::Display + '_,
+        impl fmt::Display + '_,
+    )> {
+        let start = "let ret = ";
+        // SAFETY: The function is marked with the correct retain semantics,
+        // otherwise it'd be invalid to use from Obj-C with ARC and Swift too.
+        let end = |nullability| {
+            match (nullability, returns_retained) {
+            (Nullability::NonNull, true) => {
+                ";\nunsafe { Retained::from_raw(ret.as_ptr()) }.expect(\"function was marked as returning non-null, but actually returned NULL\")"
+            }
+            (Nullability::NonNull, false) => {
+                ";\nunsafe { Retained::retain_autoreleased(ret.as_ptr()) }.expect(\"function was marked as returning non-null, but actually returned NULL\")"
+            }
+            (_, true) => ";\nunsafe { Retained::from_raw(ret) }",
+            (_, false) => ";\nunsafe { Retained::retain_autoreleased(ret) }",
+        }
+        };
+
+        match self {
+            _ if self.is_objc_bool() => Some((" -> bool".to_string(), "", ".as_bool()")),
+            Self::Pointer {
+                nullability,
+                lifetime: Lifetime::Unspecified,
+                pointee,
+                ..
+            } if pointee.is_object_like() && !pointee.is_static_object() => {
+                let res = if *nullability == Nullability::NonNull {
+                    format!(" -> Retained<{}>", pointee.behind_pointer())
+                } else {
+                    format!(" -> Option<Retained<{}>>", pointee.behind_pointer())
+                };
+                Some((res, start, end(*nullability)))
+            }
+            Self::TypeDef {
+                id, nullability, ..
+            } if self.is_object_like() && !self.is_static_object() => {
+                let res = if *nullability == Nullability::NonNull {
+                    format!(" -> Retained<{}>", id.path())
+                } else {
+                    format!(" -> Option<Retained<{}>>", id.path())
+                };
+                Some((res, start, end(*nullability)))
+            }
+            _ => None,
+        }
+    }
+
     pub(crate) fn var(&self) -> impl fmt::Display + '_ {
         FormatterFn(move |f| match self {
             Self::Pointer {
@@ -1764,6 +1821,17 @@ impl Ty {
         })
     }
 
+    pub(crate) fn fn_argument_converter(
+        &self,
+    ) -> Option<(impl fmt::Display + '_, impl fmt::Display + '_)> {
+        if self.is_objc_bool() {
+            Some(("bool", "Bool::new"))
+        } else {
+            // TODO: Support out / autoreleasing pointers?
+            None
+        }
+    }
+
     pub(crate) fn method_argument(&self) -> impl fmt::Display + '_ {
         FormatterFn(move |f| match self {
             Self::Primitive(Primitive::C99Bool) => {

crates/header-translator/src/stmt.rs

@@ -14,6 +14,7 @@ use crate::config::{ClassData, MethodData};
 use crate::context::Context;
 use crate::display_helper::FormatterFn;
 use crate::expr::Expr;
+use crate::fn_utils::follows_create_rule;
 use crate::id::cfg_gate_ln;
 use crate::id::ItemIdentifier;
 use crate::id::Location;
@@ -531,6 +532,7 @@ pub enum Stmt {
         must_use: bool,
         can_unwind: bool,
         link_name: Option<String>,
+        returns_retained: bool,
     },
     /// typedef Type TypedefName;
     AliasDecl {
@@ -1395,6 +1397,8 @@ impl Stmt {
                     warn!("unexpected static method");
                 }
 
+                let mut returns_retained = follows_create_rule(&id.name);
+
                 immediate_children(entity, |entity, _span| match entity.get_kind() {
                     EntityKind::UnexposedAttr => {
                         if let Some(attr) = UnexposedAttr::parse(&entity, context) {
@@ -1403,9 +1407,13 @@ impl Stmt {
                                 UnexposedAttr::UIActor => {
                                     warn!("unhandled UIActor on function declaration")
                                 }
-                                UnexposedAttr::ReturnsRetained
-                                | UnexposedAttr::ReturnsNotRetained => {
-                                    // TODO: Ignore for now, but at some point handle in a similar way to in methods
+                                UnexposedAttr::ReturnsRetained => {
+                                    // Override the inferred value.
+                                    returns_retained = true;
+                                }
+                                UnexposedAttr::ReturnsNotRetained => {
+                                    // Override the inferred value.
+                                    returns_retained = false;
                                 }
                                 UnexposedAttr::NoThrow => {
                                     can_unwind = false;
@@ -1464,6 +1472,7 @@ impl Stmt {
                     must_use,
                     can_unwind,
                     link_name,
+                    returns_retained,
                 }]
             }
             EntityKind::UnionDecl => {
@@ -2421,14 +2430,10 @@ impl Stmt {
                 }
                 Self::FnDecl {
                     id,
-                    availability: _,
                     arguments,
                     result_type,
                     body: Some(_),
-                    safe: _,
-                    must_use: _,
-                    can_unwind: _,
-                    link_name: _,
+                    ..
                 } => {
                     write!(f, "// TODO: ")?;
                     write!(f, "pub fn {}(", id.name)?;
@@ -2444,83 +2449,107 @@ impl Stmt {
                     arguments,
                     result_type,
                     body: None,
-                    safe: false,
+                    safe,
                     must_use,
                     can_unwind,
                     link_name,
+                    returns_retained,
                 } => {
                     let abi = if *can_unwind { "C-unwind" } else { "C" };
-                    writeln!(f, "extern {abi:?} {{")?;
 
-                    write!(f, "    {}", self.cfg_gate_ln(config))?;
-                    write!(f, "    {availability}")?;
-                    if *must_use {
-                        writeln!(f, "    #[must_use]")?;
-                    }
-                    if let Some(link_name) = link_name {
-                        // NOTE: Currently only used on Apple targets.
-                        writeln!(
-                            f,
-                            "    #[cfg_attr(target_vendor = \"apple\", link_name = {link_name:?})]"
-                        )?;
-                    }
-                    write!(f, "    pub fn {}(", id.name)?;
-                    for (param, arg_ty) in arguments {
-                        let param = handle_reserved(&crate::to_snake_case(param));
-                        write!(f, "{param}: {},", arg_ty.fn_argument())?;
-                    }
-                    write!(f, "){}", result_type.fn_return())?;
-                    writeln!(f, ";")?;
+                    let return_converter = result_type.fn_return_converter(*returns_retained);
 
-                    writeln!(f, "}}")?;
-                }
-                Self::FnDecl {
-                    id,
-                    availability,
-                    arguments,
-                    result_type,
-                    body: None,
-                    safe: true,
-                    must_use,
-                    can_unwind,
-                    link_name,
-                } => {
-                    let abi = if *can_unwind { "C-unwind" } else { "C" };
-                    write!(f, "{}", self.cfg_gate_ln(config))?;
-                    write!(f, "{availability}")?;
-                    if *must_use {
-                        writeln!(f, "#[must_use]")?;
-                    }
-                    writeln!(f, "#[inline]")?;
-                    write!(f, "pub extern {abi:?} fn {}(", id.name)?;
-                    for (param, arg_ty) in arguments {
-                        let param = handle_reserved(&crate::to_snake_case(param));
-                        write!(f, "{param}: {},", arg_ty.fn_argument())?;
-                    }
-                    writeln!(f, "){} {{", result_type.fn_return())?;
+                    let needs_wrapper = *safe
+                        || return_converter.is_some()
+                        || arguments
+                            .iter()
+                            .any(|(_, arg)| arg.fn_argument_converter().is_some());
+
+                    let raw_fn_decl = |f: &mut fmt::Formatter<'_>, vis| {
+                        if let Some(link_name) = link_name {
+                            // NOTE: Currently only used on Apple targets.
+                            writeln!(
+                                f,
+                                "#[cfg_attr(target_vendor = \"apple\", link_name = {link_name:?})]"
+                            )?;
+                        }
+                        write!(f, "{vis}fn {}(", id.name)?;
+                        for (param, arg_ty) in arguments {
+                            let param = handle_reserved(&crate::to_snake_case(param));
+                            write!(f, "{param}: {},", arg_ty.fn_argument())?;
+                        }
+                        writeln!(f, "){};", result_type.fn_return())?;
 
-                    writeln!(f, "    extern {abi:?} {{")?;
+                        Ok(())
+                    };
 
-                    if let Some(link_name) = link_name {
-                        writeln!(f, "        #[link_name = {link_name:?}]")?;
-                    }
-                    write!(f, "        fn {}(", id.name)?;
-                    for (param, arg_ty) in arguments {
-                        let param = handle_reserved(&crate::to_snake_case(param));
-                        write!(f, "{param}: {},", arg_ty.fn_argument())?;
-                    }
-                    writeln!(f, "){};", result_type.fn_return())?;
+                    if needs_wrapper {
+                        write!(f, "{}", self.cfg_gate_ln(config))?;
+                        write!(f, "{availability}")?;
+                        if *must_use {
+                            writeln!(f, "#[must_use]")?;
+                        }
+                        writeln!(f, "#[inline]")?;
+                        let unsafe_ = if *safe { "" } else { "unsafe " };
+                        write!(f, "pub {unsafe_}extern {abi:?} fn {}(", id.name)?;
+                        for (param, arg_ty) in arguments {
+                            let param = handle_reserved(&crate::to_snake_case(param));
+                            write!(f, "{param}: ")?;
+                            if let Some((converted_ty, _)) = arg_ty.fn_argument_converter() {
+                                write!(f, "{converted_ty}")?;
+                            } else {
+                                write!(f, "{}", arg_ty.fn_argument())?;
+                            }
+                            write!(f, ",")?;
+                        }
+                        write!(f, ")")?;
+                        if let Some((ty, _, _)) = &return_converter {
+                            write!(f, "{ty}")?;
+                        } else {
+                            write!(f, "{}", result_type.fn_return())?;
+                        }
+                        writeln!(f, " {{")?;
 
-                    writeln!(f, "    }}")?;
+                        // Emit raw
+                        writeln!(f, "    extern {abi:?} {{")?;
+                        raw_fn_decl(f, "")?;
+                        writeln!(f, "    }}")?;
 
-                    write!(f, "    unsafe {{ {}(", id.name)?;
-                    for (param, _) in arguments {
-                        let param = handle_reserved(&crate::to_snake_case(param));
-                        write!(f, "{param},")?;
-                    }
-                    writeln!(f, ") }}")?;
+                        // Call raw
+                        write!(f, "    ")?;
+                        if let Some((_, converter_start, _)) = &return_converter {
+                            write!(f, "{converter_start}")?;
+                        }
+                        write!(f, "unsafe {{ {}(", id.name)?;
+                        for (param, ty) in arguments {
+                            let param = handle_reserved(&crate::to_snake_case(param));
+                            if let Some((_, converter)) = ty.fn_argument_converter() {
+                                write!(f, "{converter}({param})")?;
+                            } else {
+                                write!(f, "{param}")?;
+                            }
+                            write!(f, ",")?;
+                        }
+                        write!(f, ") }}")?;
+                        if let Some((_, _, converter_end)) = &return_converter {
+                            write!(f, "{converter_end}")?;
+                        }
+                        writeln!(f)?;
 
-                    writeln!(f, "}}")?;
+                        writeln!(f, "}}")?;
+                    } else {
+                        writeln!(f, "extern {abi:?} {{")?;
+
+                        write!(f, "    {}", self.cfg_gate_ln(config))?;
+                        write!(f, "    {availability}")?;
+                        if *must_use {
+                            writeln!(f, "    #[must_use]")?;
+                        }
+
+                        raw_fn_decl(f, "pub ")?;
+
+                        writeln!(f, "}}")?;
+                    }
                 }
                 Self::AliasDecl {
                     id,