Google Git
Sign in
dart/native.git/7aa6d589^!/.
commit
7aa6d5897accf760678c3563d4e88a53f116d3a0
[log]
author
Brian Quinlan <bquinlan@google.com>
Fri Apr 29 14:39:05 2022 -0700
committer
GitHub <noreply@github.com>
Fri Apr 29 14:39:05 2022 -0700
tree
317e5ac4e39fc72d903fef38c127214b1414dc07
parent
3ce29269c4f7e5517170087a774dcb65b2eb2024 [diff]
[ffigen] Make properties that set/get objective-c instances nullable (#348)


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
index 95bce8b..1ab0798 100644
--- a/pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart
+++ b/pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart

@@ -83,11 +83,22 @@
     utilsExist = true;
 
     final objType = PointerType(objCObjectType).getCType(w);
-    s.write('\nclass _ObjCWrapper {\n');
-    s.write('  final $objType _id;\n');
-    s.write('  final ${w.className} _lib;\n');
-    s.write('  _ObjCWrapper._(this._id, this._lib);\n');
-    s.write('}\n');
+    s.write('''
+class _ObjCWrapper {
+  final $objType _id;
+  final ${w.className} _lib;
+
+  _ObjCWrapper._(this._id, this._lib);
+
+  @override
+  bool operator ==(Object other) {
+    return other is _ObjCWrapper && _id == other._id;
+  }
+
+  @override
+  int get hashCode => _id.hashCode;
+}
+''');
   }
 
   void addDependencies(Set<Binding> dependencies) {

diff --git a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart
index 6b9c4c6..ab4da9e 100644
--- a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart
+++ b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart

@@ -121,7 +121,8 @@
       s.write('  ');
       if (isStatic) {
         s.write('static ');
-        s.write(_getConvertedType(returnType, w, name));
+        s.write(
+            _getConvertedReturnType(returnType, w, name, m.isNullableReturn));
 
         switch (m.kind) {
           case ObjCMethodKind.method:
@@ -147,18 +148,20 @@
         switch (m.kind) {
           case ObjCMethodKind.method:
             // returnType methodName(...)
-            s.write(_getConvertedType(returnType, w, name));
+            s.write(_getConvertedReturnType(
+                returnType, w, name, m.isNullableReturn));
             s.write(' $methodName');
             s.write(paramsToString(m.params, isStatic: false));
             break;
           case ObjCMethodKind.propertyGetter:
             // returnType get methodName
-            s.write(_getConvertedType(returnType, w, name));
+            s.write(_getConvertedReturnType(
+                returnType, w, name, m.isNullableReturn));
             s.write(' get $methodName');
             break;
           case ObjCMethodKind.propertySetter:
             // set methodName(...)
-            s.write('set $methodName');
+            s.write(' set $methodName');
             s.write(paramsToString(m.params, isStatic: false));
             break;
         }
@@ -181,7 +184,8 @@
       }
       s.write(');\n');
       if (convertReturn) {
-        final result = _doReturnConversion(returnType, '_ret', name, '_lib');
+        final result = _doReturnConversion(
+            returnType, '_ret', name, '_lib', m.isNullableReturn);
         s.write('    return $result;');
       }
 
@@ -311,6 +315,15 @@
     return type.getDartType(w);
   }
 
+  String _getConvertedReturnType(
+      Type type, Writer w, String enclosingClass, bool isNullableReturn) {
+    final result = _getConvertedType(type, w, enclosingClass);
+    if (isNullableReturn) {
+      return result + "?";
+    }
+    return result;
+  }
+
   String _doArgConversion(ObjCMethodParam arg) {
     if (arg.type is ObjCInterface ||
         _isObject(arg.type) ||
@@ -324,12 +337,22 @@
     return arg.name;
   }
 
-  String _doReturnConversion(
-      Type type, String value, String enclosingClass, String library) {
-    if (type is ObjCInterface) return '${type.name}._($value, $library)';
-    if (_isObject(type)) return 'NSObject._($value, $library)';
-    if (_isInstanceType(type)) return '$enclosingClass._($value, $library)';
-    return value;
+  String _doReturnConversion(Type type, String value, String enclosingClass,
+      String library, bool isNullable) {
+    String prefix = "";
+    if (isNullable) {
+      prefix += "$value.address == 0 ? null : ";
+    }
+    if (type is ObjCInterface) {
+      return prefix + '${type.name}._($value, $library)';
+    }
+    if (_isObject(type)) {
+      return prefix + 'NSObject._($value, $library)';
+    }
+    if (_isInstanceType(type)) {
+      return prefix + '$enclosingClass._($value, $library)';
+    }
+    return prefix + value;
   }
 }
 
@@ -351,6 +374,7 @@
   final String originalName;
   final ObjCProperty? property;
   Type? returnType;
+  final bool isNullableReturn;
   final List<ObjCMethodParam> params;
   final ObjCMethodKind kind;
   final bool isClass;
@@ -364,6 +388,7 @@
     required this.kind,
     required this.isClass,
     this.returnType,
+    this.isNullableReturn = false,
     List<ObjCMethodParam>? params_,
   }) : params = params_ ?? [];
 

diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart
index c9451cf..658a038 100644
--- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart
+++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart

@@ -119,6 +119,9 @@
   final isReadOnly = propertyAttributes &
           clang_types.CXObjCPropertyAttrKind.CXObjCPropertyAttr_readonly >
       0;
+  // TODO(#334): Use the nullable attribute to decide this.
+  final isNullable =
+      cursor.type().kind == clang_types.CXTypeKind.CXType_ObjCObjectPointer;
 
   final property = ObjCProperty(fieldName);
 
@@ -133,8 +136,9 @@
     dartDoc: dartDoc,
     kind: ObjCMethodKind.propertyGetter,
     isClass: isClass,
+    returnType: fieldType,
+    isNullableReturn: isNullable,
   );
-  getter.returnType = fieldType;
   itf.addMethod(getter);
 
   if (!isReadOnly) {
@@ -148,7 +152,8 @@
         kind: ObjCMethodKind.propertySetter,
         isClass: isClass);
     setter.returnType = NativeType(SupportedNativeType.Void);
-    setter.params.add(ObjCMethodParam(fieldType, 'value'));
+    setter.params
+        .add(ObjCMethodParam(fieldType, 'value', isNullable: isNullable));
     itf.addMethod(setter);
   }
 }

diff --git a/pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_objc_config_bindings.dart b/pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_objc_config_bindings.dart
index 3a2029d..a3ab4bf 100644
--- a/pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_objc_config_bindings.dart
+++ b/pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_objc_config_bindings.dart

@@ -423,7 +423,16 @@
 class _ObjCWrapper {
   final ffi.Pointer<ObjCObject> _id;
   final NativeLibrary _lib;
+
   _ObjCWrapper._(this._id, this._lib);
+
+  @override
+  bool operator ==(Object other) {
+    return other is _ObjCWrapper && _id == other._id;
+  }
+
+  @override
+  int get hashCode => _id.hashCode;
 }
 
 class Foo extends NSObject {

diff --git a/pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_objc_interface_bindings.dart b/pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_objc_interface_bindings.dart
index 45201d7..9502b9f 100644
--- a/pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_objc_interface_bindings.dart
+++ b/pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_objc_interface_bindings.dart

@@ -519,7 +519,16 @@
 class _ObjCWrapper {
   final ffi.Pointer<ObjCObject> _id;
   final NativeLibrary _lib;
+
   _ObjCWrapper._(this._id, this._lib);
+
+  @override
+  bool operator ==(Object other) {
+    return other is _ObjCWrapper && _id == other._id;
+  }
+
+  @override
+  int get hashCode => _id.hashCode;
 }
 
 class Foo extends NSObject {

diff --git a/pkgs/ffigen/test/native_objc_test/cast_bindings.dart b/pkgs/ffigen/test/native_objc_test/cast_bindings.dart
index fcac886..d167a6c 100644
--- a/pkgs/ffigen/test/native_objc_test/cast_bindings.dart
+++ b/pkgs/ffigen/test/native_objc_test/cast_bindings.dart

@@ -643,7 +643,16 @@
 class _ObjCWrapper {
   final ffi.Pointer<ObjCObject> _id;
   final CastTestObjCLibrary _lib;
+
   _ObjCWrapper._(this._id, this._lib);
+
+  @override
+  bool operator ==(Object other) {
+    return other is _ObjCWrapper && _id == other._id;
+  }
+
+  @override
+  int get hashCode => _id.hashCode;
 }
 
 class Castaway extends NSObject {

diff --git a/pkgs/ffigen/test/native_objc_test/cast_test.dart b/pkgs/ffigen/test/native_objc_test/cast_test.dart
index 20f2e18..d4a93f3 100644
--- a/pkgs/ffigen/test/native_objc_test/cast_test.dart
+++ b/pkgs/ffigen/test/native_objc_test/cast_test.dart

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 // Objective C support is only available on mac.
+
 @TestOn('mac-os')
 
 import 'dart:ffi';
@@ -52,17 +53,29 @@
     });
 
     test('castFrom', () {
-      final meAsInt = testInstance.meAsInt();
-      expect(Castaway.castFrom(testInstance.meAsNSObject()).meAsInt(), meAsInt);
+      final fromCast = Castaway.castFrom(testInstance.meAsNSObject());
+      expect(fromCast, testInstance);
     });
 
     test('castFromPointer', () {
       final meAsInt = testInstance.meAsInt();
-      expect(
-          Castaway.castFromPointer(
-                  lib, Pointer<ObjCObject>.fromAddress(meAsInt))
-              .meAsInt(),
-          meAsInt);
+      final fromCast = Castaway.castFromPointer(
+          lib, Pointer<ObjCObject>.fromAddress(meAsInt));
+      expect(fromCast, testInstance);
+    });
+
+    test('equality equals', () {
+      final meAsInt = testInstance.meAsInt();
+      final fromCast = Castaway.castFromPointer(
+          lib, Pointer<ObjCObject>.fromAddress(meAsInt));
+      expect(fromCast, testInstance);
+    });
+
+    test('equality not equals', () {
+      final meAsInt = testInstance.meAsInt();
+      final fromCast = Castaway.castFromPointer(
+          lib, Pointer<ObjCObject>.fromAddress(meAsInt));
+      expect(fromCast, isNot(equals(NSObject.new1(lib))));
     });
   });
 }

diff --git a/pkgs/ffigen/test/native_objc_test/method_bindings.dart b/pkgs/ffigen/test/native_objc_test/method_bindings.dart
index 27920b0..c695b60 100644
--- a/pkgs/ffigen/test/native_objc_test/method_bindings.dart
+++ b/pkgs/ffigen/test/native_objc_test/method_bindings.dart

@@ -696,7 +696,16 @@
 class _ObjCWrapper {
   final ffi.Pointer<ObjCObject> _id;
   final MethodTestObjCLibrary _lib;
+
   _ObjCWrapper._(this._id, this._lib);
+
+  @override
+  bool operator ==(Object other) {
+    return other is _ObjCWrapper && _id == other._id;
+  }
+
+  @override
+  int get hashCode => _id.hashCode;
 }
 
 class MethodInterface extends NSObject {

diff --git a/pkgs/ffigen/test/native_objc_test/native_objc_test_bindings.dart b/pkgs/ffigen/test/native_objc_test/native_objc_test_bindings.dart
index cf75b1c..53b8516 100644
--- a/pkgs/ffigen/test/native_objc_test/native_objc_test_bindings.dart
+++ b/pkgs/ffigen/test/native_objc_test/native_objc_test_bindings.dart

@@ -1146,7 +1146,16 @@
 class _ObjCWrapper {
   final ffi.Pointer<ObjCObject> _id;
   final NativeObjCLibrary _lib;
+
   _ObjCWrapper._(this._id, this._lib);
+
+  @override
+  bool operator ==(Object other) {
+    return other is _ObjCWrapper && _id == other._id;
+  }
+
+  @override
+  int get hashCode => _id.hashCode;
 }
 
 class Foo extends NSObject {

diff --git a/pkgs/ffigen/test/native_objc_test/nullable_bindings.dart b/pkgs/ffigen/test/native_objc_test/nullable_bindings.dart
index 6f6b1b8..90533ce 100644
--- a/pkgs/ffigen/test/native_objc_test/nullable_bindings.dart
+++ b/pkgs/ffigen/test/native_objc_test/nullable_bindings.dart

@@ -510,6 +510,32 @@
       _registerName1("isNullWithNullableNSObjectArg:");
   late final ffi.Pointer<ObjCSel> _sel_isNullWithNotNullableNSObjectPtrArg_1 =
       _registerName1("isNullWithNotNullableNSObjectPtrArg:");
+  late final ffi.Pointer<ObjCSel> _sel_returnNil_1 =
+      _registerName1("returnNil:");
+  ffi.Pointer<ObjCObject> _objc_msgSend_15(
+    ffi.Pointer<ObjCObject> obj,
+    ffi.Pointer<ObjCSel> sel,
+    bool r,
+  ) {
+    return __objc_msgSend_15(
+      obj,
+      sel,
+      r ? 1 : 0,
+    );
+  }
+
+  late final __objc_msgSend_15Ptr = _lookup<
+      ffi.NativeFunction<
+          ffi.Pointer<ObjCObject> Function(ffi.Pointer<ObjCObject>,
+              ffi.Pointer<ObjCSel>, ffi.Uint8)>>('objc_msgSend');
+  late final __objc_msgSend_15 = __objc_msgSend_15Ptr.asFunction<
+      ffi.Pointer<ObjCObject> Function(
+          ffi.Pointer<ObjCObject>, ffi.Pointer<ObjCSel>, int)>();
+
+  late final ffi.Pointer<ObjCSel> _sel_nullableObjectProperty1 =
+      _registerName1("nullableObjectProperty");
+  late final ffi.Pointer<ObjCSel> _sel_setNullableObjectProperty_1 =
+      _registerName1("setNullableObjectProperty:");
 }
 
 abstract class NSComparisonResult {
@@ -611,7 +637,16 @@
 class _ObjCWrapper {
   final ffi.Pointer<ObjCObject> _id;
   final NullableTestObjCLibrary _lib;
+
   _ObjCWrapper._(this._id, this._lib);
+
+  @override
+  bool operator ==(Object other) {
+    return other is _ObjCWrapper && _id == other._id;
+  }
+
+  @override
+  int get hashCode => _id.hashCode;
 }
 
 class NullableInterface extends NSObject {
@@ -639,6 +674,22 @@
         _lib._sel_isNullWithNotNullableNSObjectPtrArg_1, x?._id ?? ffi.nullptr);
   }
 
+  static NSObject returnNil(NullableTestObjCLibrary _lib, bool r) {
+    final _ret = _lib._objc_msgSend_15(
+        _lib._class_NullableInterface1, _lib._sel_returnNil_1, r);
+    return NSObject._(_ret, _lib);
+  }
+
+  NSObject? get nullableObjectProperty {
+    final _ret = _lib._objc_msgSend_1(_id, _lib._sel_nullableObjectProperty1);
+    return _ret.address == 0 ? null : NSObject._(_ret, _lib);
+  }
+
+  set nullableObjectProperty(NSObject? value) {
+    _lib._objc_msgSend_8(
+        _id, _lib._sel_setNullableObjectProperty_1, value?._id ?? ffi.nullptr);
+  }
+
   static NullableInterface new1(NullableTestObjCLibrary _lib) {
     final _ret =
         _lib._objc_msgSend_1(_lib._class_NullableInterface1, _lib._sel_new1);

diff --git a/pkgs/ffigen/test/native_objc_test/nullable_test.dart b/pkgs/ffigen/test/native_objc_test/nullable_test.dart
index fe15f8d..c93d56e 100644
--- a/pkgs/ffigen/test/native_objc_test/nullable_test.dart
+++ b/pkgs/ffigen/test/native_objc_test/nullable_test.dart

@@ -17,6 +17,7 @@
 
 void main() {
   late NullableTestObjCLibrary lib;
+  late NullableInterface nullableInterface;
   late NSObject obj;
   group('method calls', () {
     setUpAll(() {
@@ -24,6 +25,7 @@
       final dylib = File('test/native_objc_test/nullable_test.dylib');
       verifySetupFile(dylib);
       lib = NullableTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path));
+      nullableInterface = NullableInterface.new1(lib);
       obj = NSObject.new1(lib);
     });
 
@@ -50,6 +52,26 @@
       }
     });
 
+    group('Nullable property', () {
+      test('Not null', () {
+        nullableInterface.nullableObjectProperty = obj;
+        expect(nullableInterface.nullableObjectProperty, obj);
+      });
+      test('Null', () {
+        nullableInterface.nullableObjectProperty = null;
+        expect(nullableInterface.nullableObjectProperty, null);
+      });
+    });
+
+    group('Nullable return', () {
+      test('Not null', () {
+        expect(NullableInterface.returnNil(lib, false), isA<NSObject>());
+      });
+      test('Null', () {
+        expect(NullableInterface.returnNil(lib, true), null);
+      });
+    }, skip: "TODO(#334): enable this test");
+
     group('Nullable arguments', () {
       test('Not null', () {
         expect(

diff --git a/pkgs/ffigen/test/native_objc_test/nullable_test.m b/pkgs/ffigen/test/native_objc_test/nullable_test.m
index 4733493..abf1b02 100644
--- a/pkgs/ffigen/test/native_objc_test/nullable_test.m
+++ b/pkgs/ffigen/test/native_objc_test/nullable_test.m

@@ -5,6 +5,9 @@
 
 +(BOOL) isNullWithNullableNSObjectArg:(nullable NSObject *)x;
 +(BOOL) isNullWithNotNullableNSObjectPtrArg:(NSObject *)x;
++(nullable NSObject *) returnNil:(BOOL)r;
+
+@property (nullable, retain) NSObject *nullableObjectProperty;
 
 @end
 
@@ -18,4 +21,12 @@
   return x == NULL;
 }
 
++(nullable NSObject *) returnNil:(BOOL)r {
+  if (r) {
+    return nil;
+  } else {
+    return [NSObject new];
+  }
+}
+
 @end

diff --git a/pkgs/ffigen/test/native_objc_test/property_bindings.dart b/pkgs/ffigen/test/native_objc_test/property_bindings.dart
index 0b7ecc3..90ec1be 100644
--- a/pkgs/ffigen/test/native_objc_test/property_bindings.dart
+++ b/pkgs/ffigen/test/native_objc_test/property_bindings.dart

@@ -662,7 +662,16 @@
 class _ObjCWrapper {
   final ffi.Pointer<ObjCObject> _id;
   final PropertyTestObjCLibrary _lib;
+
   _ObjCWrapper._(this._id, this._lib);
+
+  @override
+  bool operator ==(Object other) {
+    return other is _ObjCWrapper && _id == other._id;
+  }
+
+  @override
+  int get hashCode => _id.hashCode;
 }
 
 class PropertyInterface extends NSObject {

diff --git a/pkgs/ffigen/test/native_objc_test/string_bindings.dart b/pkgs/ffigen/test/native_objc_test/string_bindings.dart
index d9ea394..bfadf87 100644
--- a/pkgs/ffigen/test/native_objc_test/string_bindings.dart
+++ b/pkgs/ffigen/test/native_objc_test/string_bindings.dart

@@ -2627,7 +2627,16 @@
 class _ObjCWrapper {
   final ffi.Pointer<ObjCObject> _id;
   final StringTestObjCLibrary _lib;
+
   _ObjCWrapper._(this._id, this._lib);
+
+  @override
+  bool operator ==(Object other) {
+    return other is _ObjCWrapper && _id == other._id;
+  }
+
+  @override
+  int get hashCode => _id.hashCode;
 }
 
 class NSValue extends NSObject {
@@ -3112,9 +3121,9 @@
     return _lib._objc_msgSend_11(_id, _lib._sel_unsignedIntegerValue1);
   }
 
-  NSObject get stringValue {
+  NSObject? get stringValue {
     final _ret = _lib._objc_msgSend_1(_id, _lib._sel_stringValue1);
-    return NSObject._(_ret, _lib);
+    return _ret.address == 0 ? null : NSObject._(_ret, _lib);
   }
 
   int compare(NSObject? otherNumber) {
@@ -3627,14 +3636,14 @@
     return NSOrderedCollectionDifference._(_ret, _lib);
   }
 
-  NSObject get insertions {
+  NSObject? get insertions {
     final _ret = _lib._objc_msgSend_1(_id, _lib._sel_insertions1);
-    return NSObject._(_ret, _lib);
+    return _ret.address == 0 ? null : NSObject._(_ret, _lib);
   }
 
-  NSObject get removals {
+  NSObject? get removals {
     final _ret = _lib._objc_msgSend_1(_id, _lib._sel_removals1);
-    return NSObject._(_ret, _lib);
+    return _ret.address == 0 ? null : NSObject._(_ret, _lib);
   }
 
   bool get hasChanges {
@@ -3839,10 +3848,10 @@
         loadHandler._id);
   }
 
-  NSObject get registeredTypeIdentifiers {
+  NSObject? get registeredTypeIdentifiers {
     final _ret =
         _lib._objc_msgSend_1(_id, _lib._sel_registeredTypeIdentifiers1);
-    return NSObject._(_ret, _lib);
+    return _ret.address == 0 ? null : NSObject._(_ret, _lib);
   }
 
   bool hasItemConformingToTypeIdentifier(NSObject? typeIdentifier) {
@@ -3891,13 +3900,14 @@
     return NSProgress._(_ret, _lib);
   }
 
-  NSObject get suggestedName {
+  NSObject? get suggestedName {
     final _ret = _lib._objc_msgSend_1(_id, _lib._sel_suggestedName1);
-    return NSObject._(_ret, _lib);
+    return _ret.address == 0 ? null : NSObject._(_ret, _lib);
   }
 
-  set suggestedName(NSObject value) {
-    _lib._objc_msgSend_8(_id, _lib._sel_setSuggestedName_1, value._id);
+  set suggestedName(NSObject? value) {
+    _lib._objc_msgSend_8(
+        _id, _lib._sel_setSuggestedName_1, value?._id ?? ffi.nullptr);
   }
 
   NSItemProvider initWithObject(NSObject? object) {