Merge pull request #17 from NordSecurity/callback_fix

Callback fix

CHANGELOG.md

@@ -1,8 +1,9 @@
-#### UNRELEASED+v0.25.0
+#### v0.3.0+v0.25.0
 
 ----
 
 - Dereference optional objects in the generated bindings
+- **IMPORTANT**: Fix callback code generation
 
 #### v0.2.2+v0.25.0
 

README.md

@@ -10,7 +10,7 @@ Minimum Rust version required to install `uniffi-bindgen-cpp` is `1.72`.
 Newer Rust versions should also work fine.
 
 ```bash
-cargo install uniffi-bindgen-cpp --git https://github.com/NordSecurity/uniffi-bindgen-cpp --tag v0.2.2+v0.25.0
+cargo install uniffi-bindgen-cpp --git https://github.com/NordSecurity/uniffi-bindgen-cpp --tag v0.3.0+v0.25.0
 ```
 
 # How to generate bindings

bindgen/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "uniffi-bindgen-cpp"
-version = "0.2.2+v.0.25.0"
+version = "0.3.0+v.0.25.0"
 edition = "2021"
 
 [dependencies]

bindgen/src/bindings/cpp/gen_cpp/callback_interface.rs

@@ -1,4 +1,4 @@
-use uniffi_bindgen::backend::CodeType;
+use uniffi_bindgen::{backend::CodeType, interface::Literal};
 
 use crate::bindings::cpp::gen_cpp::filters::CppCodeOracle;
 
@@ -15,10 +15,14 @@ impl CallbackInterfaceCodeType {
 
 impl CodeType for CallbackInterfaceCodeType {
     fn type_label(&self) -> String {
-        CppCodeOracle.class_name(&self.id)
+        format!("std::shared_ptr<{}>", self.canonical_name())
     }
 
     fn canonical_name(&self) -> String {
-        format!("CallbackInterface{}", self.type_label())
+        CppCodeOracle.class_name(&self.id)
+    }
+
+    fn literal(&self, _literal: &Literal) -> String {
+        unreachable!();
     }
 }

bindgen/src/bindings/cpp/gen_cpp/compounds.rs

@@ -17,7 +17,7 @@ impl OptionalCodeType {
 
     pub(crate) fn can_dereference(inner_type: &Type) -> bool {
         match inner_type {
-            Type::Object { .. } => true,
+            Type::Object { .. } | Type::CallbackInterface { .. } => true,
             _ => false,
         }
     }

bindgen/src/bindings/cpp/gen_cpp/filters/mod.rs

@@ -179,12 +179,9 @@ pub(crate) fn class_name(nm: &str) -> Result<String> {
 
 pub(crate) fn parameter(arg: &Argument) -> Result<String> {
     Ok(match arg.as_type() {
-        Type::Object { .. } => {
+        Type::Object { .. } | Type::CallbackInterface { .. } => {
             format!("const {} &{}", arg.as_codetype().type_label(), arg.name())
         }
-        Type::CallbackInterface { name, .. } => {
-            format!("std::shared_ptr<{}> {}", name, arg.name())
-        }
         t => format!("{} {}", type_name(&t)?, arg.name()),
     })
 }

bindgen/src/bindings/cpp/templates/callback.hpp

@@ -1,8 +1,9 @@
 {%- let iface = ci|get_callback_interface_definition(name) %}
 {%- let type_name = typ|type_name %}
 {%- let class_name = type_name|class_name %}
+{%- let canonical_type_name = typ|canonical_name %}
 {% call macros::docstring(iface, 0) %}
-struct {{ class_name }} {
+struct {{ canonical_type_name }} {
     {%- for method in iface.methods() %}
     {% call macros::docstring(method, 4) %}
     virtual
@@ -11,5 +12,5 @@ struct {{ class_name }} {
     {{ method.name()|fn_name }}({% call macros::param_list(method) %}) = 0;
     {%- endfor %}
 
-    virtual ~{{ class_name }}() = default;
-};
+    virtual ~{{ canonical_type_name }}() = default;
+};

bindgen/src/bindings/cpp/templates/callback_conv.hpp

@@ -1,16 +1,15 @@
 {%- let ffi_converter_name = typ|ffi_converter_name %}
 {%- let type_name = typ|type_name %}
-{%- let class_name = type_name|class_name %}
-struct {{ ffi_converter_name|class_name }} {
-    static std::shared_ptr<{{ class_name }}> lift(uint64_t);
-    static uint64_t lower(std::shared_ptr<{{ class_name }}>);
-    static std::shared_ptr<{{ class_name }}> read(RustStream &);
-    static void write(RustStream &, uint64_t);
-    static int32_t allocation_size(const {{ class_name }} &);
-
+{%- let canonical_type_name = typ|canonical_name %}
+struct {{ typ|ffi_converter_name }} {
+    static {{ type_name }} lift(uint64_t);
+    static uint64_t lower(const {{ type_name }} &);
+    static {{ type_name }} read(RustStream &);
+    static void write(RustStream &, const {{ type_name }} &);
+    static int32_t allocation_size(const {{ type_name }} &);
 private:
     static int callback_stub(uint64_t, uint32_t, uint8_t *, int32_t, RustBuffer *);
 
     static std::once_flag once;
-    static HandleMap<{{ class_name }}> callbacks;
-};
+    static HandleMap<{{ canonical_type_name }}> callbacks;
+};

bindgen/src/bindings/cpp/templates/callback_iface_tmpl.cpp

@@ -1,44 +1,66 @@
 {%- let type_name = typ|type_name %}
 {%- let ffi_converter_name = typ|ffi_converter_name %}
 {%- let class_name = ffi_converter_name|class_name %}
+{%- let canonical_type_name = typ|canonical_name %}
 {%- let iface = ci|get_callback_interface_definition(name) %}
 std::once_flag uniffi::{{ class_name }}::once = std::once_flag();
-uniffi::HandleMap<{{ type_name|class_name }}> uniffi::{{ class_name }}::callbacks = {};
+uniffi::HandleMap<{{ canonical_type_name }}> uniffi::{{ class_name }}::callbacks = {};
 
-std::shared_ptr<{{ type_name|class_name }}> uniffi::{{ class_name }}::lift(uint64_t handle) {
+{{ type_name }} uniffi::{{ class_name }}::lift(uint64_t handle) {
     std::call_once({{ class_name }}::once, []() {
         rust_call({{ iface.ffi_init_callback().name() }}, nullptr, callback_stub);
     });
 
     return callbacks.at(handle);
 }
 
-uint64_t uniffi::{{ class_name }}::lower(std::shared_ptr<{{ type_name|class_name }}> impl) {
+uint64_t uniffi::{{ class_name }}::lower(const {{type_name}}& impl) {
     std::call_once({{ class_name }}::once, []() {
         rust_call({{ iface.ffi_init_callback().name() }}, nullptr, callback_stub);
     });
 
     return callbacks.insert(impl);
 }
 
+{{ type_name }} uniffi::{{ class_name }}::read(RustStream &stream) {
+    uint64_t handle;
+    stream >> handle;
+
+    return lift(handle);
+}
+
+void uniffi::{{ class_name }}::write(RustStream &stream, const {{ type_name }} &impl) {
+    stream << lower(impl);
+}
+
+int32_t uniffi::{{ class_name }}::allocation_size(const {{ type_name }} &impl) {
+    return sizeof(uint64_t);
+}
+
 int uniffi::{{ class_name }}::callback_stub(uint64_t handle, uint32_t method, uint8_t *args_data, int32_t args_len, RustBuffer *buf_ptr) {
-    ForeignBytes bytes = { args_len, args_data };
-    auto buf = rustbuffer_from_bytes(bytes);
+    auto buf = RustBuffer {
+        .capacity = args_len,
+        .len = args_len,
+        .data = args_data,
+    };
     auto stream = RustStream(&buf);
 
     switch (method) {
     case 0:
         callbacks.erase(handle);
         break;
     {%- for method in iface.methods() %}
-    case {{ loop.index }}:
+    case {{ loop.index }}: {
         {% if method.throws_type().is_some() %}{{ "try {" }}{% endif %}
             auto impl = lift(handle);
-        {%- if method.return_type().is_some() %}
+            {%- for arg in method.arguments() %}
+            auto arg{{ loop.index0 }} = {{- arg|read_fn }}(stream);
+            {%- endfor -%}
+            {%- if method.return_type().is_some() %}
             auto ret = {% endif -%}
             impl->{{ method.name() }}(
             {%- for arg in method.arguments() %}
-            {{- arg|read_fn }}(stream){%- if !loop.last %}, {% else %}{% endif %}
+            arg{{ loop.index0 }}{%- if !loop.last %}, {% else %}{% endif %}
             {%- endfor -%}
         );
         {%- match method.return_type() %}
@@ -55,11 +77,12 @@ int uniffi::{{ class_name }}::callback_stub(uint64_t handle, uint32_t method, ui
             return 1;
         } catch (std::exception &ex) {
             *buf_ptr = {{ Type::String.borrow()|lower_fn }}(ex.what());
-            return 1;
+            return 2;
         }
         {% else %}
         {%- endmatch %}
         break;
+    }
     {%- endfor %}
     }
 

cpp-tests/CMakeLists.txt

@@ -36,6 +36,7 @@ endmacro(test_case)
 
 test_case(arithmetic)
 test_case(callbacks)
+test_case(fixture_callbacks)
 test_case(chronological)
 test_case(custom_types)
 test_case(geometry)
@@ -56,7 +57,7 @@ add_custom_target(libs ALL
 )
 
 add_custom_target(bindings ALL
-    DEPENDS libs 
+    DEPENDS libs
     COMMAND ./uniffi-bindgen-cpp --library libuniffi_bindgen_cpp_fixtures.so --out-dir bindings
     WORKING_DIRECTORY ${BINDINGS_BUILD_DIR}
     COMMENT "Generating bindings"