From ba94da9b6e604608ada31ccf47fe15a1b3caa60e Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 26 Oct 2023 17:38:24 -0700 Subject: [PATCH] Use getDartType rather than getFfiDartType in ObjC block codegen (#632) * Blocks returning proper user facing types * More tests and refactors * fmt * Fix test * Fix vararg test * Partial fix for block ref counts * More block ref counting fixes * fmt * Daco's comments --- CHANGELOG.md | 1 + lib/src/code_generator/func_type.dart | 81 ++-- lib/src/code_generator/objc_block.dart | 141 ++++--- .../objc_built_in_functions.dart | 5 + lib/src/code_generator/objc_interface.dart | 82 ++--- lib/src/code_generator/objc_nullable.dart | 33 ++ lib/src/code_generator/pointer.dart | 18 + lib/src/code_generator/type.dart | 53 ++- lib/src/code_generator/typealias.dart | 55 ++- test/native_objc_test/block_config.yaml | 2 +- test/native_objc_test/block_test.dart | 347 ++++++++++++++++-- test/native_objc_test/block_test.m | 63 ++++ test/native_objc_test/nullable_test.dart | 7 + test/native_objc_test/nullable_test.m | 5 + 14 files changed, 701 insertions(+), 192 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9d4b880..2d90855d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Fix ObjC methods returning instancetype having the wrong type in sublasses. - When generating typedefs for `Pointer>`, also generate a typedef for the `Function`. +- Use Dart wrapper types in args and returns of ObjCBlocks. - Bump min SDK version to 3.2.0-114.0.dev. # 9.0.1 diff --git a/lib/src/code_generator/func_type.dart b/lib/src/code_generator/func_type.dart index e1d07936..1e734bc4 100644 --- a/lib/src/code_generator/func_type.dart +++ b/lib/src/code_generator/func_type.dart @@ -23,81 +23,66 @@ class FunctionType extends Type { this.varArgParameters = const [], }); - String _getCacheKeyString( - bool writeArgumentNames, String Function(Type) typeToString) { - final sb = StringBuffer(); + String _getTypeImpl( + bool writeArgumentNames, String Function(Type) typeToString, + {String? varArgWrapper}) { + final params = varArgWrapper != null ? parameters : dartTypeParameters; + String? varArgPack; + if (varArgWrapper != null && varArgParameters.isNotEmpty) { + final varArgPackBuf = StringBuffer(); + varArgPackBuf.write("$varArgWrapper<("); + varArgPackBuf.write((varArgParameters).map((p) { + return '${typeToString(p.type)} ${writeArgumentNames ? p.name : ""}'; + }).join(', ')); + varArgPackBuf.write(",)>"); + varArgPack = varArgPackBuf.toString(); + } // Write return Type. + final sb = StringBuffer(); sb.write(typeToString(returnType)); // Write Function. sb.write(' Function('); - sb.write(parameters.map((p) { - return '${typeToString(p.type)} ${writeArgumentNames ? p.name : ""}'; - }).join(', ')); + sb.write([ + ...params.map((p) { + return '${typeToString(p.type)} ${writeArgumentNames ? p.name : ""}'; + }), + if (varArgPack != null) varArgPack, + ].join(', ')); sb.write(')'); return sb.toString(); } @override - String getCType(Writer w, {bool writeArgumentNames = true}) { - final sb = StringBuffer(); - - // Write return Type. - sb.write(returnType.getCType(w)); - - // Write Function. - sb.write(' Function('); - sb.write((parameters).map((p) { - return '${p.type.getCType(w)} ${writeArgumentNames ? p.name : ""}'; - }).join(', ')); - if (varArgParameters.isNotEmpty) { - sb.write(", ${w.ffiLibraryPrefix}.VarArgs<("); - sb.write((varArgParameters).map((p) { - return '${p.type.getCType(w)} ${writeArgumentNames ? p.name : ""}'; - }).join(', ')); - sb.write(",)>"); - } - sb.write(')'); - - return sb.toString(); - } + String getCType(Writer w, {bool writeArgumentNames = true}) => + _getTypeImpl(writeArgumentNames, (Type t) => t.getCType(w), + varArgWrapper: '${w.ffiLibraryPrefix}.VarArgs'); @override - String getFfiDartType(Writer w, {bool writeArgumentNames = true}) { - final sb = StringBuffer(); + String getFfiDartType(Writer w, {bool writeArgumentNames = true}) => + _getTypeImpl(writeArgumentNames, (Type t) => t.getFfiDartType(w)); - // Write return Type. - sb.write(returnType.getFfiDartType(w)); - - // Write Function. - sb.write(' Function('); - sb.write(dartTypeParameters.map((p) { - return '${p.type.getFfiDartType(w)} ${writeArgumentNames ? p.name : ""}'; - }).join(', ')); - sb.write(')'); - - return sb.toString(); - } + @override + String getDartType(Writer w, {bool writeArgumentNames = true}) => + _getTypeImpl(writeArgumentNames, (Type t) => t.getDartType(w)); @override bool get sameFfiDartAndCType => returnType.sameFfiDartAndCType && - parameters.every((p) => p.type.sameFfiDartAndCType) && - varArgParameters.every((p) => p.type.sameFfiDartAndCType); + dartTypeParameters.every((p) => p.type.sameFfiDartAndCType); @override bool get sameDartAndCType => returnType.sameDartAndCType && - parameters.every((p) => p.type.sameDartAndCType) && - varArgParameters.every((p) => p.type.sameDartAndCType); + dartTypeParameters.every((p) => p.type.sameDartAndCType); @override - String toString() => _getCacheKeyString(false, (Type t) => t.toString()); + String toString() => _getTypeImpl(false, (Type t) => t.toString()); @override - String cacheKey() => _getCacheKeyString(false, (Type t) => t.cacheKey()); + String cacheKey() => _getTypeImpl(false, (Type t) => t.cacheKey()); @override void addDependencies(Set dependencies) { diff --git a/lib/src/code_generator/objc_block.dart b/lib/src/code_generator/objc_block.dart index eb891fcf..b61efc7c 100644 --- a/lib/src/code_generator/objc_block.dart +++ b/lib/src/code_generator/objc_block.dart @@ -73,31 +73,36 @@ class ObjCBlock extends BindingType { final trampFuncType = FunctionType( returnType: returnType, parameters: [Parameter(type: blockPtr, name: 'block'), ...params]); - final natTrampFnType = NativeFunc(trampFuncType); + final trampFuncCType = trampFuncType.getCType(w, writeArgumentNames: false); + final trampFuncFfiDartType = + trampFuncType.getFfiDartType(w, writeArgumentNames: false); + final natTrampFnType = NativeFunc(trampFuncType).getCType(w); final nativeCallableType = - '${w.ffiLibraryPrefix}.NativeCallable<${trampFuncType.getCType(w)}>'; + '${w.ffiLibraryPrefix}.NativeCallable<$trampFuncCType>'; + final funcDartType = funcType.getDartType(w, writeArgumentNames: false); + final funcFfiDartType = + funcType.getFfiDartType(w, writeArgumentNames: false); + final returnFfiDartType = returnType.getFfiDartType(w); + final blockCType = blockPtr.getCType(w); + + final paramsNameOnly = params.map((p) => p.name).join(', '); + final paramsFfiDartType = + params.map((p) => '${p.type.getFfiDartType(w)} ${p.name}').join(', '); + final paramsDartType = + params.map((p) => '${p.type.getDartType(w)} ${p.name}').join(', '); // Write the function pointer based trampoline function. - s.write(returnType.getFfiDartType(w)); - s.write(' $funcPtrTrampoline(${blockPtr.getCType(w)} block'); - for (int i = 0; i < params.length; ++i) { - s.write(', ${params[i].type.getFfiDartType(w)} ${params[i].name}'); - } - s.write(') {\n'); - s.write(' ${isVoid ? '' : 'return '}block.ref.target.cast<' - '${natFnType.getFfiDartType(w)}>().asFunction<' - '${funcType.getFfiDartType(w)}>()('); - for (int i = 0; i < params.length; ++i) { - s.write('${i == 0 ? '' : ', '}${params[i].name}'); - } - s.write(');\n'); - s.write('}\n'); + s.write(''' +$returnFfiDartType $funcPtrTrampoline($blockCType block, $paramsFfiDartType) => + block.ref.target.cast<${natFnType.getFfiDartType(w)}>() + .asFunction<$funcFfiDartType>()($paramsNameOnly); +'''); // Write the closure registry function. s.write(''' -final $closureRegistry = {}; +final $closureRegistry = {}; int $closureRegistryIndex = 0; -$voidPtr $registerClosure(Function fn) { +$voidPtr $registerClosure($funcFfiDartType fn) { final id = ++$closureRegistryIndex; $closureRegistry[id] = fn; return $voidPtr.fromAddress(id); @@ -105,33 +110,30 @@ $voidPtr $registerClosure(Function fn) { '''); // Write the closure based trampoline function. - s.write(returnType.getFfiDartType(w)); - s.write(' $closureTrampoline(${blockPtr.getCType(w)} block'); - for (int i = 0; i < params.length; ++i) { - s.write(', ${params[i].type.getFfiDartType(w)} ${params[i].name}'); - } - s.write(') {\n'); - s.write(' ${isVoid ? '' : 'return '}'); - s.write('($closureRegistry[block.ref.target.address]'); - s.write(' as ${returnType.getFfiDartType(w)} Function('); - for (int i = 0; i < params.length; ++i) { - s.write('${i == 0 ? '' : ', '}${params[i].type.getFfiDartType(w)}'); - } - s.write('))'); - s.write('('); - for (int i = 0; i < params.length; ++i) { - s.write('${i == 0 ? '' : ', '}${params[i].name}'); - } - s.write(');\n'); - s.write('}\n'); + s.write(''' +$returnFfiDartType $closureTrampoline($blockCType block, $paramsFfiDartType) => + $closureRegistry[block.ref.target.address]!($paramsNameOnly); +'''); + + // Snippet that converts a Dart typed closure to FfiDart type. This snippet + // is used below. Note that the closure being converted is called `fn`. + final convertedFnArgs = params + .map((p) => p.type + .convertFfiDartTypeToDartType(w, p.name, 'lib', objCRetain: true)) + .join(', '); + final convFnInvocation = returnType.convertDartTypeToFfiDartType( + w, 'fn($convertedFnArgs)', + objCRetain: true); + final convFn = '($paramsFfiDartType) => $convFnInvocation'; // Write the wrapper class. final defaultValue = returnType.getDefaultValue(w, '_lib'); final exceptionalReturn = defaultValue == null ? '' : ', $defaultValue'; s.write(''' class $name extends _ObjCBlockBase { - $name._(${blockPtr.getCType(w)} id, ${w.className} lib) : - super._(id, lib, retain: false, release: true); + $name._($blockCType id, ${w.className} lib, + {bool retain = false, bool release = true}) : + super._(id, lib, retain: retain, release: release); /// Creates a block from a C function pointer. /// @@ -141,7 +143,7 @@ class $name extends _ObjCBlockBase { $name.fromFunctionPointer(${w.className} lib, $natFnPtr ptr) : this._(lib.${builtInFunctions.newBlock.name}( _cFuncTrampoline ??= ${w.ffiLibraryPrefix}.Pointer.fromFunction< - ${trampFuncType.getCType(w)}>($funcPtrTrampoline + $trampFuncCType>($funcPtrTrampoline $exceptionalReturn).cast(), ptr.cast()), lib); static $voidPtr? _cFuncTrampoline; @@ -150,11 +152,11 @@ class $name extends _ObjCBlockBase { /// This block must be invoked by native code running on the same thread as /// the isolate that registered it. Invoking the block on the wrong thread /// will result in a crash. - $name.fromFunction(${w.className} lib, ${funcType.getFfiDartType(w)} fn) : + $name.fromFunction(${w.className} lib, $funcDartType fn) : this._(lib.${builtInFunctions.newBlock.name}( _dartFuncTrampoline ??= ${w.ffiLibraryPrefix}.Pointer.fromFunction< - ${trampFuncType.getCType(w)}>($closureTrampoline - $exceptionalReturn).cast(), $registerClosure(fn)), lib); + $trampFuncCType>($closureTrampoline + $exceptionalReturn).cast(), $registerClosure($convFn)), lib); static $voidPtr? _dartFuncTrampoline; '''); @@ -171,31 +173,30 @@ class $name extends _ObjCBlockBase { /// /// Note that unlike the default behavior of NativeCallable.listener, listener /// blocks do not keep the isolate alive. - $name.listener(${w.className} lib, ${funcType.getFfiDartType(w)} fn) : + $name.listener(${w.className} lib, $funcDartType fn) : this._(lib.${builtInFunctions.newBlock.name}( - (_dartFuncListenerTrampoline ??= $nativeCallableType.listener($closureTrampoline - $exceptionalReturn)..keepIsolateAlive = false).nativeFunction.cast(), - $registerClosure(fn)), lib); + (_dartFuncListenerTrampoline ??= $nativeCallableType.listener( + $closureTrampoline $exceptionalReturn)..keepIsolateAlive = + false).nativeFunction.cast(), + $registerClosure($convFn)), lib); static $nativeCallableType? _dartFuncListenerTrampoline; '''); } // Call method. - s.write(' ${returnType.getFfiDartType(w)} call('); - for (int i = 0; i < params.length; ++i) { - s.write('${i == 0 ? '' : ', '}${params[i].type.getFfiDartType(w)}'); - s.write(' ${params[i].name}'); - } - s.write(''') { - ${isVoid ? '' : 'return '}_id.ref.invoke.cast< - ${natTrampFnType.getCType(w)}>().asFunction< - ${trampFuncType.getFfiDartType(w)}>()(_id'''); - for (int i = 0; i < params.length; ++i) { - s.write(', ${params[i].name}'); - } - s.write('''); - }'''); + s.write(' ${returnType.getDartType(w)} call($paramsDartType) =>'); + final callMethodArgs = params + .map((p) => + p.type.convertDartTypeToFfiDartType(w, p.name, objCRetain: false)) + .join(', '); + final callMethodInvocation = ''' +_id.ref.invoke.cast<$natTrampFnType>().asFunction<$trampFuncFfiDartType>()( + _id, $callMethodArgs)'''; + s.write(returnType.convertFfiDartTypeToDartType( + w, callMethodInvocation, '_lib', + objCRetain: false)); + s.write(';\n'); s.write('}\n'); return BindingString( @@ -227,6 +228,24 @@ class $name extends _ObjCBlockBase { @override bool get sameDartAndCType => false; + @override + String convertDartTypeToFfiDartType( + Writer w, + String value, { + required bool objCRetain, + }) => + ObjCInterface.generateGetId(value, objCRetain); + + @override + String convertFfiDartTypeToDartType( + Writer w, + String value, + String library, { + required bool objCRetain, + String? objCEnclosingClass, + }) => + ObjCInterface.generateConstructor(name, value, library, objCRetain); + @override String toString() => '($returnType (^)(${argTypes.join(', ')}))'; } diff --git a/lib/src/code_generator/objc_built_in_functions.dart b/lib/src/code_generator/objc_built_in_functions.dart index b772861e..60c998b8 100644 --- a/lib/src/code_generator/objc_built_in_functions.dart +++ b/lib/src/code_generator/objc_built_in_functions.dart @@ -239,6 +239,11 @@ class $name implements ${w.ffiLibraryPrefix}.Finalizable { /// Return a pointer to this object. $idType get pointer => _id; + + $idType _retainAndReturnId() { + _lib.$retain(_id.cast()); + return _id; + } } '''); } diff --git a/lib/src/code_generator/objc_interface.dart b/lib/src/code_generator/objc_interface.dart index e4cf2607..2ed1f10e 100644 --- a/lib/src/code_generator/objc_interface.dart +++ b/lib/src/code_generator/objc_interface.dart @@ -207,12 +207,19 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} { s.write(isStatic ? '_lib.${_classObject.name}' : '_id'); s.write(', _lib.${m.selObject!.name}'); for (final p in m.params) { - s.write(', ${_doArgConversion(p)}'); + final convertedParam = + p.type.convertDartTypeToFfiDartType(w, p.name, objCRetain: false); + s.write(', $convertedParam'); } s.write(');\n'); if (convertReturn) { - final result = _doReturnConversion( - returnType, '_ret', name, '_lib', m.isOwnedReturn); + final result = returnType.convertFfiDartTypeToDartType( + w, + '_ret', + '_lib', + objCRetain: !m.isOwnedReturn, + objCEnclosingClass: name, + ); s.write(' return $result;'); } @@ -394,6 +401,37 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} { @override bool get sameDartAndCType => false; + @override + String convertDartTypeToFfiDartType( + Writer w, + String value, { + required bool objCRetain, + }) => + ObjCInterface.generateGetId(value, objCRetain); + + static String generateGetId(String value, bool objCRetain) => + objCRetain ? '$value._retainAndReturnId()' : '$value._id'; + + @override + String convertFfiDartTypeToDartType( + Writer w, + String value, + String library, { + required bool objCRetain, + String? objCEnclosingClass, + }) => + ObjCInterface.generateConstructor(name, value, library, objCRetain); + + static String generateConstructor( + String className, + String value, + String library, + bool objCRetain, + ) { + final ownershipFlags = 'retain: $objCRetain, release: true'; + return '$className._($value, $library, $ownershipFlags)'; + } + // Utils for converting between the internal types passed to native code, and // the external types visible to the user. For example, ObjCInterfaces are // passed to native as Pointer, but the user sees the Dart wrapper @@ -413,44 +451,6 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} { } return type.getDartType(w); } - - String _doArgConversion(ObjCMethodParam arg) { - final baseType = arg.type.typealiasType; - if (baseType is ObjCNullable) { - return '${arg.name}?._id ?? ffi.nullptr'; - } else if (arg.type is ObjCInstanceType || - baseType is ObjCInterface || - baseType is ObjCObjectPointer || - baseType is ObjCBlock) { - return '${arg.name}._id'; - } - return arg.name; - } - - String _doReturnConversion(Type type, String value, String enclosingClass, - String library, bool isOwnedReturn) { - var prefix = ''; - var baseType = type.typealiasType; - if (baseType is ObjCNullable) { - prefix = '$value.address == 0 ? null : '; - type = baseType.child; - baseType = type.typealiasType; - } - final ownerFlags = 'retain: ${!isOwnedReturn}, release: true'; - if (type is ObjCInstanceType) { - return '$prefix$enclosingClass._($value, $library, $ownerFlags)'; - } - if (baseType is ObjCInterface) { - return '$prefix${baseType.name}._($value, $library, $ownerFlags)'; - } - if (baseType is ObjCBlock) { - return '$prefix${baseType.name}._($value, $library)'; - } - if (baseType is ObjCObjectPointer) { - return '${prefix}NSObject._($value, $library, $ownerFlags)'; - } - return prefix + value; - } } enum ObjCMethodKind { diff --git a/lib/src/code_generator/objc_nullable.dart b/lib/src/code_generator/objc_nullable.dart index db57e5f3..a8d8c522 100644 --- a/lib/src/code_generator/objc_nullable.dart +++ b/lib/src/code_generator/objc_nullable.dart @@ -44,6 +44,39 @@ class ObjCNullable extends Type { @override bool get sameDartAndCType => false; + @override + String convertDartTypeToFfiDartType( + Writer w, + String value, { + required bool objCRetain, + }) { + // This is a bit of a hack, but works for all the types that are allowed to + // be a child type. If we add more allowed child types, we may have to start + // special casing each type. Turns value._id into value?._id ?? nullptr. + final convertedValue = child.convertDartTypeToFfiDartType(w, '$value?', + objCRetain: objCRetain); + return '$convertedValue ?? ${w.ffiLibraryPrefix}.nullptr'; + } + + @override + String convertFfiDartTypeToDartType( + Writer w, + String value, + String library, { + required bool objCRetain, + String? objCEnclosingClass, + }) { + // All currently supported child types have a Pointer as their FfiDartType. + final convertedValue = child.convertFfiDartTypeToDartType( + w, + value, + library, + objCRetain: objCRetain, + objCEnclosingClass: objCEnclosingClass, + ); + return '$value.address == 0 ? null : $convertedValue'; + } + @override String toString() => '$child?'; diff --git a/lib/src/code_generator/pointer.dart b/lib/src/code_generator/pointer.dart index 75363957..dcaf5973 100644 --- a/lib/src/code_generator/pointer.dart +++ b/lib/src/code_generator/pointer.dart @@ -86,4 +86,22 @@ class ObjCObjectPointer extends PointerType { @override bool get sameDartAndCType => false; + + @override + String convertDartTypeToFfiDartType( + Writer w, + String value, { + required bool objCRetain, + }) => + ObjCInterface.generateGetId(value, objCRetain); + + @override + String convertFfiDartTypeToDartType( + Writer w, + String value, + String library, { + required bool objCRetain, + String? objCEnclosingClass, + }) => + ObjCInterface.generateConstructor('NSObject', value, library, objCRetain); } diff --git a/lib/src/code_generator/type.dart b/lib/src/code_generator/type.dart index 454c7834..fff7f0f0 100644 --- a/lib/src/code_generator/type.dart +++ b/lib/src/code_generator/type.dart @@ -54,8 +54,39 @@ abstract class Type { /// Returns whether the dart type and C type string are same. bool get sameDartAndCType => sameFfiDartAndCType; - /// Returns the string representation of the Type, for debugging purposes - /// only. This string should not be printed as generated code. + /// Returns generated Dart code that converts the given value from its + /// DartType to its FfiDartType. + /// + /// [value] is the value to be converted. If [objCRetain] is true, the ObjC + /// object will be reained (ref count incremented) during conversion. + String convertDartTypeToFfiDartType( + Writer w, + String value, { + required bool objCRetain, + }) => + value; + + /// Returns generated Dart code that converts the given value from its + /// FfiDartType to its DartType. + /// + /// [value] is the value to be converted, and [library] is an instance of the + /// native library object. If [objCRetain] is true, the ObjC wrapper object + /// will retain (ref count increment) the wrapped object pointer. If this + /// conversion is occuring in the context of an ObjC class, then + /// [objCEnclosingClass] should be the name of the Dart wrapper class (this is + /// used by instancetype). + String convertFfiDartTypeToDartType( + Writer w, + String value, + String library, { + required bool objCRetain, + String? objCEnclosingClass, + }) => + value; + + /// Returns a human readable string representation of the Type. This is mostly + /// just for debugging, but it may also be used for non-functional code (eg to + /// name a variable or type in generated code). @override String toString(); @@ -107,6 +138,24 @@ abstract class BindingType extends NoLookUpBinding implements Type { @override bool get sameDartAndCType => sameFfiDartAndCType; + @override + String convertDartTypeToFfiDartType( + Writer w, + String value, { + required bool objCRetain, + }) => + value; + + @override + String convertFfiDartTypeToDartType( + Writer w, + String value, + String library, { + required bool objCRetain, + String? objCEnclosingClass, + }) => + value; + @override String toString() => originalName; diff --git a/lib/src/code_generator/typealias.dart b/lib/src/code_generator/typealias.dart index 834987b7..fe971a54 100644 --- a/lib/src/code_generator/typealias.dart +++ b/lib/src/code_generator/typealias.dart @@ -142,7 +142,15 @@ class Typealias extends BindingType { } @override - String getDartType(Writer w) => _dartAliasName ?? type.getDartType(w); + String getDartType(Writer w) { + if (_dartAliasName != null) { + return _dartAliasName!; + } else if (type.sameDartAndCType) { + return getFfiDartType(w); + } else { + return type.getDartType(w); + } + } @override bool get sameFfiDartAndCType => type.sameFfiDartAndCType; @@ -150,6 +158,30 @@ class Typealias extends BindingType { @override bool get sameDartAndCType => type.sameDartAndCType; + @override + String convertDartTypeToFfiDartType( + Writer w, + String value, { + required bool objCRetain, + }) => + type.convertDartTypeToFfiDartType(w, value, objCRetain: objCRetain); + + @override + String convertFfiDartTypeToDartType( + Writer w, + String value, + String library, { + required bool objCRetain, + String? objCEnclosingClass, + }) => + type.convertFfiDartTypeToDartType( + w, + value, + library, + objCRetain: objCRetain, + objCEnclosingClass: objCEnclosingClass, + ); + @override String cacheKey() => type.cacheKey(); @@ -173,4 +205,25 @@ class ObjCInstanceType extends Typealias { super.genFfiDartType, super.isInternal, }) : super._(); + + @override + String convertDartTypeToFfiDartType( + Writer w, + String value, { + required bool objCRetain, + }) => + ObjCInterface.generateGetId(value, objCRetain); + + @override + String convertFfiDartTypeToDartType( + Writer w, + String value, + String library, { + required bool objCRetain, + String? objCEnclosingClass, + }) => + // objCEnclosingClass must be present, because instancetype can only + // occur inside a class. + ObjCInterface.generateConstructor( + objCEnclosingClass!, value, library, objCRetain); } diff --git a/test/native_objc_test/block_config.yaml b/test/native_objc_test/block_config.yaml index e83c6b70..841439e0 100644 --- a/test/native_objc_test/block_config.yaml +++ b/test/native_objc_test/block_config.yaml @@ -1,5 +1,5 @@ name: BlockTestObjCLibrary -description: 'Tests calling Objective-C blocks' +description: 'Tests calling Objective-C blocks.' language: objc output: 'block_bindings.dart' exclude-all-by-default: true diff --git a/test/native_objc_test/block_test.dart b/test/native_objc_test/block_test.dart index 0253eaf1..7ca61d6c 100644 --- a/test/native_objc_test/block_test.dart +++ b/test/native_objc_test/block_test.dart @@ -21,6 +21,9 @@ typedef FloatBlock = ObjCBlock_ffiFloat_ffiFloat; typedef DoubleBlock = ObjCBlock_ffiDouble_ffiDouble; typedef Vec4Block = ObjCBlock_Vec4_Vec4; typedef VoidBlock = ObjCBlock_ffiVoid; +typedef ObjectBlock = ObjCBlock_DummyObject_DummyObject; +typedef NullableObjectBlock = ObjCBlock_DummyObject_DummyObject1; +typedef BlockBlock = ObjCBlock_Int32Int32_Int32Int32; void main() { late BlockTestObjCLibrary lib; @@ -125,41 +128,111 @@ void main() { }); test('Struct block', () { - final inputPtr = calloc(); - final input = inputPtr.ref; - input.x = 1.2; - input.y = 3.4; - input.z = 5.6; - input.w = 7.8; - - final tempPtr = calloc(); - final temp = tempPtr.ref; - final block = Vec4Block.fromFunction(lib, (Vec4 v) { - // Twiddle the Vec4 components. - temp.x = v.y; - temp.y = v.z; - temp.z = v.w; - temp.w = v.x; - return temp; - }); - - final result1 = block(input); - expect(result1.x, 3.4); - expect(result1.y, 5.6); - expect(result1.z, 7.8); - expect(result1.w, 1.2); - - final result2Ptr = calloc(); - final result2 = result2Ptr.ref; - BlockTester.callVec4Block_(lib, result2Ptr, block); - expect(result2.x, 3.4); - expect(result2.y, 5.6); - expect(result2.z, 7.8); - expect(result2.w, 1.2); - - calloc.free(inputPtr); - calloc.free(tempPtr); - calloc.free(result2Ptr); + using((Arena arena) { + final inputPtr = arena(); + final input = inputPtr.ref; + input.x = 1.2; + input.y = 3.4; + input.z = 5.6; + input.w = 7.8; + + final tempPtr = arena(); + final temp = tempPtr.ref; + final block = Vec4Block.fromFunction(lib, (Vec4 v) { + // Twiddle the Vec4 components. + temp.x = v.y; + temp.y = v.z; + temp.z = v.w; + temp.w = v.x; + return temp; + }); + + final result1 = block(input); + expect(result1.x, 3.4); + expect(result1.y, 5.6); + expect(result1.z, 7.8); + expect(result1.w, 1.2); + + final result2Ptr = arena(); + final result2 = result2Ptr.ref; + BlockTester.callVec4Block_(lib, result2Ptr, block); + expect(result2.x, 3.4); + expect(result2.y, 5.6); + expect(result2.z, 7.8); + expect(result2.w, 1.2); + }); + }); + + test('Object block', () { + bool isCalled = false; + final block = ObjectBlock.fromFunction(lib, (DummyObject x) { + isCalled = true; + return x; + }); + + final obj = DummyObject.new1(lib); + final result1 = block(obj); + expect(result1, obj); + expect(isCalled, isTrue); + + isCalled = false; + final result2 = BlockTester.callObjectBlock_(lib, block); + expect(result2, isNot(obj)); + expect(result2.pointer, isNot(nullptr)); + expect(isCalled, isTrue); + }); + + test('Nullable object block', () { + bool isCalled = false; + final block = NullableObjectBlock.fromFunction(lib, (DummyObject? x) { + isCalled = true; + return x; + }); + + final obj = DummyObject.new1(lib); + final result1 = block(obj); + expect(result1, obj); + expect(isCalled, isTrue); + + isCalled = false; + final result2 = block(null); + expect(result2, isNull); + expect(isCalled, isTrue); + + isCalled = false; + final result3 = BlockTester.callNullableObjectBlock_(lib, block); + expect(result3, isNull); + expect(isCalled, isTrue); + }); + + test('Block block', () { + final blockBlock = BlockBlock.fromFunction(lib, (IntBlock intBlock) { + return IntBlock.fromFunction(lib, (int x) { + return 3 * intBlock(x); + }); + }); + + final intBlock = IntBlock.fromFunction(lib, (int x) { + return 5 * x; + }); + final result1 = blockBlock(intBlock); + expect(result1(1), 15); + + final result2 = BlockTester.newBlock_withMult_(lib, blockBlock, 2); + expect(result2(1), 6); + }); + + test('Native block block', () { + final blockBlock = BlockTester.newBlockBlock_(lib, 7); + + final intBlock = IntBlock.fromFunction(lib, (int x) { + return 5 * x; + }); + final result1 = blockBlock(intBlock); + expect(result1(1), 35); + + final result2 = BlockTester.newBlock_withMult_(lib, blockBlock, 2); + expect(result2(1), 14); }); Pointer funcPointerBlockRefCountTest() { @@ -172,7 +245,7 @@ void main() { test('Function pointer block ref counting', () { final rawBlock = funcPointerBlockRefCountTest(); doGC(); - expect(BlockTester.getBlockRetainCount_(lib, rawBlock.cast()), 0); + expect(BlockTester.getBlockRetainCount_(lib, rawBlock), 0); }); Pointer funcBlockRefCountTest() { @@ -181,10 +254,208 @@ void main() { return block.pointer.cast(); } - test('Function pointer block ref counting', () { + test('Function block ref counting', () { final rawBlock = funcBlockRefCountTest(); doGC(); - expect(BlockTester.getBlockRetainCount_(lib, rawBlock.cast()), 0); + expect(BlockTester.getBlockRetainCount_(lib, rawBlock), 0); + }); + + (Pointer, Pointer, Pointer) + blockBlockDartCallRefCountTest() { + final inputBlock = IntBlock.fromFunction(lib, (int x) { + return 5 * x; + }); + final blockBlock = BlockBlock.fromFunction(lib, (IntBlock intBlock) { + return IntBlock.fromFunction(lib, (int x) { + return 3 * intBlock(x); + }); + }); + final outputBlock = blockBlock(inputBlock); + expect(outputBlock(1), 15); + doGC(); + + // One reference held by inputBlock object, another bound to the + // outputBlock lambda. + expect( + BlockTester.getBlockRetainCount_(lib, inputBlock.pointer.cast()), 2); + + expect( + BlockTester.getBlockRetainCount_(lib, blockBlock.pointer.cast()), 1); + expect( + BlockTester.getBlockRetainCount_(lib, outputBlock.pointer.cast()), 1); + return ( + inputBlock.pointer.cast(), + blockBlock.pointer.cast(), + outputBlock.pointer.cast() + ); + } + + test('Calling a block block from Dart has correct ref counting', () { + final (inputBlock, blockBlock, outputBlock) = + blockBlockDartCallRefCountTest(); + doGC(); + + // This leaks because block functions aren't cleaned up at the moment. + // TODO(https://github.com/dart-lang/ffigen/issues/428): Fix this leak. + expect(BlockTester.getBlockRetainCount_(lib, inputBlock), 1); + + expect(BlockTester.getBlockRetainCount_(lib, blockBlock), 0); + expect(BlockTester.getBlockRetainCount_(lib, outputBlock), 0); + }); + + (Pointer, Pointer, Pointer) + blockBlockObjCCallRefCountTest() { + late Pointer inputBlock; + final blockBlock = BlockBlock.fromFunction(lib, (IntBlock intBlock) { + inputBlock = intBlock.pointer.cast(); + return IntBlock.fromFunction(lib, (int x) { + return 3 * intBlock(x); + }); + }); + final outputBlock = BlockTester.newBlock_withMult_(lib, blockBlock, 2); + expect(outputBlock(1), 6); + doGC(); + + expect(BlockTester.getBlockRetainCount_(lib, inputBlock), 2); + expect( + BlockTester.getBlockRetainCount_(lib, blockBlock.pointer.cast()), 1); + expect( + BlockTester.getBlockRetainCount_(lib, outputBlock.pointer.cast()), 1); + return ( + inputBlock, + blockBlock.pointer.cast(), + outputBlock.pointer.cast() + ); + } + + test('Calling a block block from ObjC has correct ref counting', () { + final (inputBlock, blockBlock, outputBlock) = + blockBlockObjCCallRefCountTest(); + doGC(); + + // This leaks because block functions aren't cleaned up at the moment. + // TODO(https://github.com/dart-lang/ffigen/issues/428): Fix this leak. + expect(BlockTester.getBlockRetainCount_(lib, inputBlock), 2); + + expect(BlockTester.getBlockRetainCount_(lib, blockBlock), 0); + expect(BlockTester.getBlockRetainCount_(lib, outputBlock), 0); + }); + + (Pointer, Pointer, Pointer) + nativeBlockBlockDartCallRefCountTest() { + final inputBlock = IntBlock.fromFunction(lib, (int x) { + return 5 * x; + }); + final blockBlock = BlockTester.newBlockBlock_(lib, 7); + final outputBlock = blockBlock(inputBlock); + expect(outputBlock(1), 35); + doGC(); + + // One reference held by inputBlock object, another held internally by the + // ObjC implementation of the blockBlock. + expect( + BlockTester.getBlockRetainCount_(lib, inputBlock.pointer.cast()), 2); + + expect( + BlockTester.getBlockRetainCount_(lib, blockBlock.pointer.cast()), 1); + expect( + BlockTester.getBlockRetainCount_(lib, outputBlock.pointer.cast()), 1); + return ( + inputBlock.pointer.cast(), + blockBlock.pointer.cast(), + outputBlock.pointer.cast() + ); + } + + test('Calling a native block block from Dart has correct ref counting', () { + final (inputBlock, blockBlock, outputBlock) = + nativeBlockBlockDartCallRefCountTest(); + doGC(); + expect(BlockTester.getBlockRetainCount_(lib, inputBlock), 0); + expect(BlockTester.getBlockRetainCount_(lib, blockBlock), 0); + expect(BlockTester.getBlockRetainCount_(lib, outputBlock), 0); + }); + + (Pointer, Pointer) nativeBlockBlockObjCCallRefCountTest() { + final blockBlock = BlockTester.newBlockBlock_(lib, 7); + final outputBlock = BlockTester.newBlock_withMult_(lib, blockBlock, 2); + expect(outputBlock(1), 14); + doGC(); + + expect( + BlockTester.getBlockRetainCount_(lib, blockBlock.pointer.cast()), 1); + expect( + BlockTester.getBlockRetainCount_(lib, outputBlock.pointer.cast()), 1); + return (blockBlock.pointer.cast(), outputBlock.pointer.cast()); + } + + test('Calling a native block block from ObjC has correct ref counting', () { + final (blockBlock, outputBlock) = nativeBlockBlockObjCCallRefCountTest(); + doGC(); + expect(BlockTester.getBlockRetainCount_(lib, blockBlock), 0); + expect(BlockTester.getBlockRetainCount_(lib, outputBlock), 0); + }); + + (Pointer, Pointer) objectBlockRefCountTest(Allocator alloc) { + final inputCounter = alloc(); + final outputCounter = alloc(); + inputCounter.value = 0; + outputCounter.value = 0; + + final block = ObjectBlock.fromFunction(lib, (DummyObject x) { + return DummyObject.newWithCounter_(lib, outputCounter); + }); + + final inputObj = DummyObject.newWithCounter_(lib, inputCounter); + final outputObj = block(inputObj); + expect(inputCounter.value, 1); + expect(outputCounter.value, 1); + + return (inputCounter, outputCounter); + } + + test('Objects received and returned by blocks have correct ref counts', () { + using((Arena arena) { + final (inputCounter, outputCounter) = objectBlockRefCountTest(arena); + doGC(); + expect(inputCounter.value, 0); + expect(outputCounter.value, 0); + }); + }); + + (Pointer, Pointer) objectNativeBlockRefCountTest( + Allocator alloc) { + final inputCounter = alloc(); + final outputCounter = alloc(); + inputCounter.value = 0; + outputCounter.value = 0; + + final block = ObjectBlock.fromFunction(lib, (DummyObject x) { + x.setCounter_(inputCounter); + return DummyObject.newWithCounter_(lib, outputCounter); + }); + + final outputObj = BlockTester.callObjectBlock_(lib, block); + expect(inputCounter.value, 1); + expect(outputCounter.value, 1); + + return (inputCounter, outputCounter); + } + + test( + 'Objects received and returned by native blocks have correct ref counts', + () { + using((Arena arena) { + final (inputCounter, outputCounter) = + objectNativeBlockRefCountTest(arena); + doGC(); + + // This leaks because block functions aren't cleaned up at the moment. + // TODO(https://github.com/dart-lang/ffigen/issues/428): Fix this leak. + expect(inputCounter.value, 1); + + expect(outputCounter.value, 0); + }); }); test('Block fields have sensible values', () { diff --git a/test/native_objc_test/block_test.m b/test/native_objc_test/block_test.m index a864f2ef..3dd4289d 100644 --- a/test/native_objc_test/block_test.m +++ b/test/native_objc_test/block_test.m @@ -12,11 +12,47 @@ double w; } Vec4; +@interface DummyObject : NSObject { + int32_t* counter; +} ++ (instancetype)newWithCounter:(int32_t*) _counter; +- (instancetype)initWithCounter:(int32_t*) _counter; +- (void)setCounter:(int32_t*) _counter; +- (void)dealloc; +@end +@implementation DummyObject + ++ (instancetype)newWithCounter:(int32_t*) _counter { + return [[DummyObject alloc] initWithCounter: _counter]; +} + +- (instancetype)initWithCounter:(int32_t*) _counter { + counter = _counter; + ++*counter; + return [super init]; +} + +- (void)setCounter:(int32_t*) _counter { + counter = _counter; + ++*counter; +} + +- (void)dealloc { + if (counter != nil) --*counter; + [super dealloc]; +} + +@end + + typedef int32_t (^IntBlock)(int32_t); typedef float (^FloatBlock)(float); typedef double (^DoubleBlock)(double); typedef Vec4 (^Vec4Block)(Vec4); typedef void (^VoidBlock)(); +typedef DummyObject* (^ObjectBlock)(DummyObject*); +typedef DummyObject* _Nullable (^NullableObjectBlock)(DummyObject* _Nullable); +typedef IntBlock (^BlockBlock)(IntBlock); // Wrapper around a block, so that our Dart code can test creating and invoking // blocks in Objective C code. @@ -34,6 +70,10 @@ + (NSThread*)callOnNewThread:(VoidBlock)block; + (float)callFloatBlock:(FloatBlock)block; + (double)callDoubleBlock:(DoubleBlock)block; + (Vec4)callVec4Block:(Vec4Block)block; ++ (DummyObject*)callObjectBlock:(ObjectBlock)block NS_RETURNS_RETAINED; ++ (nullable DummyObject*)callNullableObjectBlock:(NullableObjectBlock)block; ++ (IntBlock)newBlock:(BlockBlock)block withMult:(int)mult; ++ (BlockBlock)newBlockBlock:(int)mult; @end @implementation BlockTester @@ -116,4 +156,27 @@ + (Vec4)callVec4Block:(Vec4Block)block { return block(vec4); } ++ (DummyObject*)callObjectBlock:(ObjectBlock)block NS_RETURNS_RETAINED { + return block([DummyObject new]); +} + ++ (nullable DummyObject*)callNullableObjectBlock:(NullableObjectBlock)block { + return block(nil); +} + ++ (IntBlock)newBlock:(BlockBlock)block withMult:(int)mult { + return block([^int(int x) { + return mult * x; + } copy]); + // ^ copy this stack allocated block to the heap. +} + ++ (BlockBlock)newBlockBlock:(int)mult { + return [^IntBlock(IntBlock block) { + return [^int(int x) { + return mult * block(x); + } copy]; + } copy]; +} + @end diff --git a/test/native_objc_test/nullable_test.dart b/test/native_objc_test/nullable_test.dart index 9aed94e1..b264531d 100644 --- a/test/native_objc_test/nullable_test.dart +++ b/test/native_objc_test/nullable_test.dart @@ -64,6 +64,13 @@ void main() { expect(NullableInterface.isNullWithNotNullableNSObjectPtrArg_(lib, obj), false); }); + + test('Explicit non null', () { + expect( + NullableInterface.isNullWithExplicitNonNullableNSObjectPtrArg_( + lib, obj), + false); + }); }); }); } diff --git a/test/native_objc_test/nullable_test.m b/test/native_objc_test/nullable_test.m index abf1b02b..06d257d5 100644 --- a/test/native_objc_test/nullable_test.m +++ b/test/native_objc_test/nullable_test.m @@ -5,6 +5,7 @@ @interface NullableInterface : NSObject { +(BOOL) isNullWithNullableNSObjectArg:(nullable NSObject *)x; +(BOOL) isNullWithNotNullableNSObjectPtrArg:(NSObject *)x; ++(BOOL) isNullWithExplicitNonNullableNSObjectPtrArg:(nonnull NSObject *)x; +(nullable NSObject *) returnNil:(BOOL)r; @property (nullable, retain) NSObject *nullableObjectProperty; @@ -21,6 +22,10 @@ +(BOOL) isNullWithNotNullableNSObjectPtrArg:(NSObject *)x { return x == NULL; } ++(BOOL) isNullWithExplicitNonNullableNSObjectPtrArg:(nonnull NSObject *)x { + return x == NULL; +} + +(nullable NSObject *) returnNil:(BOOL)r { if (r) { return nil;