diff --git a/.github/workflows/test-downstream.yml b/.github/workflows/test-downstream.yml index 7e2d1395..38116141 100644 --- a/.github/workflows/test-downstream.yml +++ b/.github/workflows/test-downstream.yml @@ -42,6 +42,24 @@ jobs: uniffi-dart = { path = "../main" } EOF + - name: Bump payjoin-ffi to uniffi 0.32 + # rust-payjoin still pins uniffi 0.31.2; its 0.31 metadata cannot be read + # by this PR's 0.32 bindgen (`Unexpected metadata type code`). Bump its + # own uniffi to 0.32 so the metadata matches — payjoin-ffi compiles and + # its Dart suite passes unchanged on 0.32 (probed: 24 tests). Removed once + # rust-payjoin ships 0.32 upstream. + run: | + sed -i 's/uniffi = { version = "0.31.2"/uniffi = { version = "0.32"/g' rust-payjoin/payjoin-ffi/Cargo.toml + # payjoin's native-assets build pins rustc 1.85.1 (< uniffi 0.32's 1.91 + # floor). native_toolchain_rust requires an *exact* channel and a + # `targets` list, so pin 1.91.0 with just the Linux host target (the CI + # only builds for the host). + for f in $(find rust-payjoin -name rust-toolchain.toml); do + printf '[toolchain]\nchannel = "1.91.0"\ntargets = ["x86_64-unknown-linux-gnu"]\n' > "$f" + done + echo "--- payjoin-ffi uniffi pins now ---" + grep -n 'uniffi = ' rust-payjoin/payjoin-ffi/Cargo.toml || true + - name: Generate dart bindings and run tests run: cd rust-payjoin/payjoin-ffi/dart && bash scripts/generate_bindings.sh && dart test @@ -71,13 +89,45 @@ jobs: - name: Use cache uses: Swatinem/rust-cache@v2 - - name: Patch uniffi-dart dependency + - name: Fetch and bump bdk-ffi to uniffi 0.32 + # bdk-dart re-exports bdk-ffi, pinned by git rev at uniffi 0.31.2 — so the + # metadata bdk_dart_ffi embeds is 0.31 and this PR's 0.32 bindgen cannot + # read it. Clone that exact rev, bump its uniffi to 0.32 (it compiles + # clean on 0.32), and `[patch]` bdk-dart onto the local copy. Removed once + # bdk-ffi ships 0.32 upstream. run: | + BDK_FFI_REV=17c48b8b52ba81cdc58531e75ad1165be0cc25d9 + git init -q bdk-ffi + git -C bdk-ffi remote add origin https://github.com/bitcoindevkit/bdk-ffi.git + git -C bdk-ffi fetch --depth 1 origin "$BDK_FFI_REV" -q + git -C bdk-ffi checkout -q FETCH_HEAD + sed -i 's/uniffi = { version = "=0.31.2"/uniffi = { version = "0.32"/g' bdk-ffi/bdk-ffi/Cargo.toml + echo "--- bdk-ffi uniffi pins now ---" + grep -n 'uniffi = ' bdk-ffi/bdk-ffi/Cargo.toml || true + + - name: Patch bdk-dart onto uniffi 0.32 (uniffi, uniffi-dart, bdk-ffi) + run: | + # bdk-dart's own native crate also pins uniffi 0.31.2 — bump it too. + sed -i 's/uniffi = { version = "=0.31.2"/uniffi = { version = "0.32"/g' bdk-dart/native/Cargo.toml cat >> bdk-dart/native/Cargo.toml << 'EOF' [patch.'https://github.com/Uniffi-Dart/uniffi-dart'] uniffi-dart = { path = "../../main" } + + [patch.'https://github.com/bitcoindevkit/bdk-ffi.git'] + bdk-ffi = { path = "../../bdk-ffi/bdk-ffi" } EOF + # Lockfile pins uniffi 0.31.2 / the old bdk-ffi rev; drop it so the bump + # and patches re-resolve cleanly. + rm -f bdk-dart/native/Cargo.lock + # bdk-dart/bdk-ffi pin rustc 1.85.1, but uniffi 0.32 needs 1.91. + # native_toolchain_rust requires an *exact* channel and a `targets` + # list, so pin 1.91.0 with just the Linux host target. + for f in $(find bdk-dart bdk-ffi -name rust-toolchain.toml); do + printf '[toolchain]\nchannel = "1.91.0"\ntargets = ["x86_64-unknown-linux-gnu"]\n' > "$f" + done + echo "--- bdk-dart native uniffi pins now ---" + grep -n 'uniffi = ' bdk-dart/native/Cargo.toml || true - name: Generate dart bindings and run tests run: cd bdk-dart && bash scripts/generate_bindings.sh && dart test diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 56059579..0e970284 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -15,7 +15,7 @@ jobs: matrix: rust: - stable - - "1.85" + - "1.91" - nightly steps: - name: Checkout sources @@ -47,7 +47,7 @@ jobs: matrix: rust: - stable - - "1.85" + - "1.91" - nightly steps: - name: Checkout sources diff --git a/Cargo.toml b/Cargo.toml index 5eb76add..01978f82 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" license = "Apache-2 or MIT" homepage = "https://github.com/acterglobal/uniffi-dart" description = "Dart Frontend for UniFFI" -rust-version = "1.85" +rust-version = "1.91" [features] defaults = [] @@ -38,7 +38,13 @@ uniffi_bindgen = { workspace = true } camino = "1" cargo_metadata = "0.18" serde = "1" -toml = ">=0.8, <=0.9" +# Match uniffi_bindgen 0.32's own toml: its `BindingGenerator::new_config` takes +# `&toml::Value`, so our impl must resolve the SAME toml version or the trait +# signature mismatches (E0053). uniffi_bindgen 0.32.0 declares `toml = ">=0.9, +# <2"` and resolves to 1.x, so pin `toml = "1"`; a looser pin lets the resolver +# put us on 0.9 while uniffi stays on 1.x (splitting the crate) — which has no +# committed Cargo.lock to stabilize it, so CI re-resolves and breaks. +toml = "1" genco = "0.17.5" proc-macro2 = "1.0.66" @@ -82,12 +88,13 @@ members = [ "fixtures/proc-macro", "fixtures/proc-macro-no-implicit-prelude", "fixtures/enum_variant_collision", + "fixtures/map_type", #"fixtures/*", ] [workspace.dependencies] -uniffi = { version = "0.31.2" } -uniffi_bindgen = { version = "0.31.2" } -uniffi_build = { version = "0.31.2" } -uniffi_testing = { version = "0.31.2" } +uniffi = { version = "0.32" } +uniffi_bindgen = { version = "0.32" } +uniffi_build = { version = "0.32" } +uniffi_testing = { version = "0.32" } camino = { version = "1.1" } diff --git a/fixtures/benchmarks/Cargo.toml b/fixtures/benchmarks/Cargo.toml index 2915d083..e9fb446e 100644 --- a/fixtures/benchmarks/Cargo.toml +++ b/fixtures/benchmarks/Cargo.toml @@ -9,7 +9,7 @@ crate-type = ["lib", "cdylib"] bench = false [dependencies] -uniffi = "0.31" +uniffi = { workspace = true } clap = { version = "4", features = ["cargo", "std", "derive"] } criterion = "0.5.1" diff --git a/fixtures/bytes_types/src/lib.rs b/fixtures/bytes_types/src/lib.rs index 20cea336..73b481f8 100644 --- a/fixtures/bytes_types/src/lib.rs +++ b/fixtures/bytes_types/src/lib.rs @@ -7,6 +7,13 @@ fn take_bytes(v: Vec) -> Vec { v } +// Borrowed `&[u8]` argument: uniffi 0.32 lowers this through the zero-copy +// `ForeignBytes` FFI path rather than the owned `RustBuffer` path. +#[uniffi::export] +fn take_bytes_by_ref(v: &[u8]) -> Vec { + v.to_vec() +} + #[uniffi::export] fn take_bytes_with_validation(v: Vec) -> Vec { // Validate that it's valid UTF-8 if it should be diff --git a/fixtures/bytes_types/test/bytes_types_test.dart b/fixtures/bytes_types/test/bytes_types_test.dart index e1171a4a..fc7676fb 100644 --- a/fixtures/bytes_types/test/bytes_types_test.dart +++ b/fixtures/bytes_types/test/bytes_types_test.dart @@ -11,6 +11,17 @@ void main() { expect(result, equals(input)); }); + test('take_bytes_by_ref (&[u8], ForeignBytes path) returns same data', () { + final input = [10, 20, 30, 40, 50]; + final result = takeBytesByRef(v: Uint8List.fromList(input)); + expect(result, equals(input)); + }); + + test('take_bytes_by_ref handles empty bytes', () { + final result = takeBytesByRef(v: Uint8List.fromList([])); + expect(result, isEmpty); + }); + // test('take_bytes_with_validation handles UTF-8', () { // final utf8Input = 'Hello, 世界!'.codeUnits; // final result = takeBytesWithValidation(utf8Input); diff --git a/fixtures/docstring-proc-macro/Cargo.toml b/fixtures/docstring-proc-macro/Cargo.toml index 861854a8..47dfbe82 100644 --- a/fixtures/docstring-proc-macro/Cargo.toml +++ b/fixtures/docstring-proc-macro/Cargo.toml @@ -9,7 +9,7 @@ crate-type = ["lib", "cdylib"] [dependencies] thiserror = "1.0" -uniffi = "0.31" +uniffi = { workspace = true } [build-dependencies] uniffi-dart = { path = "../../", features = ["build"] } diff --git a/fixtures/map_type/Cargo.toml b/fixtures/map_type/Cargo.toml new file mode 100644 index 00000000..43d35923 --- /dev/null +++ b/fixtures/map_type/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "map_type" +version = "0.1.0" +edition = "2021" +publish = false + +[lib] +name = "map_type" +crate-type = ["lib", "cdylib"] + +[dependencies] +uniffi = { workspace = true, features = [ + "build", +] } + +[build-dependencies] +uniffi-dart = { path = "../../", features = ["build"] } + +[dev-dependencies] +uniffi-dart = { path = "../../", features = ["bindgen-tests"] } +uniffi = { workspace = true, features = [ + "bindgen-tests", +] } +anyhow = "1" diff --git a/fixtures/map_type/build.rs b/fixtures/map_type/build.rs new file mode 100644 index 00000000..17e39498 --- /dev/null +++ b/fixtures/map_type/build.rs @@ -0,0 +1,3 @@ +fn main() { + uniffi_dart::generate_scaffolding("./src/api.udl".into()).unwrap(); +} diff --git a/fixtures/map_type/src/api.udl b/fixtures/map_type/src/api.udl new file mode 100644 index 00000000..0c711997 --- /dev/null +++ b/fixtures/map_type/src/api.udl @@ -0,0 +1 @@ +namespace map_type { }; diff --git a/fixtures/map_type/src/lib.rs b/fixtures/map_type/src/lib.rs new file mode 100644 index 00000000..ccb32608 --- /dev/null +++ b/fixtures/map_type/src/lib.rs @@ -0,0 +1,43 @@ +use std::collections::HashMap; + +// Minimal Map round-trip surface. Mirrors how our qdrant-edge-ffi crate +// exposes payload/config maps: plain proc-macro exports over HashMap. + +#[uniffi::export] +pub fn roundtrip_map(m: HashMap) -> HashMap { + m +} + +#[uniffi::export] +pub fn count_entries(m: HashMap) -> u32 { + m.len() as u32 +} + +#[uniffi::export] +pub fn map_with_record_values(m: HashMap) -> HashMap { + m +} + +// Variable-length value converters (nested Map, Option) are what stress the Map +// FfiConverter's offset arithmetic — a fixed-size value (i32/Point) can't reveal +// a drifting offset. These mirror real payload shapes: string -> nested/nullable. + +#[uniffi::export] +pub fn roundtrip_nested_map( + m: HashMap>, +) -> HashMap> { + m +} + +#[uniffi::export] +pub fn roundtrip_optional_map(m: HashMap>) -> HashMap> { + m +} + +#[derive(uniffi::Record, Clone)] +pub struct Point { + x: i64, + y: i64, +} + +uniffi::include_scaffolding!("api"); diff --git a/fixtures/map_type/test/map_type_test.dart b/fixtures/map_type/test/map_type_test.dart new file mode 100644 index 00000000..41b12988 --- /dev/null +++ b/fixtures/map_type/test/map_type_test.dart @@ -0,0 +1,47 @@ +import 'package:test/test.dart'; +import '../map_type.dart'; + +void main() { + test('map roundtrip preserves entries', () { + final m = {'a': 1, 'b': 2}; + final out = roundtripMap(m: m); + expect(out['a'], 1); + expect(out['b'], 2); + expect(out.length, 2); + }); + + test('map count', () { + expect(countEntries(m: {'x': 10, 'y': 20, 'z': 30}), 3); + }); + + test('map with record values', () { + final out = mapWithRecordValues(m: {'origin': Point(x: 0, y: 0), 'unit': Point(x: 1, y: 1)}); + expect(out['unit']!.x, 1); + expect(out['unit']!.y, 1); + expect(out.length, 2); + }); + + test('nested map round-trips (variable-length values)', () { + final out = roundtripNestedMap(m: { + 'a': {'x': 1, 'y': 2}, + 'b': {'z': 3}, + }); + expect(out['a']!['x'], 1); + expect(out['a']!['y'], 2); + expect(out['b']!['z'], 3); + expect(out.length, 2); + }); + + test('map with optional values round-trips (null preserved, distinct from absent)', () { + final out = roundtripOptionalMap(m: {'present': 7, 'absent': null}); + expect(out['present'], 7); + expect(out['absent'], null); + expect(out.containsKey('absent'), true); + expect(out.length, 2); + }); + + test('empty map round-trips at the length-0 boundary', () { + expect(roundtripMap(m: {}), isEmpty); + expect(countEntries(m: {}), 0); + }); +} diff --git a/fixtures/map_type/tests/mod.rs b/fixtures/map_type/tests/mod.rs new file mode 100644 index 00000000..2a8a2dc2 --- /dev/null +++ b/fixtures/map_type/tests/mod.rs @@ -0,0 +1,6 @@ +use anyhow::Result; + +#[test] +fn map_type() -> Result<()> { + uniffi_dart::testing::run_test("map_type", "src/api.udl", None) +} diff --git a/fixtures/metadata/Cargo.toml b/fixtures/metadata/Cargo.toml index 08eb5a6f..74aa95f8 100644 --- a/fixtures/metadata/Cargo.toml +++ b/fixtures/metadata/Cargo.toml @@ -9,7 +9,7 @@ crate-type = ["lib", "cdylib"] [dependencies] thiserror = "1.0" -uniffi = "0.31" +uniffi = { workspace = true } uniffi-dart = { path = "../../", features = ["bindgen-tests"] } [build-dependencies] diff --git a/fixtures/proc-macro/src/lib.rs b/fixtures/proc-macro/src/lib.rs index 09f07204..90d88001 100644 --- a/fixtures/proc-macro/src/lib.rs +++ b/fixtures/proc-macro/src/lib.rs @@ -29,10 +29,25 @@ impl Object { Arc::new(Self) } + // Constructor taking a borrowed `&[u8]` — exercises the sync constructor + // initializer-list call site (`_ptr = using((Arena _uniffiArena) => rustCall(...))`, + // via `wrap_ffi_call_expr`), a distinct generator from the method/function ones. + #[uniffi::constructor] + fn from_bytes(data: &[u8]) -> Arc { + let _ = data.len(); + Arc::new(Self) + } + fn is_heavy(&self) -> MaybeBool { MaybeBool::Uncertain } + // Borrowed `&[u8]` on a *method* — exercises the object method call site's + // borrowed-bytes free (distinct from the free-function one). + fn borrowed_bytes_len(&self, data: &[u8]) -> u64 { + data.len() as u64 + } + fn get_trait(&self, inc: Option>) -> Arc { inc.unwrap_or_else(|| Arc::new(TraitImpl {})) } @@ -199,4 +214,34 @@ pub fn callback_get_other_multiply( callback.get_other_callback_interface().multiply(a, b) } +// Borrowed `&[u8]` argument (proc-macro `by_ref`): exercises the `ForeignBytes` +// lowering and the arena-scoped free the call site emits around it. Sums the +// bytes so a caller can assert the buffer content crossed the FFI boundary +// intact — and, called in a loop, that the per-call native copy does not leak. +#[uniffi::export] +pub fn sum_borrowed_bytes(data: &[u8]) -> u64 { + data.iter().map(|&b| u64::from(b)).sum() +} + +// A fallible variant, so the throwing call-site shape (which must still free the +// borrowed copy on the error path) is exercised. Errors on a NON-empty input (a +// leading 0xFF sentinel) so the error path unwinds with a real data buffer to +// free through `using`, not merely the empty `(null, 0)` struct. +#[uniffi::export] +pub fn sum_borrowed_bytes_checked(data: &[u8]) -> Result { + if data.first() == Some(&0xFF) { + return Err(BasicError::OsError); + } + Ok(data.iter().map(|&b| u64::from(b)).sum()) +} + +// Void return with a borrowed `&[u8]`: exercises the `void` call-site branch +// (`rustCall((status){...})`) wrapped in the arena — distinct from the non-void +// `rustCallWithLifter` branch the functions above hit. +#[uniffi::export] +pub fn consume_borrowed_bytes(data: &[u8]) { + // Touch the bytes so the argument is genuinely lowered/read. + let _ = data.iter().fold(0u64, |acc, &b| acc.wrapping_add(u64::from(b))); +} + uniffi::include_scaffolding!("api"); diff --git a/fixtures/proc-macro/test/proc_macro_test.dart b/fixtures/proc-macro/test/proc_macro_test.dart index f6a6f329..adcb325c 100644 --- a/fixtures/proc-macro/test/proc_macro_test.dart +++ b/fixtures/proc-macro/test/proc_macro_test.dart @@ -155,5 +155,50 @@ void main() { fromFunction.dispose(); functionRoundtrip.dispose(); }); + + test('borrowed &[u8] arguments cross the boundary intact (all call shapes)', () { + // Free function (non-void): content crosses the boundary intact. + expect(sumBorrowedBytes(data: Uint8List.fromList([1, 2, 3])), 6); + // Empty slice hits the (null, 0) path — must not throw or miscount. + expect(sumBorrowedBytes(data: Uint8List.fromList([])), 0); + + // Method on an object (a distinct call-site generator from the free fn). + final obj = Object(); + expect(obj.borrowedBytesLen(data: Uint8List.fromList([9, 8, 7, 6])), 4); + expect(obj.borrowedBytesLen(data: Uint8List.fromList([])), 0); + + // Constructor taking &[u8] (initializer-list call site — its own generator). + final fromBytes = Object.fromBytes(data: Uint8List.fromList([1, 2, 3])); + expect(fromBytes.isHeavy(), MaybeBool.uncertain); + Object.fromBytes(data: Uint8List.fromList([])).dispose(); // empty ctor path + fromBytes.dispose(); + + // Void-return call site (rustCall branch, not rustCallWithLifter). + consumeBorrowedBytes(data: Uint8List.fromList([1, 2, 3])); + consumeBorrowedBytes(data: Uint8List.fromList([])); + + // Fallible variant: success path, and an error path triggered by a NON-empty + // input (leading 0xFF) so the throw unwinds through `using` with a real data + // buffer to free (not just the empty (null,0) struct). + expect(sumBorrowedBytesChecked(data: Uint8List.fromList([10, 20])), 30); + expect( + () => sumBorrowedBytesChecked(data: Uint8List.fromList([0xFF, 1, 2])), + throwsA(isA()), + ); + + // Smoke test only: many calls confirm the emitted lower+free code compiles + // and does not crash / double-free over repeated use. NOTE: this does NOT + // detect the native-memory leak this fix addresses — a leak does not fail a + // functional test (memory grows, assertions still pass). Catching the leak + // itself needs an RSS/sanitizer check (tracked as a follow-up). + var acc = 0; + final payload = Uint8List.fromList(List.generate(64, (i) => i & 0xff)); + for (var i = 0; i < 50000; i++) { + acc += sumBorrowedBytes(data: payload); + } + expect(acc, 50000 * 2016); // sum(0..63) == 2016 + + obj.dispose(); + }); }); } diff --git a/flake.nix b/flake.nix index ac61e5ed..7ccafedb 100644 --- a/flake.nix +++ b/flake.nix @@ -25,7 +25,7 @@ flake-utils.lib.eachDefaultSystem ( system: let - msrvVersion = "1.85.0"; + msrvVersion = "1.91.0"; rustExtensions = [ "clippy" diff --git a/src/gen/code_type.rs b/src/gen/code_type.rs index 8524e0a6..11d3c72f 100644 --- a/src/gen/code_type.rs +++ b/src/gen/code_type.rs @@ -1,6 +1,6 @@ use std::fmt::Debug; -use uniffi_bindgen::pipeline::general::nodes::Literal; +use uniffi_bindgen::interface::Literal; /// A trait tor the implementation. pub trait CodeType: Debug { diff --git a/src/gen/enums.rs b/src/gen/enums.rs index afb8880a..aaa0a040 100644 --- a/src/gen/enums.rs +++ b/src/gen/enums.rs @@ -1,7 +1,6 @@ use genco::prelude::*; use heck::ToLowerCamelCase; -use uniffi_bindgen::interface::{AsType, Enum, Field, Type}; -use uniffi_bindgen::pipeline::general::nodes::Literal; +use uniffi_bindgen::interface::{AsType, Enum, Field, Literal, Type}; use super::oracle::{AsCodeType, DartCodeOracle}; use super::render::{AsRenderable, Renderable, TypeHelperRenderer}; diff --git a/src/gen/functions.rs b/src/gen/functions.rs index 610e1d05..c3892926 100644 --- a/src/gen/functions.rs +++ b/src/gen/functions.rs @@ -36,6 +36,10 @@ pub fn generate_function(func: &Function, type_helper: &dyn TypeHelperRenderer) quote!(null) }; + // Whether any argument takes the borrowed `&[u8]` path, whose native copy + // must be freed after the call (see `wrap_ffi_call_stmt`). + let has_borrowed = DartCodeOracle::any_borrowed_bytes(&func.arguments()); + // Use centralized callback-aware argument lowering if func.is_async() { // For async methods returning objects, we need to convert the int pointer to Pointer @@ -52,38 +56,44 @@ pub fn generate_function(func: &Function, type_helper: &dyn TypeHelperRenderer) quote!( Future<$ret> $(DartCodeOracle::fn_name(func.name()))($args) { - return uniffiRustCallAsync( - () => $(func.ffi_func().name())( - $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) - ), - $(DartCodeOracle::async_poll(func, type_helper.get_ci())), - $(DartCodeOracle::async_complete(func, type_helper.get_ci())), - $(DartCodeOracle::async_free(func, type_helper.get_ci())), - $async_lifter, - $error_handler, - ); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, true, quote!( + uniffiRustCallAsync( + () => $(func.ffi_func().name())( + $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) + ), + $(DartCodeOracle::async_poll(func, type_helper.get_ci())), + $(DartCodeOracle::async_complete(func, type_helper.get_ci())), + $(DartCodeOracle::async_free(func, type_helper.get_ci())), + $async_lifter, + $error_handler, + ) + ))) } ) } else if ret == quote!(void) { quote!( $ret $(DartCodeOracle::fn_name(func.name()))($args) { - return rustCall((status) { - $(func.ffi_func().name())( - $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) status - ); - }, $error_handler); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, false, quote!( + rustCall((status) { + $(func.ffi_func().name())( + $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) status + ); + }, $error_handler) + ))) } ) } else { quote!( $ret $(DartCodeOracle::fn_name(func.name()))($args) { - return rustCallWithLifter( - (status) => $(func.ffi_func().name())( - $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) status - ), - $lifter, - $error_handler - ); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, false, quote!( + rustCallWithLifter( + (status) => $(func.ffi_func().name())( + $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) status + ), + $lifter, + $error_handler + ) + ))) } ) } diff --git a/src/gen/objects.rs b/src/gen/objects.rs index e3c3eafd..6f21d160 100644 --- a/src/gen/objects.rs +++ b/src/gen/objects.rs @@ -2,8 +2,9 @@ use std::fmt::Debug; use genco::prelude::*; use heck::ToLowerCamelCase; -use uniffi_bindgen::interface::{AsType, Method, Object, ObjectImpl, UniffiTrait}; -use uniffi_bindgen::pipeline::general::nodes::Literal; +use uniffi_bindgen::interface::{ + AsType, Literal, Method, Object, ObjectImpl, TraitKind, UniffiTrait, +}; use super::defaults::render_argument_param; use super::stream::generate_stream; @@ -46,10 +47,17 @@ impl CodeType for ObjectCodeType { // that a Rust method named `read`/`write`/`lower` cannot collide with the // converter's statics (Dart rejects a static and an instance member sharing // a name). Matches how records, enums and callback interfaces are emitted. - ObjectImpl::Struct | ObjectImpl::Trait => { + ObjectImpl::Struct => format!("FfiConverter{}", self.canonical_name()), + // Foreign-implementable traits (with_foreign / callback_interface) are + // lowered through a callback-interface FfiConverter. Mirrors + // uniffi's `ObjectImpl::has_callback_interface()` (Both | ForeignOnly). + ObjectImpl::Trait(TraitKind::Both | TraitKind::ForeignOnly) => { + format!("FfiConverterCallbackInterface{}", self.id) + } + // A pure Rust-only trait object uses its own sibling FfiConverter class. + ObjectImpl::Trait(TraitKind::RustOnly) => { format!("FfiConverter{}", self.canonical_name()) } - ObjectImpl::CallbackTrait => format!("FfiConverterCallbackInterface{}", self.id), } } } @@ -155,6 +163,7 @@ pub fn generate_object(obj: &Object, type_helper: &dyn TypeHelperRenderer) -> da let ffi_call_args = quote!($(for arg in constructor.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) ); + let has_borrowed = DartCodeOracle::any_borrowed_bytes(&constructor.arguments()); // Ensure argument types are included for arg in constructor.arguments() { @@ -164,27 +173,31 @@ pub fn generate_object(obj: &Object, type_helper: &dyn TypeHelperRenderer) -> da if constructor.is_async() { async_constructor_factories.push(quote! { static Future<$cls_name> $(DartCodeOracle::fn_name(constructor_name))($dart_params) { - return uniffiRustCallAsync( - () => $ffi_func_name( - $ffi_call_args - ), - $(DartCodeOracle::async_poll(constructor, type_helper.get_ci())), - $(DartCodeOracle::async_complete(constructor, type_helper.get_ci())), - $(DartCodeOracle::async_free(constructor, type_helper.get_ci())), - (int handle) => $cls_name._(Pointer.fromAddress(handle)), - $error_handler, - ); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, true, quote!( + uniffiRustCallAsync( + () => $ffi_func_name( + $ffi_call_args + ), + $(DartCodeOracle::async_poll(constructor, type_helper.get_ci())), + $(DartCodeOracle::async_complete(constructor, type_helper.get_ci())), + $(DartCodeOracle::async_free(constructor, type_helper.get_ci())), + (int handle) => $cls_name._(Pointer.fromAddress(handle)), + $error_handler, + ) + ))) } }); } else { constructor_definitions.push(quote! { // Public constructor - $dart_constructor_decl($dart_params) : _ptr = rustCall((status) => - $ffi_func_name( - $ffi_call_args status - ), - $error_handler - ) { + $dart_constructor_decl($dart_params) : _ptr = $(DartCodeOracle::wrap_ffi_call_expr(has_borrowed, quote!( + rustCall((status) => + $ffi_func_name( + $ffi_call_args status + ), + $error_handler + ) + ))) { _$finalizer_cls_name.attach(this, _ptr, detach: this); } }); @@ -401,6 +414,8 @@ fn generate_callback_trait_rust_method( quote!(null) }; + let has_borrowed = DartCodeOracle::any_borrowed_bytes(&method.arguments()); + if method.is_async() { let async_lifter = if let Some(ret_type) = method.return_type() { match ret_type { @@ -416,45 +431,51 @@ fn generate_callback_trait_rust_method( quote! { @override Future<$ret> $method_name($(for a in &dart_args => $a,)) { - return uniffiRustCallAsync( - () => $(method.ffi_func().name())( - uniffiClonePointer(), - $(for arg in &method.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) - ), - $(DartCodeOracle::async_poll(method, type_helper.get_ci())), - $(DartCodeOracle::async_complete(method, type_helper.get_ci())), - $(DartCodeOracle::async_free(method, type_helper.get_ci())), - $async_lifter, - $error_handler, - ); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, true, quote!( + uniffiRustCallAsync( + () => $(method.ffi_func().name())( + uniffiClonePointer(), + $(for arg in &method.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) + ), + $(DartCodeOracle::async_poll(method, type_helper.get_ci())), + $(DartCodeOracle::async_complete(method, type_helper.get_ci())), + $(DartCodeOracle::async_free(method, type_helper.get_ci())), + $async_lifter, + $error_handler, + ) + ))) } } } else if ret == quote!(void) { quote! { @override $ret $method_name($(for a in &dart_args => $a,)) { - return rustCall((status) { - $(method.ffi_func().name())( - uniffiClonePointer(), - $(for arg in &method.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) - status - ); - }, $error_handler); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, false, quote!( + rustCall((status) { + $(method.ffi_func().name())( + uniffiClonePointer(), + $(for arg in &method.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) + status + ); + }, $error_handler) + ))) } } } else { quote! { @override $ret $method_name($(for a in &dart_args => $a,)) { - return rustCallWithLifter( - (status) => $(method.ffi_func().name())( - uniffiClonePointer(), - $(for arg in &method.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) - status - ), - $lifter, - $error_handler - ); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, false, quote!( + rustCallWithLifter( + (status) => $(method.ffi_func().name())( + uniffiClonePointer(), + $(for arg in &method.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) + status + ), + $lifter, + $error_handler + ) + ))) } } } @@ -490,6 +511,8 @@ pub fn generate_method(func: &Method, type_helper: &dyn TypeHelperRenderer) -> d quote!(null) }; + let has_borrowed = DartCodeOracle::any_borrowed_bytes(&func.arguments()); + if func.is_async() { // For async methods returning objects, we need to convert the int pointer to Pointer let async_lifter = if let Some(ret_type) = func.return_type() { @@ -505,42 +528,48 @@ pub fn generate_method(func: &Method, type_helper: &dyn TypeHelperRenderer) -> d quote!( Future<$ret> $(DartCodeOracle::fn_name(func.name()))($args) { - return uniffiRustCallAsync( - () => $(func.ffi_func().name())( - uniffiClonePointer(), - $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) - ), - $(DartCodeOracle::async_poll(func, type_helper.get_ci())), - $(DartCodeOracle::async_complete(func, type_helper.get_ci())), - $(DartCodeOracle::async_free(func, type_helper.get_ci())), - $async_lifter, - $error_handler, - ); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, true, quote!( + uniffiRustCallAsync( + () => $(func.ffi_func().name())( + uniffiClonePointer(), + $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) + ), + $(DartCodeOracle::async_poll(func, type_helper.get_ci())), + $(DartCodeOracle::async_complete(func, type_helper.get_ci())), + $(DartCodeOracle::async_free(func, type_helper.get_ci())), + $async_lifter, + $error_handler, + ) + ))) } ) } else if ret == quote!(void) { quote!( $ret $(DartCodeOracle::fn_name(func.name()))($args) { - return rustCall((status) { - $(func.ffi_func().name())( - uniffiClonePointer(), - $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) status - ); - }, $error_handler); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, false, quote!( + rustCall((status) { + $(func.ffi_func().name())( + uniffiClonePointer(), + $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) status + ); + }, $error_handler) + ))) } ) } else { quote!( $ret $(DartCodeOracle::fn_name(func.name()))($args) { - return rustCallWithLifter( - (status) => $(func.ffi_func().name())( - uniffiClonePointer(), - $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) status - ), - $lifter, - $error_handler - ); + $(DartCodeOracle::wrap_ffi_call_stmt(has_borrowed, false, quote!( + rustCallWithLifter( + (status) => $(func.ffi_func().name())( + uniffiClonePointer(), + $(for arg in &func.arguments() => $(DartCodeOracle::lower_arg_with_callback_handling(arg)),) status + ), + $lifter, + $error_handler + ) + ))) } ) } @@ -629,6 +658,18 @@ fn trait_method_call( ) -> dart::Tokens { assert_eq!(method.arguments().len(), arg_exprs.len()); + // These derived trait methods (Display/Debug/Eq/Hash) never take a borrowed + // `&[u8]`, so — unlike the other call-site generators — this path does not wrap + // the call in an `Arena` and lowers args via `type_lower_fn` directly. If a + // future trait-derived method DID take borrowed bytes, `type_lower_fn` would + // fall through to the owned `RustBuffer` lowering (the wrong FFI type) and the + // native copy would leak — such a method must be routed through + // `wrap_ffi_call_stmt` instead. + debug_assert!( + method.arguments().iter().all(|a| !a.is_borrowed_bytes()), + "trait_method_call does not handle borrowed &[u8] args; route through wrap_ffi_call_stmt", + ); + let ffi_name = method.ffi_func().name(); let error_handler = if let Some(error_type) = method.throws_type() { diff --git a/src/gen/oracle.rs b/src/gen/oracle.rs index 2e51190f..304e7b03 100644 --- a/src/gen/oracle.rs +++ b/src/gen/oracle.rs @@ -1,7 +1,9 @@ use genco::lang::dart; use genco::quote; use heck::{ToLowerCamelCase, ToUpperCamelCase}; -use uniffi_bindgen::interface::{Argument, AsType, Callable, FfiType, Object, ObjectImpl, Type}; +use uniffi_bindgen::interface::{ + Argument, AsType, Callable, FfiType, Object, ObjectImpl, TraitKind, Type, +}; use uniffi_bindgen::ComponentInterface; // use super::render::{AsRenderable, Renderable}; @@ -545,14 +547,95 @@ impl DartCodeOracle { /// Lower argument with special handling for callback traits pub fn lower_arg_with_callback_handling(arg: &Argument) -> dart::Tokens { + // Borrowed `&[u8]` / `[ByRef] bytes` arguments take the `ForeignBytes` + // FFI path, not the owned `RustBuffer` path. uniffi 0.32 selects + // `FfiType::ForeignBytes` for these (see `Argument::is_borrowed_bytes`), + // so the FFI signature already expects `ForeignBytes`; lower the + // `Uint8List` into one instead of a `RustBuffer`. "Borrowed" is the + // Rust view (it borrows for the call rather than owning a RustBuffer); + // the emitted `lowerForeignBytes` still copies into native memory since + // a GC-managed `Uint8List` has no stable address. Mirrors the ByRef-bytes + // converter in uniffi's Kotlin/Python backends. + // + // The copy is allocated from `_uniffiArena` — an `Arena` the call site + // wraps around the FFI call (see `wrap_ffi_call_stmt`/`wrap_ffi_call_expr`) + // so the native memory is freed after the call instead of leaking. Any call + // site emitting a borrowed-bytes argument therefore has `_uniffiArena` in + // scope. The name is `_uniffi`-prefixed so it cannot collide with a + // host-supplied argument name (which are lower-camel-cased, never + // `_`-prefixed). + if arg.is_borrowed_bytes() { + return quote!(lowerForeignBytes($(Self::var_name(arg.name())), _uniffiArena)); + } let base_lower = Self::type_lower_fn(&arg.as_type(), quote!($(Self::var_name(arg.name())))); match arg.as_type() { - Type::Object { imp: ObjectImpl::CallbackTrait, .. } => base_lower, + Type::Object { + imp: ObjectImpl::Trait(TraitKind::Both | TraitKind::ForeignOnly), + .. + } => base_lower, Type::CallbackInterface { .. } => quote!($base_lower.address), _ => base_lower, } } + /// True if any argument takes the borrowed `&[u8]` (`ForeignBytes`) path, + /// whose native copy the call site must free after the FFI call. + pub fn any_borrowed_bytes(args: &[&Argument]) -> bool { + args.iter().any(|a| a.is_borrowed_bytes()) + } + + /// Wrap a full FFI-call statement so the native copies of borrowed `&[u8]` + /// arguments (see `lower_arg_with_callback_handling`) are freed after the + /// call. `call` is the call expression, without `return`/`;`. + /// + /// When no argument is borrowed bytes this is exactly `return call;`, so + /// ordinary calls keep their previous shape with zero overhead. Otherwise an + /// `Arena` named `_uniffiArena` (which the lowering allocates from) wraps the + /// call: + /// - sync: `using` frees the arena on scope exit, including if the call + /// throws — the Rust side only borrows for the duration of the call; + /// - async: the Rust future holds the borrow until it settles, so the arena + /// is released via `whenComplete` after the returned future completes. + /// + /// NOTE: the async+borrowed-bytes combination is currently unreachable — + /// uniffi 0.32 rejects a borrowed `&[u8]` on an `async fn` (the returned + /// future would capture the raw pointer and is required to be `Send + 'static`, + /// which a borrow is not). The `is_async` arm is therefore defensive: it keeps + /// the free correct-by-construction if uniffi ever gains owned-copy async + /// by-ref support, rather than silently leaking. + pub fn wrap_ffi_call_stmt( + has_borrowed: bool, + is_async: bool, + call: dart::Tokens, + ) -> dart::Tokens { + if !has_borrowed { + return quote!(return $call;); + } + if is_async { + quote! { + final _uniffiArena = Arena(); + return $call.whenComplete(_uniffiArena.releaseAll); + } + } else { + quote! { + return using((Arena _uniffiArena) { + return $call; + }); + } + } + } + + /// Expression form of [`wrap_ffi_call_stmt`] for constructor initializer + /// lists (`_ptr = `), which cannot host statements. Sync only — the + /// async constructor uses the statement form. + pub fn wrap_ffi_call_expr(has_borrowed: bool, call: dart::Tokens) -> dart::Tokens { + if has_borrowed { + quote!(using((Arena _uniffiArena) => $call)) + } else { + call + } + } + pub fn object_interface_name(_ci: &ComponentInterface, obj: &Object) -> String { let class_name = Self::class_name(obj.name()); if obj.has_callback_interface() || obj.is_trait_interface() { diff --git a/src/gen/primitives/macros.rs b/src/gen/primitives/macros.rs index ef30df2f..10b23f18 100644 --- a/src/gen/primitives/macros.rs +++ b/src/gen/primitives/macros.rs @@ -9,7 +9,7 @@ macro_rules! impl_code_type_for_primitive { $class_name.into() } - fn literal(&self, literal: &uniffi_bindgen::pipeline::general::nodes::Literal) -> String { + fn literal(&self, literal: &uniffi_bindgen::interface::Literal) -> String { $crate::gen::primitives::render_literal(&literal) } diff --git a/src/gen/primitives/mod.rs b/src/gen/primitives/mod.rs index 1f030c8f..db582b1b 100644 --- a/src/gen/primitives/mod.rs +++ b/src/gen/primitives/mod.rs @@ -13,9 +13,6 @@ use uniffi_bindgen::interface::{ DefaultValue as InterfaceDefaultValue, Literal as InterfaceLiteral, Radix as InterfaceRadix, Type as InterfaceType, }; -use uniffi_bindgen::pipeline::general::nodes::{ - Literal as PipelineLiteral, Radix as PipelineRadix, Type as PipelineType, TypeNode, -}; use crate::gen::render::{Renderable, TypeHelperRenderer}; use crate::gen::CodeType; @@ -29,43 +26,44 @@ pub(crate) fn escape_dart_string(value: &str) -> String { .replace('\t', "\\t") } -fn render_literal(literal: &PipelineLiteral) -> String { - fn typed_number(type_node: &TypeNode, num_str: String) -> String { - match &type_node.ty { - PipelineType::Int8 - | PipelineType::UInt8 - | PipelineType::Int16 - | PipelineType::UInt16 - | PipelineType::Int32 - | PipelineType::UInt32 - | PipelineType::UInt64 - | PipelineType::Float32 - | PipelineType::Float64 - | PipelineType::Duration => num_str, +fn render_literal(literal: &InterfaceLiteral) -> String { + fn typed_number(ty: &InterfaceType, num_str: String) -> String { + match ty { + InterfaceType::Int8 + | InterfaceType::UInt8 + | InterfaceType::Int16 + | InterfaceType::UInt16 + | InterfaceType::Int32 + | InterfaceType::UInt32 + | InterfaceType::Int64 + | InterfaceType::UInt64 + | InterfaceType::Float32 + | InterfaceType::Float64 + | InterfaceType::Duration => num_str, _ => panic!("Unexpected literal: {num_str} is not a number"), } } match literal { - PipelineLiteral::Boolean(v) => format!("{v}"), - PipelineLiteral::String(s) => format!("'{}'", escape_dart_string(s)), - PipelineLiteral::Int(i, radix, type_node) => typed_number( - type_node, + InterfaceLiteral::Boolean(v) => format!("{v}"), + InterfaceLiteral::String(s) => format!("'{}'", escape_dart_string(s)), + InterfaceLiteral::Int(i, radix, ty) => typed_number( + ty, match radix { - PipelineRadix::Octal => format!("{i:#x}"), - PipelineRadix::Decimal => format!("{i}"), - PipelineRadix::Hexadecimal => format!("{i:#x}"), + InterfaceRadix::Octal => format!("{i:#x}"), + InterfaceRadix::Decimal => format!("{i}"), + InterfaceRadix::Hexadecimal => format!("{i:#x}"), }, ), - PipelineLiteral::UInt(i, radix, type_node) => typed_number( - type_node, + InterfaceLiteral::UInt(i, radix, ty) => typed_number( + ty, match radix { - PipelineRadix::Octal => format!("{i:#x}"), - PipelineRadix::Decimal => format!("{i}"), - PipelineRadix::Hexadecimal => format!("{i:#x}"), + InterfaceRadix::Octal => format!("{i:#x}"), + InterfaceRadix::Decimal => format!("{i}"), + InterfaceRadix::Hexadecimal => format!("{i:#x}"), }, ), - PipelineLiteral::Float(string, type_node) => typed_number(type_node, string.clone()), + InterfaceLiteral::Float(string, ty) => typed_number(ty, string.clone()), _ => unreachable!("Literal"), } } @@ -115,6 +113,11 @@ where .or_else(|| enum_variant_renderer(field_type, variant)), InterfaceLiteral::EmptySequence => Some("const []".to_string()), InterfaceLiteral::EmptyMap => Some("const {}".to_string()), + // `Set` types are not yet rendered by this generator (every Set path + // is a `todo!`), so there is no Set-typed position for `const {}` to land + // in — it would emit an empty Map. Degrade to no-default (→ required), + // mirroring `default_for_interface_type`, until real Set support exists. + InterfaceLiteral::EmptySet => None, InterfaceLiteral::None => Some("null".to_string()), InterfaceLiteral::Some { inner } => { render_interface_default_value(inner, field_type, enum_variant_renderer) diff --git a/src/gen/records.rs b/src/gen/records.rs index bf70ef0b..a43a4c91 100644 --- a/src/gen/records.rs +++ b/src/gen/records.rs @@ -1,6 +1,5 @@ use genco::prelude::*; -use uniffi_bindgen::interface::{AsType, Record, Type}; -use uniffi_bindgen::pipeline::general::nodes::Literal as PipelineLiteral; +use uniffi_bindgen::interface::{AsType, Literal as PipelineLiteral, Record, Type}; use super::defaults::render_default_value; use super::oracle::{AsCodeType, DartCodeOracle}; diff --git a/src/gen/types.rs b/src/gen/types.rs index de5d7c92..7c7157c6 100644 --- a/src/gen/types.rs +++ b/src/gen/types.rs @@ -424,6 +424,40 @@ pub fn runtime_scaffolding(ci: &ComponentInterface) -> dart::Tokens { return RustBuffer.fromBytes(bytes.ref); } + // Lowers a `Uint8List` into a `ForeignBytes` for a borrowed `&[u8]` + // (`[ByRef] bytes`) argument. The Rust side only borrows the buffer + // for the duration of the call, so this is valid in argument + // position only (never lifted or read back). Dart's GC-managed + // `Uint8List` has no stable native address, so the bytes are copied + // into native memory allocated from the caller-supplied `alloc`. + // Both the struct and the copied buffer come from `alloc`, so the + // caller frees them by releasing that allocator (an `Arena` scoped + // around the FFI call) — no native memory leaks per call. + ForeignBytes lowerForeignBytes(Uint8List data, Allocator alloc) { + final length = data.length; + // `ForeignBytes.len` is an Int32; fail loudly rather than + // silently truncate a >2GiB buffer into a bogus length. + if (length > 0x7fffffff) { + throw ArgumentError( + "Uint8List too large for a borrowed &[u8] FFI argument: " + length.toString() + " bytes"); + } + final bytes = alloc(); + if (length == 0) { + // Empty slice: pass (null, 0). `alloc(0)` is + // platform-variable (some allocators return null and make + // `calloc` throw); Rust reads (null, 0) as `&[]`. Mirrors the + // ByRef-bytes converters in uniffi's Kotlin/Python backends. + bytes.ref.len = 0; + bytes.ref.data = Pointer.fromAddress(0); + return bytes.ref; + } + final Pointer frameData = alloc(length); + frameData.asTypedList(length).setAll(0, data); + bytes.ref.len = length; + bytes.ref.data = frameData; + return bytes.ref; + } + final class ForeignBytes extends Struct { @Int32() external int len;