[ffigen] Return structs by value from ObjC methods (#1556)

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
 

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<returnType> 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');

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:

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<Vec4>();
-        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);

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<Vec4>();
-        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<Vec4>();
-        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);
       });
     });
   });

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<Vec4>();
-        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', () {