[ffigen] Method families

pkgs/ffigen/CHANGELOG.md

@@ -18,6 +18,8 @@
   if you had a line like `s.requires_arc = []` in your podspec, this should
   either be removed, or you should add the ffigen generated ObjC code to the
   list. If you're compiling directly with clang, add the `-fobjc-arc` flag.
+- Fix some bugs in the way ObjC method families and ownership annotations were
+  being handled: https://github.com/dart-lang/native/issues/1446
 
 ## 13.0.0
 

pkgs/ffigen/lib/src/code_generator/objc_interface.dart

@@ -188,7 +188,10 @@ class ObjCInterface extends BindingType with ObjCMethods {
       }
       s.write(m.msgSend!.invoke(
           w,
-          isStatic ? _classObject.name : 'this.pointer',
+          isStatic
+              ? _classObject.name
+              : convertDartTypeToFfiDartType(w, 'this',
+                  objCRetain: m.consumesSelf),
           m.selObject!.name,
           m.params.map((p) => p.type
               .convertDartTypeToFfiDartType(w, p.name, objCRetain: false)),
@@ -198,7 +201,7 @@ class ObjCInterface extends BindingType with ObjCMethods {
         final result = returnType.convertFfiDartTypeToDartType(
           w,
           '_ret',
-          objCRetain: !m.isOwnedReturn,
+          objCRetain: !m.returnsRetained,
           objCEnclosingClass: name,
         );
         s.write('    return $result;');

pkgs/ffigen/lib/src/code_generator/objc_methods.dart

@@ -107,6 +107,51 @@ enum ObjCMethodKind {
   propertySetter,
 }
 
+enum ObjCMethodOwnership {
+  retained,
+  notRetained,
+  autoreleased,
+}
+
+// In ObjC, the name of a method affects its ref counting semantics. See
+// https://clang.llvm.org/docs/AutomaticReferenceCounting.html#method-families
+enum ObjCMethodFamily {
+  alloc('alloc', returnsRetained: true, consumesSelf: false),
+  init('init', returnsRetained: true, consumesSelf: true),
+  new_('new', returnsRetained: true, consumesSelf: false),
+  copy('copy', returnsRetained: true, consumesSelf: false),
+  mutableCopy('mutableCopy', returnsRetained: true, consumesSelf: false);
+
+  final String name;
+  final bool returnsRetained;
+  final bool consumesSelf;
+
+  const ObjCMethodFamily(this.name,
+      {required this.returnsRetained, required this.consumesSelf});
+
+  static ObjCMethodFamily? parse(String methodName) {
+    final name = methodName.substring(_findFamilyStart(methodName));
+    for (final family in ObjCMethodFamily.values) {
+      if (_matchesFamily(name, family.name)) return family;
+    }
+    return null;
+  }
+
+  static int _findFamilyStart(String methodName) {
+    for (var i = 0; i < methodName.length; ++i) {
+      if (methodName[i] != '_') return i;
+    }
+    return methodName.length;
+  }
+
+  static final _lowerCase = RegExp('[a-z]');
+  static bool _matchesFamily(String name, String familyName) {
+    if (!name.startsWith(familyName)) return false;
+    final tail = name.substring(familyName.length);
+    return !tail.startsWith(_lowerCase);
+  }
+}
+
 class ObjCProperty {
   final String originalName;
   String? dartName;
@@ -123,7 +168,9 @@ class ObjCMethod {
   final ObjCMethodKind kind;
   final bool isClassMethod;
   final bool isOptional;
-  bool returnsRetained = false;
+  ObjCMethodOwnership? ownershipAttribute;
+  final ObjCMethodFamily? family;
+  bool consumesSelfAttribute = false;
   ObjCInternalGlobal? selObject;
   ObjCMsgSendFunc? msgSend;
   late ObjCBlock protocolBlock;
@@ -136,6 +183,7 @@ class ObjCMethod {
     required this.isClassMethod,
     required this.isOptional,
     required this.returnType,
+    required this.family,
     List<ObjCMethodParam>? params_,
   }) : params = params_ ?? [];
 
@@ -201,12 +249,15 @@ class ObjCMethod {
     return msgSend == other.msgSend;
   }
 
-  static final _copyRegExp = RegExp('[cC]opy');
-  bool get isOwnedReturn =>
-      returnsRetained ||
-      originalName.startsWith('new') ||
-      originalName.startsWith('alloc') ||
-      originalName.contains(_copyRegExp);
+  bool get returnsRetained {
+    if (ownershipAttribute != null) {
+      return ownershipAttribute == ObjCMethodOwnership.retained;
+    }
+    return family?.returnsRetained ?? false;
+  }
+
+  bool get consumesSelf =>
+      consumesSelfAttribute || (family?.consumesSelf ?? false);
 
   Iterable<Type> get childTypes sync* {
     yield returnType;
@@ -216,7 +267,7 @@ class ObjCMethod {
   }
 
   @override
-  String toString() => '${isOptional ? "@optional " : ""}$returnType '
+  String toString() => '${isOptional ? '@optional ' : ''}$returnType '
       '$originalName(${params.join(', ')})';
 }
 

pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart

@@ -152,6 +152,7 @@ void _parseProperty(clang_types.CXCursor cursor, ObjCInterface itf) {
     isClassMethod: isClassMethod,
     isOptional: isOptionalMethod,
     returnType: fieldType,
+    family: null,
   );
   itf.addMethod(getter);
 
@@ -160,13 +161,15 @@ void _parseProperty(clang_types.CXCursor cursor, ObjCInterface itf) {
         .clang_Cursor_getObjCPropertySetterName(cursor)
         .toStringAndDispose();
     final setter = ObjCMethod(
-        originalName: setterName,
-        property: property,
-        dartDoc: dartDoc,
-        kind: ObjCMethodKind.propertySetter,
-        isClassMethod: isClassMethod,
-        isOptional: isOptionalMethod,
-        returnType: NativeType(SupportedNativeType.voidType));
+      originalName: setterName,
+      property: property,
+      dartDoc: dartDoc,
+      kind: ObjCMethodKind.propertySetter,
+      isClassMethod: isClassMethod,
+      isOptional: isOptionalMethod,
+      returnType: NativeType(SupportedNativeType.voidType),
+      family: null,
+    );
     setter.params.add(ObjCMethodParam(fieldType, 'value'));
     itf.addMethod(setter);
   }
@@ -204,6 +207,7 @@ ObjCMethod? parseObjCMethod(clang_types.CXCursor cursor, String itfName) {
     isClassMethod: isClassMethod,
     isOptional: isOptionalMethod,
     returnType: returnType,
+    family: ObjCMethodFamily.parse(methodName),
   );
   _logger.fine('       > ${isClassMethod ? 'Class' : 'Instance'} method: '
       '${method.originalName} ${cursor.completeStringRepr()}');
@@ -216,7 +220,16 @@ ObjCMethod? parseObjCMethod(clang_types.CXCursor cursor, String itfName) {
         }
         break;
       case clang_types.CXCursorKind.CXCursor_NSReturnsRetained:
-        method.returnsRetained = true;
+        method.ownershipAttribute = ObjCMethodOwnership.retained;
+        break;
+      case clang_types.CXCursorKind.CXCursor_NSReturnsNotRetained:
+        method.ownershipAttribute = ObjCMethodOwnership.notRetained;
+        break;
+      case clang_types.CXCursorKind.CXCursor_NSReturnsAutoreleased:
+        method.ownershipAttribute = ObjCMethodOwnership.autoreleased;
+        break;
+      case clang_types.CXCursorKind.CXCursor_NSConsumesSelf:
+        method.consumesSelfAttribute = true;
         break;
       default:
     }

pkgs/ffigen/test/native_objc_test/arc_test.dart

@@ -113,49 +113,104 @@ void main() {
     });
 
     (
+      Pointer<ObjCObject>,
+      Pointer<ObjCObject>,
+      Pointer<ObjCObject>,
+      Pointer<ObjCObject>,
       Pointer<ObjCObject>,
       Pointer<ObjCObject>,
       Pointer<ObjCObject>,
       Pointer<ObjCObject>,
       Pointer<ObjCObject>
     ) copyMethodsInner(Pointer<Int32> counter) {
+      final pool = lib.objc_autoreleasePoolPush();
       final obj1 = ArcTestObject.newWithCounter_(counter);
       expect(counter.value, 1);
       final obj2 = obj1.copyMe();
       expect(counter.value, 2);
-      final obj3 = obj1.makeACopy();
+      final obj3 = obj1.mutableCopyMe();
       expect(counter.value, 3);
       final obj4 = obj1.copyWithZone_(nullptr);
       expect(counter.value, 4);
       final obj5 = obj1.copy();
       expect(counter.value, 5);
+      final obj6 = obj1.returnsRetained();
+      expect(counter.value, 6);
+      final obj7 = obj1.copyMeNoRetain();
+      expect(counter.value, 7);
+      final obj8 = obj1.copyMeAutorelease();
+      expect(counter.value, 8);
+      final obj9 = obj1.copyMeConsumeSelf();
+      expect(counter.value, 9);
 
       final obj1raw = obj1.pointer;
       final obj2raw = obj2.pointer;
       final obj3raw = obj3.pointer;
       final obj4raw = obj4.pointer;
       final obj5raw = obj5.pointer;
+      final obj6raw = obj6.pointer;
+      final obj7raw = obj7.pointer;
+      final obj8raw = obj8.pointer;
+      final obj9raw = obj9.pointer;
 
       expect(objectRetainCount(obj1raw), 1);
       expect(objectRetainCount(obj2raw), 1);
       expect(objectRetainCount(obj3raw), 1);
       expect(objectRetainCount(obj4raw), 1);
       expect(objectRetainCount(obj5raw), 1);
+      expect(objectRetainCount(obj6raw), 1);
+      expect(objectRetainCount(obj7raw), 2); // One ref in autorelease pool.
+      expect(objectRetainCount(obj8raw), 2); // One ref in autorelease pool.
+      expect(objectRetainCount(obj9raw), 1);
 
-      return (obj1raw, obj2raw, obj3raw, obj4raw, obj5raw);
+      lib.objc_autoreleasePoolPop(pool);
+      expect(objectRetainCount(obj1raw), 1);
+      expect(objectRetainCount(obj2raw), 1);
+      expect(objectRetainCount(obj3raw), 1);
+      expect(objectRetainCount(obj4raw), 1);
+      expect(objectRetainCount(obj5raw), 1);
+      expect(objectRetainCount(obj6raw), 1);
+      expect(objectRetainCount(obj7raw), 1);
+      expect(objectRetainCount(obj8raw), 1);
+      expect(objectRetainCount(obj9raw), 1);
+
+      return (
+        obj1raw,
+        obj2raw,
+        obj3raw,
+        obj4raw,
+        obj5raw,
+        obj6raw,
+        obj7raw,
+        obj8raw,
+        obj9raw
+      );
     }
 
     test('copy methods ref count correctly', () {
       final counter = calloc<Int32>();
       counter.value = 0;
-      final (obj1raw, obj2raw, obj3raw, obj4raw, obj5raw) =
-          copyMethodsInner(counter);
+      final (
+        obj1raw,
+        obj2raw,
+        obj3raw,
+        obj4raw,
+        obj5raw,
+        obj6raw,
+        obj7raw,
+        obj8raw,
+        obj9raw
+      ) = copyMethodsInner(counter);
       doGC();
       expect(objectRetainCount(obj1raw), 0);
       expect(objectRetainCount(obj2raw), 0);
       expect(objectRetainCount(obj3raw), 0);
       expect(objectRetainCount(obj4raw), 0);
       expect(objectRetainCount(obj5raw), 0);
+      expect(objectRetainCount(obj6raw), 0);
+      expect(objectRetainCount(obj7raw), 0);
+      expect(objectRetainCount(obj8raw), 0);
+      expect(objectRetainCount(obj9raw), 0);
       expect(counter.value, 0);
       calloc.free(counter);
     });

pkgs/ffigen/test/native_objc_test/arc_test.m

@@ -24,9 +24,12 @@ + (ArcTestObject*)makeAndAutorelease:(int32_t*) _counter;
 - (void)setCounter:(int32_t*) _counter;
 - (void)dealloc;
 - (ArcTestObject*)copyMe;
-- (ArcTestObject*)makeACopy NS_RETURNS_RETAINED;
+- (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));
 
 @property (assign) ArcTestObject* assignedProperty;
 @property (retain) ArcTestObject* retainedProperty;
@@ -67,7 +70,7 @@ - (ArcTestObject*)copyMe {
   return [[ArcTestObject alloc] initWithCounter: counter];
 }
 
-- (ArcTestObject*)makeACopy NS_RETURNS_RETAINED {
+- (ArcTestObject*)mutableCopyMe {
   return [[ArcTestObject alloc] initWithCounter: counter];
 }
 
@@ -76,7 +79,19 @@ - (id)copyWithZone:(NSZone*) zone {
 }
 
 - (ArcTestObject*)returnsRetained NS_RETURNS_RETAINED {
-  return self;
+  return [self copyMe];
+}
+
+- (ArcTestObject*)copyMeNoRetain __attribute__((ns_returns_not_retained)) {
+  return [self copyMe];
+}
+
+- (ArcTestObject*)copyMeAutorelease __attribute__((ns_returns_autoreleased)) {
+  return [self copyMe];
+}
+
+- (ArcTestObject*)copyMeConsumeSelf __attribute__((ns_consumes_self)) {
+  return [self copyMe];
 }
 
 @end

pkgs/ffigen/test/native_objc_test/ref_count_config.yaml

@@ -5,8 +5,8 @@ output: 'ref_count_bindings.dart'
 exclude-all-by-default: true
 functions:
   include:
-    - createAutoreleasePool
-    - destroyAutoreleasePool
+    - objc_autoreleasePoolPop
+    - objc_autoreleasePoolPush
 objc-interfaces:
   include:
     - RefCountTestObject