From 001bd08882c709f3ea244d5441feab4d9d94f555 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Mon, 30 Sep 2024 13:48:17 +1300 Subject: [PATCH] [ffigen] Return structs by value from ObjC methods (#1556) --- pkgs/ffigen/CHANGELOG.md | 3 + .../src/code_generator/objc_interface.dart | 100 ++++++++---------- pkgs/ffigen/pubspec.yaml | 4 +- .../test/native_objc_test/block_test.dart | 4 +- .../test/native_objc_test/method_test.dart | 10 +- .../test/native_objc_test/property_test.dart | 5 +- 6 files changed, 56 insertions(+), 70 deletions(-) diff --git a/pkgs/ffigen/CHANGELOG.md b/pkgs/ffigen/CHANGELOG.md index ceb33ba38..4f3aa0f78 100644 --- a/pkgs/ffigen/CHANGELOG.md +++ b/pkgs/ffigen/CHANGELOG.md @@ -1,5 +1,6 @@ ## 15.0.0-wip +- Bump minimum Dart version to 3.4. - Dedupe `ObjCBlock` trampolines to reduce generated ObjC code. - ObjC objects now include the methods from the protocols they implement. Both required and optional methods are included. Optional methods will throw an @@ -16,6 +17,8 @@ - `sort:` config option now affects ObjC interface/protocol methods. - Fix a bug where `NSRange` was not being imported from package:objective_c: https://github.com/dart-lang/native/issues/1180 +- __Breaking change__: Return structs from ObjC methods by value instead of + taking a struct return pointer. ## 14.0.1 diff --git a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart index ce301ac82..ffb7ad729 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart @@ -121,28 +121,17 @@ class ObjCInterface extends BindingType with ObjCMethods { for (final m in methods) { final methodName = m.getDartMethodName(methodNamer); final isStatic = m.isClassMethod; - final isStret = m.msgSend!.isStret; - - var returnType = m.returnType; - var params = m.params; - if (isStret) { - params = [ - Parameter( - name: 'stret', - type: PointerType(returnType), - objCConsumed: false), - ...params - ]; - returnType = voidType; - } + + final returnType = m.returnType; + final returnTypeStr = _getConvertedType(returnType, w, name); + final params = m.params; // The method declaration. s.write('\n '); s.write(makeDartDoc(m.dartDoc ?? m.originalName)); s.write(' '); if (isStatic) { - s.write('static '); - s.write(_getConvertedType(returnType, w, name)); + s.write('static $returnTypeStr'); switch (m.kind) { case ObjCMethodKind.method: @@ -165,21 +154,12 @@ class ObjCInterface extends BindingType with ObjCMethods { switch (m.kind) { case ObjCMethodKind.method: // returnType methodName(...) - s.write(_getConvertedType(returnType, w, name)); - s.write(' $methodName'); + s.write('$returnTypeStr $methodName'); s.write(paramsToString(params)); break; case ObjCMethodKind.propertyGetter: - s.write(_getConvertedType(returnType, w, name)); - if (isStret) { - // void getMethodName(Pointer stret) - s.write(' get'); - s.write(methodName[0].toUpperCase() + methodName.substring(1)); - s.write(paramsToString(params)); - } else { - // returnType get methodName - s.write(' get $methodName'); - } + // returnType get methodName + s.write('$returnTypeStr get $methodName'); break; case ObjCMethodKind.propertySetter: // set methodName(...) @@ -204,36 +184,50 @@ class ObjCInterface extends BindingType with ObjCMethods { final convertReturn = m.kind != ObjCMethodKind.propertySetter && !returnType.sameDartAndFfiDartType; - if (returnType != voidType) { - s.write(' ${convertReturn ? 'final _ret = ' : 'return '}'); - } - s.write(m.msgSend!.invoke( - w, - isStatic - ? _classObject.name - : convertDartTypeToFfiDartType( - w, - 'this', - objCRetain: m.consumesSelf, - objCAutorelease: false, - ), - sel, + final target = isStatic + ? _classObject.name + : convertDartTypeToFfiDartType( + w, + 'this', + objCRetain: m.consumesSelf, + objCAutorelease: false, + ); + final msgSendParams = m.params.map((p) => p.type.convertDartTypeToFfiDartType( w, p.name, objCRetain: p.objCConsumed, objCAutorelease: false, - )), - structRetPtr: 'stret')); - s.write(';\n'); - if (convertReturn) { - final result = returnType.convertFfiDartTypeToDartType( - w, - '_ret', - objCRetain: !m.returnsRetained, - objCEnclosingClass: name, - ); - s.write(' return $result;'); + )); + if (m.msgSend!.isStret) { + assert(!convertReturn); + final calloc = '${w.ffiPkgLibraryPrefix}.calloc'; + final sizeOf = '${w.ffiLibraryPrefix}.sizeOf'; + final uint8Type = NativeType(SupportedNativeType.uint8).getCType(w); + final invoke = m.msgSend! + .invoke(w, target, sel, msgSendParams, structRetPtr: '_ptr'); + s.write(''' + final _ptr = $calloc<$returnTypeStr>(); + $invoke; + final _finalizable = _ptr.cast<$uint8Type>().asTypedList( + $sizeOf<$returnTypeStr>(), finalizer: $calloc.nativeFree); + return ${w.ffiLibraryPrefix}.Struct.create<$returnTypeStr>(_finalizable); +'''); + } else { + if (returnType != voidType) { + s.write(' ${convertReturn ? 'final _ret = ' : 'return '}'); + } + s.write(m.msgSend!.invoke(w, target, sel, msgSendParams)); + s.write(';\n'); + if (convertReturn) { + final result = returnType.convertFfiDartTypeToDartType( + w, + '_ret', + objCRetain: !m.returnsRetained, + objCEnclosingClass: name, + ); + s.write(' return $result;'); + } } s.write('\n }\n'); diff --git a/pkgs/ffigen/pubspec.yaml b/pkgs/ffigen/pubspec.yaml index b818f9462..e4fab2612 100644 --- a/pkgs/ffigen/pubspec.yaml +++ b/pkgs/ffigen/pubspec.yaml @@ -14,7 +14,7 @@ topics: - codegen environment: - sdk: '>=3.3.0 <4.0.0' + sdk: '>=3.4.0 <4.0.0' dependencies: args: ^2.0.0 @@ -38,7 +38,7 @@ dev_dependencies: json_schema: ^5.1.1 leak_tracker: ^10.0.7 meta: ^1.11.0 - objective_c: ^2.0.0 + objective_c: ^2.1.0 test: ^1.16.2 dependency_overrides: diff --git a/pkgs/ffigen/test/native_objc_test/block_test.dart b/pkgs/ffigen/test/native_objc_test/block_test.dart index 4d4a78eec..8ca83af17 100644 --- a/pkgs/ffigen/test/native_objc_test/block_test.dart +++ b/pkgs/ffigen/test/native_objc_test/block_test.dart @@ -154,9 +154,7 @@ void main() { expect(result1.z, 7.8); expect(result1.w, 1.2); - final result2Ptr = arena(); - final result2 = result2Ptr.ref; - BlockTester.callVec4Block_(result2Ptr, block); + final result2 = BlockTester.callVec4Block_(block); expect(result2.x, 3.4); expect(result2.y, 5.6); expect(result2.z, 7.8); diff --git a/pkgs/ffigen/test/native_objc_test/method_test.dart b/pkgs/ffigen/test/native_objc_test/method_test.dart index 2d03fe879..7653be508 100644 --- a/pkgs/ffigen/test/native_objc_test/method_test.dart +++ b/pkgs/ffigen/test/native_objc_test/method_test.dart @@ -73,16 +73,13 @@ void main() { input.z = 5.6; input.w = 7.8; - final resultPtr = calloc(); - final result = resultPtr.ref; - testInstance.twiddleVec4Components_(resultPtr, input); + final result = testInstance.twiddleVec4Components_(input); expect(result.x, 3.4); expect(result.y, 5.6); expect(result.z, 7.8); expect(result.w, 1.2); calloc.free(inputPtr); - calloc.free(resultPtr); }); test('Floats', () { @@ -95,14 +92,11 @@ void main() { test('Method with same name as a type', () { // Test for https://github.com/dart-lang/native/issues/1007 - final resultPtr = calloc(); - final result = resultPtr.ref; - testInstance.Vec41(resultPtr); // A slightly unfortunate rename :P + final result = testInstance.Vec41(); // A slightly unfortunate rename :P expect(result.x, 1); expect(result.y, 2); expect(result.z, 3); expect(result.w, 4); - calloc.free(resultPtr); }); }); }); diff --git a/pkgs/ffigen/test/native_objc_test/property_test.dart b/pkgs/ffigen/test/native_objc_test/property_test.dart index d0067d704..caba1cc0c 100644 --- a/pkgs/ffigen/test/native_objc_test/property_test.dart +++ b/pkgs/ffigen/test/native_objc_test/property_test.dart @@ -59,17 +59,14 @@ void main() { input.z = 5.6; input.w = 7.8; - final resultPtr = calloc(); - final result = resultPtr.ref; testInstance.structProperty = input; - testInstance.getStructProperty(resultPtr); + final result = testInstance.structProperty; expect(result.x, 1.2); expect(result.y, 3.4); expect(result.z, 5.6); expect(result.w, 7.8); calloc.free(inputPtr); - calloc.free(resultPtr); }); test('Floats', () {