Merge remote-tracking branch 'upstream/main' into stream

dart-lang · Sep 30, 2024 · f725e2a · f725e2a
2 parents 67d6f18 + f664a30
commit f725e2a
Show file tree

Hide file tree

Showing 113 changed files with 1,772 additions and 1,838 deletions.
diff --git a/.github/workflows/ffigen.yml b/.github/workflows/ffigen.yml
@@ -37,7 +37,7 @@ jobs:
           channel: 'stable'
       - id: install
         name: Install dependencies
-        run: flutter pub get && flutter pub get --directory="example/shared_bindings"
+        run: flutter pub get && flutter pub get --directory="example/shared_bindings" && flutter pub get --directory="../objective_c"
       - name: Check formatting
         run: dart format --output=none --set-exit-if-changed .
         if: always() && steps.install.outcome == 'success'
@@ -79,7 +79,7 @@ jobs:
         with:
           channel: 'stable'
       - name: Install dependencies
-        run: flutter pub get
+        run: flutter pub get && flutter pub get --directory="../objective_c"
       - name: Build test dylib and bindings
         run: dart test/setup.dart
       - name: Run VM tests and collect coverage
@@ -110,9 +110,9 @@ jobs:
         with:
           channel: 'stable'
       - name: Install dependencies
-        run: flutter pub get
+        run: flutter pub get && flutter pub get --directory="../objective_c"
       - name: Build test dylib and bindings
-        run: dart test/setup.dart
+        run: dart test/setup.dart --main-thread-dispatcher
       - name: Run Flutter tests
         run: flutter test
 

diff --git a/.github/workflows/ffigen_weekly.yml b/.github/workflows/ffigen_weekly.yml
@@ -25,7 +25,7 @@ jobs:
           flutter-version: 3.19.0
           channel: 'stable'
       - name: Install dependencies
-        run: flutter pub get
+        run: flutter pub get && flutter pub get --directory="../objective_c"
       - name: Build test dylib and bindings
         run: dart test/setup.dart
       - name: Run VM tests

diff --git a/.github/workflows/native.yaml b/.github/workflows/native.yaml
@@ -52,7 +52,7 @@ jobs:
 
       - uses: nttld/setup-ndk@afb4c9964b521afb97c864b7d40b11e6911bd410
         with:
-          ndk-version: r26b
+          ndk-version: r27
         if: ${{ matrix.os != 'macos' }}
 
       - run: dart pub get

diff --git a/.github/workflows/native_toolchain_c.yaml b/.github/workflows/native_toolchain_c.yaml
@@ -40,7 +40,7 @@ jobs:
 
       - uses: nttld/setup-ndk@afb4c9964b521afb97c864b7d40b11e6911bd410
         with:
-          ndk-version: r26b
+          ndk-version: r27
         if: ${{ matrix.sdk == 'stable' }}
 
       - run: dart pub get

diff --git 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.
 - Update to latest `package:objective_c`.
 - ObjC objects now include the methods from the protocols they implement. Both
@@ -15,6 +16,10 @@
   ObjC but before the invocation was received by Dart:
   https://github.com/dart-lang/native/issues/1571
 - `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/compound.dart b/pkgs/ffigen/lib/src/code_generator/compound.dart
@@ -105,7 +105,7 @@ abstract class Compound extends BindingType {
   }
 
   bool get _isBuiltIn =>
-      objCBuiltInFunctions?.isBuiltInCompound(originalName) ?? false;
+      objCBuiltInFunctions?.getBuiltInCompoundName(originalName) != null;
 
   @override
   BindingString toBindingString(Writer w) {
@@ -188,7 +188,11 @@ abstract class Compound extends BindingType {
   bool get isIncompleteCompound => isIncomplete;
 
   @override
-  String getCType(Writer w) => _isBuiltIn ? '${w.objcPkgPrefix}.$name' : name;
+  String getCType(Writer w) {
+    final builtInName =
+        objCBuiltInFunctions?.getBuiltInCompoundName(originalName);
+    return builtInName != null ? '${w.objcPkgPrefix}.$builtInName' : name;
+  }
 
   @override
   String getNativeType({String varName = ''}) => '$nativeType $varName';

diff --git a/pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart b/pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart
@@ -43,7 +43,7 @@ class ObjCBuiltInFunctions {
       ObjCImport('UnimplementedOptionalMethodException');
 
   // Keep in sync with pkgs/objective_c/ffigen_objc.yaml.
-  static const builtInInterfaces = {
+  static const _builtInInterfaces = {
     'DartInputStreamAdapter',
     'DartProxy',
     'DartProxyBuilder',
@@ -82,11 +82,11 @@ class ObjCBuiltInFunctions {
     'NSValue',
     'Protocol',
   };
-  static const builtInCompounds = {
-    'NSFastEnumerationState',
-    'NSRange',
+  static const _builtInCompounds = {
+    'NSFastEnumerationState': 'NSFastEnumerationState',
+    '_NSRange': 'NSRange',
   };
-  static const builtInEnums = {
+  static const _builtInEnums = {
     'NSBinarySearchingOptions',
     'NSComparisonResult',
     'NSDataBase64DecodingOptions',
@@ -114,11 +114,11 @@ class ObjCBuiltInFunctions {
   // TODO(https://github.com/dart-lang/native/issues/1173): Ideally this check
   // would be based on more than just the name.
   bool isBuiltInInterface(String name) =>
-      !generateForPackageObjectiveC && builtInInterfaces.contains(name);
-  bool isBuiltInCompound(String name) =>
-      !generateForPackageObjectiveC && builtInCompounds.contains(name);
+      !generateForPackageObjectiveC && _builtInInterfaces.contains(name);
+  String? getBuiltInCompoundName(String name) =>
+      generateForPackageObjectiveC ? null : _builtInCompounds[name];
   bool isBuiltInEnum(String name) =>
-      !generateForPackageObjectiveC && builtInEnums.contains(name);
+      !generateForPackageObjectiveC && _builtInEnums.contains(name);
   bool isNSObject(String name) => name == 'NSObject';
 
   // We need to load a separate instance of objc_msgSend for each signature. If

diff --git 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<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');

diff --git 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

diff --git a/pkgs/ffigen/test/native_objc_test/arc_config.yaml b/pkgs/ffigen/test/native_objc_test/arc_config.yaml
@@ -1,7 +1,9 @@
 name: ArcTestObjCLibrary
 description: 'Tests ARC'
 language: objc
-output: 'arc_bindings.dart'
+output:
+  bindings: 'arc_bindings.dart'
+  objc-bindings: 'arc_bindings.m'
 exclude-all-by-default: true
 functions:
   include:
@@ -10,8 +12,9 @@ functions:
 objc-interfaces:
   include:
     - ArcTestObject
+    - ArcDtorTestObject
 headers:
   entry-points:
-    - 'arc_test.m'
+    - 'arc_test.h'
 preamble: |
   // ignore_for_file: camel_case_types, non_constant_identifier_names, unnecessary_non_null_assertion, unused_element, unused_field
diff --git a/pkgs/ffigen/test/native_objc_test/arc_test.dart b/pkgs/ffigen/test/native_objc_test/arc_test.dart
@@ -20,7 +20,7 @@ import 'util.dart';
 void main() {
   late ArcTestObjCLibrary lib;
 
-  group('Reference counting', () {
+  group('ARC', () {
     setUpAll(() {
       // TODO(https://github.com/dart-lang/native/issues/1068): Remove this.
       DynamicLibrary.open('../objective_c/test/objective_c.dylib');
@@ -474,5 +474,23 @@ void main() {
       expect(counter.value, 0);
       calloc.free(counter);
     }, skip: !canDoGC);
+
+    test('Destroy on main thread', () async {
+      const numTestObjects = 1000;
+
+      final dtorCounter = calloc<Int32>();
+      final dtorOnMainThreadCounter = calloc<Int32>();
+      final objects = <ArcDtorTestObject>[];
+      for (var i = 0; i < numTestObjects; ++i) {
+        objects.add(ArcDtorTestObject.alloc().initWithCounters_onMainThread_(
+            dtorCounter, dtorOnMainThreadCounter));
+      }
+      objects.clear();
+
+      while (dtorCounter.value < numTestObjects) {
+        await flutterDoGC();
+      }
+      expect(dtorOnMainThreadCounter.value, numTestObjects);
+    }, skip: !isFlutterTester);
   });
 }
diff --git a/pkgs/ffigen/test/native_objc_test/arc_test.h b/pkgs/ffigen/test/native_objc_test/arc_test.h
@@ -0,0 +1,41 @@
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#import <Foundation/NSObject.h>
+
+void objc_autoreleasePoolPop(void *pool);
+void *objc_autoreleasePoolPush();
+
+@interface ArcTestObject : NSObject {
+  int32_t* counter;
+}
+
++ (instancetype)allocTheThing;
++ (instancetype)newWithCounter:(int32_t*) _counter;
+- (instancetype)initWithCounter:(int32_t*) _counter;
++ (ArcTestObject*)makeAndAutorelease:(int32_t*) _counter;
+- (void)setCounter:(int32_t*) _counter;
+- (void)dealloc;
+- (ArcTestObject*)copyMe;
+- (ArcTestObject*)mutableCopyMe;
+- (id)copyWithZone:(NSZone*) zone;
+- (ArcTestObject*)returnsRetained NS_RETURNS_RETAINED;
+- (ArcTestObject*)copyMeNoRetain __attribute__((ns_returns_not_retained));
+- (ArcTestObject*)copyMeAutorelease __attribute__((ns_returns_autoreleased));
+- (ArcTestObject*)copyMeConsumeSelf __attribute__((ns_consumes_self));
++ (void)consumeArg:(ArcTestObject*) __attribute((ns_consumed)) arg;
+
+@property (assign) ArcTestObject* assignedProperty;
+@property (retain) ArcTestObject* retainedProperty;
+@property (copy) ArcTestObject* copiedProperty;
+
+@end
+
+@interface ArcDtorTestObject : NSObject {
+  int32_t* dtorCounter;
+  int32_t* dtorOnMainThreadCounter;
+}
+- (instancetype)initWithCounters:(int32_t*) _dtorCounter
+    onMainThread: (int32_t*) _dtorOnMainThreadCounter;
+@end