From c1b6fff3d5e0828c80cc56be6821fca987d717c0 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 10 Sep 2024 09:45:08 +1000 Subject: [PATCH 1/7] Make listener trampoline args generic --- .../ffigen/lib/src/code_generator/objc_block.dart | 15 +++++++-------- .../lib/src/code_generator/objc_interface.dart | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/pkgs/ffigen/lib/src/code_generator/objc_block.dart b/pkgs/ffigen/lib/src/code_generator/objc_block.dart index 5b03f26ae..eb4ad5168 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_block.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_block.dart @@ -288,15 +288,17 @@ ref.pointer.ref.invoke.cast<$natTrampFnType>().asFunction<$trampFuncFfiDartType> argsReceived.add(param.getNativeType(varName: argName)); retains.add(param.type.generateRetain(argName) ?? argName); } + final argStr = argsReceived.join(', '); final fnName = _wrapListenerBlock!.name; - final blockTypedef = w.objCLevelUniqueNamer.makeUnique('ListenerBlock'); + final blockName = w.objCLevelUniqueNamer.makeUnique('_ListenerTrampoline'); + final blockTypedef = '${returnType.getNativeType()} (^$blockName)($argStr)'; final s = StringBuffer(); s.write(''' -typedef ${getNativeType(varName: blockTypedef)}; -$blockTypedef $fnName($blockTypedef block) NS_RETURNS_RETAINED { - return ^void(${argsReceived.join(', ')}) { +typedef $blockTypedef; +$blockName $fnName($blockName block) NS_RETURNS_RETAINED { + return ^void($argStr) { block(${retains.join(', ')}); }; } @@ -342,10 +344,7 @@ $blockTypedef $fnName($blockTypedef block) NS_RETURNS_RETAINED { String getObjCBlockSignatureType(Writer w) => getDartType(w); @override - String getNativeType({String varName = ''}) { - final paramStrs = params.map((p) => p.getNativeType()); - return '${returnType.getNativeType()} (^$varName)(${paramStrs.join(', ')})'; - } + String getNativeType({String varName = ''}) => 'id $varName'; @override bool get sameFfiDartAndCType => true; diff --git a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart index bd6cec663..b4e4262e7 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart @@ -318,7 +318,7 @@ class ObjCInterface extends BindingType with ObjCMethods { _isBuiltIn ? '${w.objcPkgPrefix}.$name' : name; @override - String getNativeType({String varName = ''}) => '$originalName* $varName'; + String getNativeType({String varName = ''}) => 'id $varName'; @override String getObjCBlockSignatureType(Writer w) => getDartType(w); From 14aa23c9740f5dbb4ecda689166142e2c996b8ee Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 10 Sep 2024 13:30:32 +1000 Subject: [PATCH 2/7] Dedupe ObjC listener block trampolines --- .../lib/src/code_generator/library.dart | 4 ++ .../lib/src/code_generator/objc_block.dart | 27 +++++----- .../objc_built_in_functions.dart | 49 +++++++++++++++++++ .../src/code_generator/objc_interface.dart | 2 - .../lib/src/code_generator/objc_methods.dart | 1 + pkgs/ffigen/lib/src/header_parser/parser.dart | 1 + .../sub_parsers/objc_block_parser.dart | 1 + 7 files changed, 67 insertions(+), 18 deletions(-) diff --git a/pkgs/ffigen/lib/src/code_generator/library.dart b/pkgs/ffigen/lib/src/code_generator/library.dart index beb590cab..a06b3a5d1 100644 --- a/pkgs/ffigen/lib/src/code_generator/library.dart +++ b/pkgs/ffigen/lib/src/code_generator/library.dart @@ -20,6 +20,8 @@ class Library { /// List of bindings in this library. late List bindings; + final ObjCBuiltInFunctions? objCBuiltInFunctions; + late Writer _writer; Writer get writer => _writer; @@ -34,6 +36,7 @@ class Library { List? libraryImports, bool silenceEnumWarning = false, List nativeEntryPoints = const [], + this.objCBuiltInFunctions, }) { _findBindings(bindings, sort); @@ -103,6 +106,7 @@ class Library { for (final b in original) { b.addDependencies(dependencies); } + objCBuiltInFunctions?.addDependencies(dependencies); /// Save bindings. bindings = dependencies.toList(); diff --git a/pkgs/ffigen/lib/src/code_generator/objc_block.dart b/pkgs/ffigen/lib/src/code_generator/objc_block.dart index eb4ad5168..aa9c0ae53 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_block.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_block.dart @@ -3,22 +3,23 @@ // BSD-style license that can be found in the LICENSE file. import '../code_generator.dart'; -import '../config_provider/config_types.dart'; import '../header_parser/data.dart' show bindingsIndex; import 'binding_string.dart'; import 'writer.dart'; class ObjCBlock extends BindingType { + final ObjCBuiltInFunctions builtInFunctions; final Type returnType; final List params; final bool returnsRetained; - Func? _wrapListenerBlock; + ObjCListenerBlockTrampoline? _wrapListenerBlock; factory ObjCBlock({ required Type returnType, required List params, required bool returnsRetained, + required ObjCBuiltInFunctions builtInFunctions, }) { final usr = _getBlockUsr(returnType, params, returnsRetained); @@ -33,6 +34,7 @@ class ObjCBlock extends BindingType { returnType: returnType, params: params, returnsRetained: returnsRetained, + builtInFunctions: builtInFunctions, ); bindingsIndex.addObjCBlockToSeen(usr, block); @@ -45,6 +47,7 @@ class ObjCBlock extends BindingType { required this.returnType, required this.params, required this.returnsRetained, + required this.builtInFunctions, }) : super(originalName: name); // Generates a human readable name for the block based on the args and return @@ -213,7 +216,7 @@ abstract final class $name { ); final listenerConvFn = '($paramsFfiDartType) => $listenerConvFnInvocation'; - final wrapFn = _wrapListenerBlock?.name; + final wrapFn = _wrapListenerBlock?.func.name; final releaseFn = ObjCBuiltInFunctions.objectRelease.gen(w); s.write(''' @@ -278,7 +281,8 @@ ref.pointer.ref.invoke.cast<$natTrampFnType>().asFunction<$trampFuncFfiDartType> @override BindingString? toObjCBindingString(Writer w) { - if (_wrapListenerBlock == null) return null; + if (_wrapListenerBlock?.objCBindingsGenerated ?? true) return null; + _wrapListenerBlock!.objCBindingsGenerated = true; final argsReceived = []; final retains = []; @@ -289,7 +293,7 @@ ref.pointer.ref.invoke.cast<$natTrampFnType>().asFunction<$trampFuncFfiDartType> retains.add(param.type.generateRetain(argName) ?? argName); } final argStr = argsReceived.join(', '); - final fnName = _wrapListenerBlock!.name; + final fnName = _wrapListenerBlock!.func.name; final blockName = w.objCLevelUniqueNamer.makeUnique('_ListenerTrampoline'); final blockTypedef = '${returnType.getNativeType()} (^$blockName)($argStr)'; @@ -317,17 +321,8 @@ $blockName $fnName($blockName block) NS_RETURNS_RETAINED { p.type.addDependencies(dependencies); } - if (hasListener && params.any((p) => p.type.generateRetain('') != null)) { - _wrapListenerBlock = Func( - name: 'wrapListenerBlock_$name', - returnType: this, - parameters: [Parameter(name: 'block', type: this, objCConsumed: false)], - objCReturnsRetained: true, - isLeaf: true, - isInternal: true, - useNameForLookup: true, - ffiNativeConfig: const FfiNativeConfig(enabled: true), - )..addDependencies(dependencies); + if (hasListener) { + _wrapListenerBlock = builtInFunctions.getListenerBlockTrampoline(this); } } 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 9841e5631..6ff57b5dd 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 @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import '../code_generator.dart'; +import '../config_provider/config_types.dart'; import 'binding_string.dart'; import 'writer.dart'; @@ -12,6 +13,7 @@ class ObjCBuiltInFunctions { ObjCBuiltInFunctions(this.generateForPackageObjectiveC); final bool generateForPackageObjectiveC; + var _depsAdded = false; static const registerName = ObjCImport('registerName'); static const getClass = ObjCImport('getClass'); @@ -116,6 +118,7 @@ class ObjCBuiltInFunctions { // for float return types we need objc_msgSend_fpret. final _msgSendFuncs = {}; ObjCMsgSendFunc getMsgSendFunc(Type returnType, List params) { + assert(!_depsAdded); var key = returnType.cacheKey(); for (final p in params) { key += ' ${p.type.cacheKey()}'; @@ -129,19 +132,58 @@ class ObjCBuiltInFunctions { final _selObjects = {}; ObjCInternalGlobal getSelObject(String methodName) { + assert(!_depsAdded); return _selObjects[methodName] ??= ObjCInternalGlobal( '_sel_${methodName.replaceAll(":", "_")}', (Writer w) => '${registerName.gen(w)}("$methodName")', ); } + final _blockTrampolines = {}; + ObjCListenerBlockTrampoline? getListenerBlockTrampoline(ObjCBlock block) { + assert(!_depsAdded); + + var needsTrampoline = false; + final paramIds = []; + for (final param in block.params) { + final retainFunc = param.type.generateRetain(''); + if (retainFunc != null) { + needsTrampoline = true; + } + + // The trampoline ID is based on the getNativeType of the param. Objects + // and blocks both have id as their native type, but need separate + // trampolines since they have different retain functions. So add the + // retainFunc to their id. + paramIds.add('${param.getNativeType()}-${retainFunc ?? ''}'); + } + if (!needsTrampoline) return null; + final id = paramIds.join(','); + + return _blockTrampolines[id] ??= ObjCListenerBlockTrampoline(Func( + name: 'wrapListenerBlock', + returnType: PointerType(objCBlockType), + parameters: [Parameter(name: 'block', type: PointerType(objCBlockType), objCConsumed: false)], + objCReturnsRetained: true, + isLeaf: true, + isInternal: true, + useNameForLookup: true, + ffiNativeConfig: const FfiNativeConfig(enabled: true), + )); + } + void addDependencies(Set dependencies) { + if (_depsAdded) return; + _depsAdded = true; for (final msgSendFunc in _msgSendFuncs.values) { msgSendFunc.addDependencies(dependencies); } for (final sel in _selObjects.values) { sel.addDependencies(dependencies); } + for (final tramp in _blockTrampolines.values) { + tramp.func.addDependencies(dependencies); + } } static bool isInstanceType(Type type) { @@ -151,6 +193,13 @@ class ObjCBuiltInFunctions { } } +/// A native trampoline function for a listener block. +class ObjCListenerBlockTrampoline { + final Func func; + var objCBindingsGenerated = false; + ObjCListenerBlockTrampoline(this.func); +} + /// A function, global variable, or helper type defined in package:objective_c. class ObjCImport { final String name; diff --git a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart index b4e4262e7..0f7513f4c 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart @@ -256,8 +256,6 @@ class ObjCInterface extends BindingType with ObjCMethods { // Add dependencies for any methods that were added. addMethodDependencies(dependencies, needMsgSend: true); } - - builtInFunctions.addDependencies(dependencies); } void _copyMethodsFromSuperType() { diff --git a/pkgs/ffigen/lib/src/code_generator/objc_methods.dart b/pkgs/ffigen/lib/src/code_generator/objc_methods.dart index 11eaee601..8724c1652 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_methods.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_methods.dart @@ -225,6 +225,7 @@ class ObjCMethod { ...params, ], returnsRetained: returnsRetained, + builtInFunctions: builtInFunctions, )..addDependencies(dependencies); } } diff --git a/pkgs/ffigen/lib/src/header_parser/parser.dart b/pkgs/ffigen/lib/src/header_parser/parser.dart index da44bf982..ecf4fd0e7 100644 --- a/pkgs/ffigen/lib/src/header_parser/parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/parser.dart @@ -35,6 +35,7 @@ Library parse(Config c) { libraryImports: c.libraryImports.values.toList(), silenceEnumWarning: c.silenceEnumWarning, nativeEntryPoints: c.entryPoints.map((uri) => uri.toFilePath()).toList(), + objCBuiltInFunctions: objCBuiltInFunctions, ); return library; diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objc_block_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objc_block_parser.dart index 587649821..d53b46c37 100644 --- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objc_block_parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objc_block_parser.dart @@ -27,5 +27,6 @@ ObjCBlock parseObjCBlock(clang_types.CXType cxtype) { returnType: returnType, params: params, returnsRetained: false, + builtInFunctions: objCBuiltInFunctions, ); } From 55f834537afd483b0377251c2e2893b270090d61 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 10 Sep 2024 13:50:48 +1000 Subject: [PATCH 3/7] Rename the trampoline --- pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6ff57b5dd..83829abf6 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 @@ -161,7 +161,7 @@ class ObjCBuiltInFunctions { final id = paramIds.join(','); return _blockTrampolines[id] ??= ObjCListenerBlockTrampoline(Func( - name: 'wrapListenerBlock', + name: '_wrapListenerBlock', returnType: PointerType(objCBlockType), parameters: [Parameter(name: 'block', type: PointerType(objCBlockType), objCConsumed: false)], objCReturnsRetained: true, From d298d5861bbd24d4c2ff1b41e6b5e0949325f2b7 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 10 Sep 2024 13:58:53 +1000 Subject: [PATCH 4/7] changelog --- pkgs/ffigen/CHANGELOG.md | 1 + .../lib/src/code_generator/objc_built_in_functions.dart | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkgs/ffigen/CHANGELOG.md b/pkgs/ffigen/CHANGELOG.md index 31b51b4cb..a89e6cbbc 100644 --- a/pkgs/ffigen/CHANGELOG.md +++ b/pkgs/ffigen/CHANGELOG.md @@ -1,6 +1,7 @@ ## 14.1.0-wip - Fix bug with nullable types in `ObjCBlock`'s type arguments. +- Dedupe `ObjCBlock` trampolines to reduce generated ObjC code. ## 14.0.0 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 83829abf6..8f30f8959 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 @@ -152,9 +152,9 @@ class ObjCBuiltInFunctions { } // The trampoline ID is based on the getNativeType of the param. Objects - // and blocks both have id as their native type, but need separate + // and blocks both have `id` as their native type, but need separate // trampolines since they have different retain functions. So add the - // retainFunc to their id. + // retainFunc (if any) to all the param IDs. paramIds.add('${param.getNativeType()}-${retainFunc ?? ''}'); } if (!needsTrampoline) return null; From ee718cf86c11ccd8a440d6874b2fee9fa0b05e88 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 10 Sep 2024 14:23:49 +1000 Subject: [PATCH 5/7] Fix analysis --- .../objc_built_in_functions.dart | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) 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 8f30f8959..f1e59d98f 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 @@ -161,15 +161,20 @@ class ObjCBuiltInFunctions { final id = paramIds.join(','); return _blockTrampolines[id] ??= ObjCListenerBlockTrampoline(Func( - name: '_wrapListenerBlock', - returnType: PointerType(objCBlockType), - parameters: [Parameter(name: 'block', type: PointerType(objCBlockType), objCConsumed: false)], - objCReturnsRetained: true, - isLeaf: true, - isInternal: true, - useNameForLookup: true, - ffiNativeConfig: const FfiNativeConfig(enabled: true), - )); + name: '_wrapListenerBlock', + returnType: PointerType(objCBlockType), + parameters: [ + Parameter( + name: 'block', + type: PointerType(objCBlockType), + objCConsumed: false) + ], + objCReturnsRetained: true, + isLeaf: true, + isInternal: true, + useNameForLookup: true, + ffiNativeConfig: const FfiNativeConfig(enabled: true), + )); } void addDependencies(Set dependencies) { @@ -196,7 +201,7 @@ class ObjCBuiltInFunctions { /// A native trampoline function for a listener block. class ObjCListenerBlockTrampoline { final Func func; - var objCBindingsGenerated = false; + bool objCBindingsGenerated = false; ObjCListenerBlockTrampoline(this.func); } From 705d54329e55a15e1a31a2c14fe14afc200e8cc0 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 11 Sep 2024 17:13:09 +1000 Subject: [PATCH 6/7] Fix CI failure --- pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f1e59d98f..307e11212 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 @@ -161,7 +161,7 @@ class ObjCBuiltInFunctions { final id = paramIds.join(','); return _blockTrampolines[id] ??= ObjCListenerBlockTrampoline(Func( - name: '_wrapListenerBlock', + name: '_wrapListenerBlock_${id.hashCode.toRadixString(16)}', returnType: PointerType(objCBlockType), parameters: [ Parameter( From a80233fb0a4d5726c0c914c6867a8295a597e4d2 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 11 Sep 2024 17:29:56 +1000 Subject: [PATCH 7/7] Add a test that checks the bindings --- .../test/native_objc_test/block_test.dart | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pkgs/ffigen/test/native_objc_test/block_test.dart b/pkgs/ffigen/test/native_objc_test/block_test.dart index ed7c4a585..eacc8b02d 100644 --- a/pkgs/ffigen/test/native_objc_test/block_test.dart +++ b/pkgs/ffigen/test/native_objc_test/block_test.dart @@ -667,6 +667,23 @@ void main() { expect(descPtr.ref.dispose_helper, isNot(nullptr)); expect(descPtr.ref.signature, nullptr); }); + + test('Block trampoline args converted to id', () { + final objCBindings = + File('test/native_objc_test/block_bindings.m').readAsStringSync(); + + // Objects are converted to id. + expect(objCBindings, isNot(contains('NSObject'))); + expect(objCBindings, isNot(contains('NSString'))); + expect(objCBindings, contains('id')); + + // Blocks are also converted to id. Note: (^) is part of a block type. + expect(objCBindings, isNot(contains('(^)'))); + + // Other types, like structs, are still there. + expect(objCBindings, contains('Vec2')); + expect(objCBindings, contains('Vec4')); + }); }); }