From 88a47b04095989b7722362751419d580c20fc227 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 26 Sep 2023 13:47:22 -0700 Subject: [PATCH 01/10] WIP static functions --- lib/src/code_generator/func.dart | 8 ++--- lib/src/code_generator/type.dart | 20 +++++++++++ test/native_objc_test/static_func_config.yaml | 13 +++++++ test/native_objc_test/static_func_test.dart | 34 +++++++++++++++++++ test/native_objc_test/static_func_test.m | 9 +++++ 5 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 test/native_objc_test/static_func_config.yaml create mode 100644 test/native_objc_test/static_func_test.dart create mode 100644 test/native_objc_test/static_func_test.m diff --git a/lib/src/code_generator/func.dart b/lib/src/code_generator/func.dart index f8a81773..a67f9bf7 100644 --- a/lib/src/code_generator/func.dart +++ b/lib/src/code_generator/func.dart @@ -131,17 +131,17 @@ class Func extends LookUpBinding { "@${w.ffiLibraryPrefix}.Native<$cType>(symbol: '$originalName'$assetString$isLeafString)\n"); s.write( - 'external ${functionType.returnType.getFfiDartType(w)} $enclosingFuncName(\n'); + 'external ${functionType.returnType.getDartType(w)} $enclosingFuncName(\n'); for (final p in functionType.dartTypeParameters) { - s.write(' ${p.type.getFfiDartType(w)} ${p.name},\n'); + s.write(' ${p.type.getDartType(w)} ${p.name},\n'); } s.write(');\n\n'); } else { // Write enclosing function. s.write( - '${functionType.returnType.getFfiDartType(w)} $enclosingFuncName(\n'); + '${functionType.returnType.getDartType(w)} $enclosingFuncName(\n'); for (final p in functionType.dartTypeParameters) { - s.write(' ${p.type.getFfiDartType(w)} ${p.name},\n'); + s.write(' ${p.type.getDartType(w)} ${p.name},\n'); } s.write(') {\n'); s.write('return $funcVarName'); diff --git a/lib/src/code_generator/type.dart b/lib/src/code_generator/type.dart index 1a69b607..0e93dd79 100644 --- a/lib/src/code_generator/type.dart +++ b/lib/src/code_generator/type.dart @@ -48,6 +48,26 @@ abstract class Type { /// as getFfiDartType. For ObjC bindings this refers to the wrapper object. String getDartType(Writer w) => getFfiDartType(w); + /// Converts a value from Dart type to FFI Dart type. Use this when taking an + /// arg from a user and converting it to something that can be passed to FFI. + String convertDartTypeToFfiDartType(Writer w, String value, { + String? nativeLib, + bool isNullable = false, + }) => value; + + /// Converts a value from FFI Dart type to Dart type. Use this when returning + /// a value from FFI to the user. + String convertFfiDartTypeToDartType(Writer w, String value, { + String? nativeLib, + bool isNullable = false, + bool objcRetain = false, + }) => value; + + /// Returns whether the above conversion functions require a reference to the + /// native library. If this method returns true, a non-null value must be + /// passed to the [nativeLib] argument of those functions. + bool convertRequiresNativeLib => false; + /// Returns the string representation of the Type, for debugging purposes /// only. This string should not be printed as generated code. @override diff --git a/test/native_objc_test/static_func_config.yaml b/test/native_objc_test/static_func_config.yaml new file mode 100644 index 00000000..72044198 --- /dev/null +++ b/test/native_objc_test/static_func_config.yaml @@ -0,0 +1,13 @@ +name: StaticFuncTestObjCLibrary +description: 'Test ObjC static functions' +language: objc +output: 'static_func_bindings.dart' +exclude-all-by-default: true +functions: + include: + - staticFuncReturningNSString +headers: + entry-points: + - 'static_func_test.m' +preamble: | + // ignore_for_file: camel_case_types, non_constant_identifier_names, unused_element, unused_field diff --git a/test/native_objc_test/static_func_test.dart b/test/native_objc_test/static_func_test.dart new file mode 100644 index 00000000..ba762473 --- /dev/null +++ b/test/native_objc_test/static_func_test.dart @@ -0,0 +1,34 @@ +// Copyright (c) 2023, 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. + +// Objective C support is only available on mac. +@TestOn('mac-os') + +import 'dart:ffi'; +import 'dart:io'; + +import 'package:test/test.dart'; +import '../test_utils.dart'; +import 'static_func_bindings.dart'; +import 'util.dart'; + +void main() { + late StaticFuncTestObjCLibrary lib; + + group('static functions', () { + setUpAll(() { + logWarnings(); + final dylib = File('test/native_objc_test/static_func_test.dylib'); + verifySetupFile(dylib); + lib = + StaticFuncTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path)); + generateBindingsForCoverage('static_func'); + }); + + test('Static function involving ObjC objects', () { + expect(lib.staticFuncReturningNSString().length, 12); + expect(lib.staticFuncReturningNSString().toString(), "Hello World!"); + }); + }); +} diff --git a/test/native_objc_test/static_func_test.m b/test/native_objc_test/static_func_test.m new file mode 100644 index 00000000..fcd3ef54 --- /dev/null +++ b/test/native_objc_test/static_func_test.m @@ -0,0 +1,9 @@ +// Copyright (c) 2023, 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 + +NSString* staticFuncReturningNSString() { + return @"Hello World!"; +} From deaf149d798c4e92f013889c142dd7f222134512 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Mon, 30 Oct 2023 12:09:16 +1300 Subject: [PATCH 02/10] Revert old code gen changes --- lib/src/code_generator/func.dart | 47 +++++--------- lib/src/code_generator/type.dart | 106 ++++++++++++++++++++----------- 2 files changed, 86 insertions(+), 67 deletions(-) diff --git a/lib/src/code_generator/func.dart b/lib/src/code_generator/func.dart index a67f9bf7..5e081cf7 100644 --- a/lib/src/code_generator/func.dart +++ b/lib/src/code_generator/func.dart @@ -46,23 +46,22 @@ class Func extends LookUpBinding { late final String funcPointerName; /// Contains typealias for function type if [exposeFunctionTypedefs] is true. - Typealias? _exposedCFunctionTypealias; - Typealias? _exposedDartFunctionTypealias; + Typealias? _exposedFunctionTypealias; /// [originalName] is looked up in dynamic library, if not /// provided, takes the value of [name]. Func({ - String? usr, + super.usr, required String name, - String? originalName, - String? dartDoc, + super.originalName, + super.dartDoc, required Type returnType, List? parameters, List? varArgParameters, this.exposeSymbolAddress = false, this.exposeFunctionTypedefs = false, this.isLeaf = false, - bool isInternal = false, + super.isInternal, this.ffiNativeConfig = const FfiNativeConfig(enabled: false), }) : functionType = FunctionType( returnType: returnType, @@ -70,11 +69,7 @@ class Func extends LookUpBinding { varArgParameters: varArgParameters ?? const [], ), super( - usr: usr, - originalName: originalName, name: name, - dartDoc: dartDoc, - isInternal: isInternal, ) { for (var i = 0; i < functionType.parameters.length; i++) { if (functionType.parameters[i].name.trim() == '') { @@ -85,15 +80,10 @@ class Func extends LookUpBinding { // Get function name with first letter in upper case. final upperCaseName = name[0].toUpperCase() + name.substring(1); if (exposeFunctionTypedefs) { - _exposedCFunctionTypealias = Typealias( - name: 'Native$upperCaseName', + _exposedFunctionTypealias = Typealias( + name: upperCaseName, type: functionType, - isInternal: true, - ); - _exposedDartFunctionTypealias = Typealias( - name: 'Dart$upperCaseName', - type: functionType, - useDartType: true, + genFfiDartType: true, isInternal: true, ); } @@ -115,12 +105,10 @@ class Func extends LookUpBinding { p.name = paramNamer.makeUnique(p.name); } - final cType = exposeFunctionTypedefs - ? _exposedCFunctionTypealias!.name - : functionType.getCType(w, writeArgumentNames: false); - final dartType = exposeFunctionTypedefs - ? _exposedDartFunctionTypealias!.name - : functionType.getFfiDartType(w, writeArgumentNames: false); + final cType = _exposedFunctionTypealias?.getCType(w) ?? + functionType.getCType(w, writeArgumentNames: false); + final dartType = _exposedFunctionTypealias?.getFfiDartType(w) ?? + functionType.getFfiDartType(w, writeArgumentNames: false); if (ffiNativeConfig.enabled) { final assetString = ffiNativeConfig.asset != null @@ -131,17 +119,17 @@ class Func extends LookUpBinding { "@${w.ffiLibraryPrefix}.Native<$cType>(symbol: '$originalName'$assetString$isLeafString)\n"); s.write( - 'external ${functionType.returnType.getDartType(w)} $enclosingFuncName(\n'); + 'external ${functionType.returnType.getFfiDartType(w)} $enclosingFuncName(\n'); for (final p in functionType.dartTypeParameters) { - s.write(' ${p.type.getDartType(w)} ${p.name},\n'); + s.write(' ${p.type.getFfiDartType(w)} ${p.name},\n'); } s.write(');\n\n'); } else { // Write enclosing function. s.write( - '${functionType.returnType.getDartType(w)} $enclosingFuncName(\n'); + '${functionType.returnType.getFfiDartType(w)} $enclosingFuncName(\n'); for (final p in functionType.dartTypeParameters) { - s.write(' ${p.type.getDartType(w)} ${p.name},\n'); + s.write(' ${p.type.getFfiDartType(w)} ${p.name},\n'); } s.write(') {\n'); s.write('return $funcVarName'); @@ -180,8 +168,7 @@ class Func extends LookUpBinding { dependencies.add(this); functionType.addDependencies(dependencies); if (exposeFunctionTypedefs) { - _exposedCFunctionTypealias!.addDependencies(dependencies); - _exposedDartFunctionTypealias!.addDependencies(dependencies); + _exposedFunctionTypealias!.addDependencies(dependencies); } } } diff --git a/lib/src/code_generator/type.dart b/lib/src/code_generator/type.dart index 0e93dd79..fff7f0f0 100644 --- a/lib/src/code_generator/type.dart +++ b/lib/src/code_generator/type.dart @@ -48,28 +48,45 @@ abstract class Type { /// as getFfiDartType. For ObjC bindings this refers to the wrapper object. String getDartType(Writer w) => getFfiDartType(w); - /// Converts a value from Dart type to FFI Dart type. Use this when taking an - /// arg from a user and converting it to something that can be passed to FFI. - String convertDartTypeToFfiDartType(Writer w, String value, { - String? nativeLib, - bool isNullable = false, - }) => value; - - /// Converts a value from FFI Dart type to Dart type. Use this when returning - /// a value from FFI to the user. - String convertFfiDartTypeToDartType(Writer w, String value, { - String? nativeLib, - bool isNullable = false, - bool objcRetain = false, - }) => value; - - /// Returns whether the above conversion functions require a reference to the - /// native library. If this method returns true, a non-null value must be - /// passed to the [nativeLib] argument of those functions. - bool convertRequiresNativeLib => false; - - /// Returns the string representation of the Type, for debugging purposes - /// only. This string should not be printed as generated code. + /// Returns whether the FFI dart type and C type string are same. + bool get sameFfiDartAndCType; + + /// Returns whether the dart type and C type string are same. + bool get sameDartAndCType => sameFfiDartAndCType; + + /// 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(); @@ -86,9 +103,6 @@ abstract class Type { String? getDefaultValue(Writer w, String nativeLib) => null; } -/// Function to check if the dart and C type string are same. -bool sameDartAndCType(Type t, Writer w) => t.getCType(w) == t.getFfiDartType(w); - /// Base class for all Type bindings. /// /// Since Dart doesn't have multiple inheritance, this type exists so that we @@ -96,18 +110,12 @@ bool sameDartAndCType(Type t, Writer w) => t.getCType(w) == t.getFfiDartType(w); /// to extend both NoLookUpBinding and Type. abstract class BindingType extends NoLookUpBinding implements Type { BindingType({ - String? usr, - String? originalName, - required String name, - String? dartDoc, - bool isInternal = false, - }) : super( - usr: usr, - originalName: originalName, - name: name, - dartDoc: dartDoc, - isInternal: isInternal, - ); + super.usr, + super.originalName, + required super.name, + super.dartDoc, + super.isInternal, + }); @override Type get baseType => this; @@ -127,6 +135,27 @@ abstract class BindingType extends NoLookUpBinding implements Type { @override String getDartType(Writer w) => getFfiDartType(w); + @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; @@ -145,4 +174,7 @@ class UnimplementedType extends Type { @override String toString() => '(Unimplemented: $reason)'; + + @override + bool get sameFfiDartAndCType => true; } From 45928c0d80c028324e0b4a95b60f7e0e2d4ae940 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Mon, 30 Oct 2023 12:42:43 +1300 Subject: [PATCH 03/10] Refactor function code gen --- lib/src/code_generator/func.dart | 53 +++++++++++++++----------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/lib/src/code_generator/func.dart b/lib/src/code_generator/func.dart index 5e081cf7..6518d19d 100644 --- a/lib/src/code_generator/func.dart +++ b/lib/src/code_generator/func.dart @@ -110,36 +110,31 @@ class Func extends LookUpBinding { final dartType = _exposedFunctionTypealias?.getFfiDartType(w) ?? functionType.getFfiDartType(w, writeArgumentNames: false); + final returnType = functionType.returnType.getFfiDartType(w); + final argDeclString = functionType.dartTypeParameters + .map((p) => '${p.type.getFfiDartType(w)} ${p.name},\n') + .join(''); + final argString = + functionType.dartTypeParameters.map((p) => '${p.name},\n').join(''); + final isLeafString = isLeaf ? 'isLeaf:true' : ''; + if (ffiNativeConfig.enabled) { final assetString = ffiNativeConfig.asset != null ? ", asset: '${ffiNativeConfig.asset}'" : ''; - final isLeafString = isLeaf ? ', isLeaf: true' : ''; - s.write( - "@${w.ffiLibraryPrefix}.Native<$cType>(symbol: '$originalName'$assetString$isLeafString)\n"); - - s.write( - 'external ${functionType.returnType.getFfiDartType(w)} $enclosingFuncName(\n'); - for (final p in functionType.dartTypeParameters) { - s.write(' ${p.type.getFfiDartType(w)} ${p.name},\n'); - } - s.write(');\n\n'); + s.write(''' +@${w.ffiLibraryPrefix}.Native<$cType>(symbol: '$originalName'$assetString$isLeafString) +external $returnType $enclosingFuncName($argDeclString); + +'''); } else { // Write enclosing function. - s.write( - '${functionType.returnType.getFfiDartType(w)} $enclosingFuncName(\n'); - for (final p in functionType.dartTypeParameters) { - s.write(' ${p.type.getFfiDartType(w)} ${p.name},\n'); - } - s.write(') {\n'); - s.write('return $funcVarName'); + s.write(''' +$returnType $enclosingFuncName($argDeclString) { + return $funcVarName($argString); +} - s.write('(\n'); - for (final p in functionType.dartTypeParameters) { - s.write(' ${p.name},\n'); - } - s.write(' );\n'); - s.write('}\n'); +'''); if (exposeSymbolAddress) { // Add to SymbolAddress in writer. @@ -150,12 +145,14 @@ class Func extends LookUpBinding { ptrName: funcPointerName, ); } + // Write function pointer. - s.write( - "late final $funcPointerName = ${w.lookupFuncIdentifier}<${w.ffiLibraryPrefix}.NativeFunction<$cType>>('$originalName');\n"); - final isLeafString = isLeaf ? 'isLeaf:true' : ''; - s.write( - 'late final $funcVarName = $funcPointerName.asFunction<$dartType>($isLeafString);\n\n'); + s.write(''' +late final $funcPointerName = ${w.lookupFuncIdentifier}< + ${w.ffiLibraryPrefix}.NativeFunction<$cType>>('$originalName'); +late final $funcVarName = $funcPointerName.asFunction<$dartType>($isLeafString); + +'''); } return BindingString(type: BindingStringType.func, string: s.toString()); From f299b245d49c73fe19717f4407672162ac0bd5f0 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Mon, 30 Oct 2023 14:05:13 +1300 Subject: [PATCH 04/10] Arg and return conversions --- lib/src/code_generator/func.dart | 47 +++++++++++++++++----- lib/src/code_generator/func_type.dart | 5 +++ lib/src/code_generator/objc_block.dart | 3 ++ lib/src/code_generator/objc_interface.dart | 3 ++ lib/src/code_generator/objc_nullable.dart | 3 ++ lib/src/code_generator/pointer.dart | 3 ++ lib/src/code_generator/type.dart | 6 +++ lib/src/code_generator/typealias.dart | 3 ++ 8 files changed, 63 insertions(+), 10 deletions(-) diff --git a/lib/src/code_generator/func.dart b/lib/src/code_generator/func.dart index 6518d19d..6cc52468 100644 --- a/lib/src/code_generator/func.dart +++ b/lib/src/code_generator/func.dart @@ -93,8 +93,6 @@ class Func extends LookUpBinding { BindingString toBindingString(Writer w) { final s = StringBuffer(); final enclosingFuncName = name; - final funcVarName = w.wrapperLevelUniqueNamer.makeUnique('_$name'); - funcPointerName = w.wrapperLevelUniqueNamer.makeUnique('_${name}Ptr'); if (dartDoc != null) { s.write(makeDartDoc(dartDoc!)); @@ -109,29 +107,58 @@ class Func extends LookUpBinding { functionType.getCType(w, writeArgumentNames: false); final dartType = _exposedFunctionTypealias?.getFfiDartType(w) ?? functionType.getFfiDartType(w, writeArgumentNames: false); + final needsWrapper = !functionType.sameDartAndFfiDartType; - final returnType = functionType.returnType.getFfiDartType(w); - final argDeclString = functionType.dartTypeParameters + final isLeafString = isLeaf ? 'isLeaf:true' : ''; + final funcVarName = w.wrapperLevelUniqueNamer.makeUnique('_$name'); + final ffiReturnType = functionType.returnType.getFfiDartType(w); + final dartReturnType = functionType.returnType.getDartType(w); + final ffiArgDeclString = functionType.dartTypeParameters .map((p) => '${p.type.getFfiDartType(w)} ${p.name},\n') .join(''); - final argString = - functionType.dartTypeParameters.map((p) => '${p.name},\n').join(''); - final isLeafString = isLeaf ? 'isLeaf:true' : ''; + + String dartArgDeclString = functionType.dartTypeParameters + .map((p) => '${p.type.getDartType(w)} ${p.name},\n') + .join(''); + if (needsWrapper) { + dartArgDeclString = '${w.className} lib, $dartArgDeclString'; + } + + final wrappedArgString = functionType.dartTypeParameters + .map((p) => + '${p.type.convertDartTypeToFfiDartType(w, p.name, objCRetain: false)},\n') + .join(''); + final wrappedFuncCall = + functionType.returnType.convertFfiDartTypeToDartType( + w, + '$funcVarName($wrappedArgString)', + 'lib', + objCRetain: true, + ); if (ffiNativeConfig.enabled) { final assetString = ffiNativeConfig.asset != null ? ", asset: '${ffiNativeConfig.asset}'" : ''; + final nativeFuncName = needsWrapper ? funcVarName : enclosingFuncName; s.write(''' @${w.ffiLibraryPrefix}.Native<$cType>(symbol: '$originalName'$assetString$isLeafString) -external $returnType $enclosingFuncName($argDeclString); +external $ffiReturnType $nativeFuncName($ffiArgDeclString); + +'''); + if (needsWrapper) { + s.write(''' +$dartReturnType $enclosingFuncName($dartArgDeclString) => $wrappedFuncCall; '''); + } } else { + funcPointerName = w.wrapperLevelUniqueNamer.makeUnique('_${name}Ptr'); + // Write enclosing function. s.write(''' -$returnType $enclosingFuncName($argDeclString) { - return $funcVarName($argString); +$dartReturnType $enclosingFuncName($dartArgDeclString) { + return $wrappedFuncCall; } '''); diff --git a/lib/src/code_generator/func_type.dart b/lib/src/code_generator/func_type.dart index 1e734bc4..49dcf605 100644 --- a/lib/src/code_generator/func_type.dart +++ b/lib/src/code_generator/func_type.dart @@ -78,6 +78,11 @@ class FunctionType extends Type { returnType.sameDartAndCType && dartTypeParameters.every((p) => p.type.sameDartAndCType); + @override + bool get sameDartAndFfiDartType => + returnType.sameDartAndFfiDartType && + dartTypeParameters.every((p) => p.type.sameDartAndFfiDartType); + @override String toString() => _getTypeImpl(false, (Type t) => t.toString()); diff --git a/lib/src/code_generator/objc_block.dart b/lib/src/code_generator/objc_block.dart index b61efc7c..d16ab112 100644 --- a/lib/src/code_generator/objc_block.dart +++ b/lib/src/code_generator/objc_block.dart @@ -228,6 +228,9 @@ _id.ref.invoke.cast<$natTrampFnType>().asFunction<$trampFuncFfiDartType>()( @override bool get sameDartAndCType => false; + @override + bool get sameDartAndFfiDartType => false; + @override String convertDartTypeToFfiDartType( Writer w, diff --git a/lib/src/code_generator/objc_interface.dart b/lib/src/code_generator/objc_interface.dart index 2ed1f10e..495cdf18 100644 --- a/lib/src/code_generator/objc_interface.dart +++ b/lib/src/code_generator/objc_interface.dart @@ -401,6 +401,9 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} { @override bool get sameDartAndCType => false; + @override + bool get sameDartAndFfiDartType => false; + @override String convertDartTypeToFfiDartType( Writer w, diff --git a/lib/src/code_generator/objc_nullable.dart b/lib/src/code_generator/objc_nullable.dart index a8d8c522..f3c0b877 100644 --- a/lib/src/code_generator/objc_nullable.dart +++ b/lib/src/code_generator/objc_nullable.dart @@ -44,6 +44,9 @@ class ObjCNullable extends Type { @override bool get sameDartAndCType => false; + @override + bool get sameDartAndFfiDartType => false; + @override String convertDartTypeToFfiDartType( Writer w, diff --git a/lib/src/code_generator/pointer.dart b/lib/src/code_generator/pointer.dart index dcaf5973..30e2e6ba 100644 --- a/lib/src/code_generator/pointer.dart +++ b/lib/src/code_generator/pointer.dart @@ -87,6 +87,9 @@ class ObjCObjectPointer extends PointerType { @override bool get sameDartAndCType => false; + @override + bool get sameDartAndFfiDartType => false; + @override String convertDartTypeToFfiDartType( Writer w, diff --git a/lib/src/code_generator/type.dart b/lib/src/code_generator/type.dart index fff7f0f0..c1d1e227 100644 --- a/lib/src/code_generator/type.dart +++ b/lib/src/code_generator/type.dart @@ -54,6 +54,9 @@ abstract class Type { /// Returns whether the dart type and C type string are same. bool get sameDartAndCType => sameFfiDartAndCType; + /// Returns whether the dart type and FFI dart type string are same. + bool get sameDartAndFfiDartType => true; + /// Returns generated Dart code that converts the given value from its /// DartType to its FfiDartType. /// @@ -138,6 +141,9 @@ abstract class BindingType extends NoLookUpBinding implements Type { @override bool get sameDartAndCType => sameFfiDartAndCType; + @override + bool get sameDartAndFfiDartType => true; + @override String convertDartTypeToFfiDartType( Writer w, diff --git a/lib/src/code_generator/typealias.dart b/lib/src/code_generator/typealias.dart index fe971a54..e31d1488 100644 --- a/lib/src/code_generator/typealias.dart +++ b/lib/src/code_generator/typealias.dart @@ -158,6 +158,9 @@ class Typealias extends BindingType { @override bool get sameDartAndCType => type.sameDartAndCType; + @override + bool get sameDartAndFfiDartType => type.sameDartAndFfiDartType; + @override String convertDartTypeToFfiDartType( Writer w, From 6aaf85795564e803f7e6d3c86017a77eae857623 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Mon, 30 Oct 2023 14:27:33 +1300 Subject: [PATCH 05/10] fix bugs --- lib/src/code_generator/func.dart | 49 +++++++++++++++++++------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/lib/src/code_generator/func.dart b/lib/src/code_generator/func.dart index 6cc52468..e475e11c 100644 --- a/lib/src/code_generator/func.dart +++ b/lib/src/code_generator/func.dart @@ -107,35 +107,44 @@ class Func extends LookUpBinding { functionType.getCType(w, writeArgumentNames: false); final dartType = _exposedFunctionTypealias?.getFfiDartType(w) ?? functionType.getFfiDartType(w, writeArgumentNames: false); - final needsWrapper = !functionType.sameDartAndFfiDartType; + final needsWrapper = !functionType.sameDartAndFfiDartType && !isInternal; final isLeafString = isLeaf ? 'isLeaf:true' : ''; final funcVarName = w.wrapperLevelUniqueNamer.makeUnique('_$name'); final ffiReturnType = functionType.returnType.getFfiDartType(w); - final dartReturnType = functionType.returnType.getDartType(w); final ffiArgDeclString = functionType.dartTypeParameters .map((p) => '${p.type.getFfiDartType(w)} ${p.name},\n') .join(''); - String dartArgDeclString = functionType.dartTypeParameters - .map((p) => '${p.type.getDartType(w)} ${p.name},\n') - .join(''); + late final String dartReturnType; + late final String dartArgDeclString; + late final String funcImplCall; if (needsWrapper) { - dartArgDeclString = '${w.className} lib, $dartArgDeclString'; + dartReturnType = functionType.returnType.getDartType(w); + dartArgDeclString = functionType.dartTypeParameters + .map((p) => '${p.type.getDartType(w)} ${p.name},\n') + .join(''); + + final argString = functionType.dartTypeParameters + .map((p) => + '${p.type.convertDartTypeToFfiDartType(w, p.name, objCRetain: false)},\n') + .join(''); + funcImplCall = + functionType.returnType.convertFfiDartTypeToDartType( + w, + '$funcVarName($argString)', + 'this', + objCRetain: true, + ); + } else { + dartReturnType = ffiReturnType; + dartArgDeclString = ffiArgDeclString; + final argString = functionType.dartTypeParameters + .map((p) => '${p.name},\n') + .join(''); + funcImplCall = '$funcVarName($argString)'; } - final wrappedArgString = functionType.dartTypeParameters - .map((p) => - '${p.type.convertDartTypeToFfiDartType(w, p.name, objCRetain: false)},\n') - .join(''); - final wrappedFuncCall = - functionType.returnType.convertFfiDartTypeToDartType( - w, - '$funcVarName($wrappedArgString)', - 'lib', - objCRetain: true, - ); - if (ffiNativeConfig.enabled) { final assetString = ffiNativeConfig.asset != null ? ", asset: '${ffiNativeConfig.asset}'" @@ -148,7 +157,7 @@ external $ffiReturnType $nativeFuncName($ffiArgDeclString); '''); if (needsWrapper) { s.write(''' -$dartReturnType $enclosingFuncName($dartArgDeclString) => $wrappedFuncCall; +$dartReturnType $enclosingFuncName($dartArgDeclString) => $funcImplCall; '''); } @@ -158,7 +167,7 @@ $dartReturnType $enclosingFuncName($dartArgDeclString) => $wrappedFuncCall; // Write enclosing function. s.write(''' $dartReturnType $enclosingFuncName($dartArgDeclString) { - return $wrappedFuncCall; + return $funcImplCall; } '''); From 8e9faf6c2cad12452cb7d6ceee8c3dceeced8241 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 31 Oct 2023 15:39:36 +1300 Subject: [PATCH 06/10] Fix analysis, add more tests --- CHANGELOG.md | 1 + lib/src/code_generator/func.dart | 15 ++- test/native_objc_test/block_config.yaml | 3 + test/native_objc_test/block_test.dart | 62 ++++----- test/native_objc_test/block_test.m | 30 +---- test/native_objc_test/static_func_config.yaml | 4 + .../static_func_native_config.yaml | 18 +++ .../static_func_native_test.dart | 122 ++++++++++++++++++ test/native_objc_test/static_func_test.dart | 88 ++++++++++++- test/native_objc_test/static_func_test.m | 51 ++++++++ test/native_objc_test/util.h | 35 +++++ 11 files changed, 355 insertions(+), 74 deletions(-) create mode 100644 test/native_objc_test/static_func_native_config.yaml create mode 100644 test/native_objc_test/static_func_native_test.dart create mode 100644 test/native_objc_test/util.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d90855d..ac9e0378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - When generating typedefs for `Pointer>`, also generate a typedef for the `Function`. - Use Dart wrapper types in args and returns of ObjCBlocks. +- Use Dart wrapper types in args and returns of static functions. - Bump min SDK version to 3.2.0-114.0.dev. # 9.0.1 diff --git a/lib/src/code_generator/func.dart b/lib/src/code_generator/func.dart index e475e11c..3a86de8f 100644 --- a/lib/src/code_generator/func.dart +++ b/lib/src/code_generator/func.dart @@ -129,19 +129,17 @@ class Func extends LookUpBinding { .map((p) => '${p.type.convertDartTypeToFfiDartType(w, p.name, objCRetain: false)},\n') .join(''); - funcImplCall = - functionType.returnType.convertFfiDartTypeToDartType( + funcImplCall = functionType.returnType.convertFfiDartTypeToDartType( w, '$funcVarName($argString)', - 'this', + ffiNativeConfig.enabled ? 'lib' : 'this', objCRetain: true, ); } else { dartReturnType = ffiReturnType; dartArgDeclString = ffiArgDeclString; - final argString = functionType.dartTypeParameters - .map((p) => '${p.name},\n') - .join(''); + final argString = + functionType.dartTypeParameters.map((p) => '${p.name},\n').join(''); funcImplCall = '$funcVarName($argString)'; } @@ -156,8 +154,11 @@ external $ffiReturnType $nativeFuncName($ffiArgDeclString); '''); if (needsWrapper) { + final libArg = functionType.returnType.sameDartAndFfiDartType + ? '' + : '${w.className} lib, '; s.write(''' -$dartReturnType $enclosingFuncName($dartArgDeclString) => $funcImplCall; +$dartReturnType $enclosingFuncName($libArg$dartArgDeclString) => $funcImplCall; '''); } diff --git a/test/native_objc_test/block_config.yaml b/test/native_objc_test/block_config.yaml index 841439e0..9f12686e 100644 --- a/test/native_objc_test/block_config.yaml +++ b/test/native_objc_test/block_config.yaml @@ -6,6 +6,9 @@ exclude-all-by-default: true objc-interfaces: include: - BlockTester +functions: + include: + - getBlockRetainCount headers: entry-points: - 'block_test.m' diff --git a/test/native_objc_test/block_test.dart b/test/native_objc_test/block_test.dart index 7ca61d6c..d2068ced 100644 --- a/test/native_objc_test/block_test.dart +++ b/test/native_objc_test/block_test.dart @@ -238,26 +238,26 @@ void main() { Pointer funcPointerBlockRefCountTest() { final block = IntBlock.fromFunctionPointer(lib, Pointer.fromFunction(_add100, 999)); - expect(BlockTester.getBlockRetainCount_(lib, block.pointer.cast()), 1); + expect(lib.getBlockRetainCount(block.pointer.cast()), 1); return block.pointer.cast(); } test('Function pointer block ref counting', () { final rawBlock = funcPointerBlockRefCountTest(); doGC(); - expect(BlockTester.getBlockRetainCount_(lib, rawBlock), 0); + expect(lib.getBlockRetainCount(rawBlock), 0); }); Pointer funcBlockRefCountTest() { final block = IntBlock.fromFunction(lib, makeAdder(4000)); - expect(BlockTester.getBlockRetainCount_(lib, block.pointer.cast()), 1); + expect(lib.getBlockRetainCount(block.pointer.cast()), 1); return block.pointer.cast(); } test('Function block ref counting', () { final rawBlock = funcBlockRefCountTest(); doGC(); - expect(BlockTester.getBlockRetainCount_(lib, rawBlock), 0); + expect(lib.getBlockRetainCount(rawBlock), 0); }); (Pointer, Pointer, Pointer) @@ -276,13 +276,10 @@ void main() { // One reference held by inputBlock object, another bound to the // outputBlock lambda. - expect( - BlockTester.getBlockRetainCount_(lib, inputBlock.pointer.cast()), 2); + expect(lib.getBlockRetainCount(inputBlock.pointer.cast()), 2); - expect( - BlockTester.getBlockRetainCount_(lib, blockBlock.pointer.cast()), 1); - expect( - BlockTester.getBlockRetainCount_(lib, outputBlock.pointer.cast()), 1); + expect(lib.getBlockRetainCount(blockBlock.pointer.cast()), 1); + expect(lib.getBlockRetainCount(outputBlock.pointer.cast()), 1); return ( inputBlock.pointer.cast(), blockBlock.pointer.cast(), @@ -297,10 +294,10 @@ void main() { // 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(lib.getBlockRetainCount(inputBlock), 1); - expect(BlockTester.getBlockRetainCount_(lib, blockBlock), 0); - expect(BlockTester.getBlockRetainCount_(lib, outputBlock), 0); + expect(lib.getBlockRetainCount(blockBlock), 0); + expect(lib.getBlockRetainCount(outputBlock), 0); }); (Pointer, Pointer, Pointer) @@ -316,11 +313,9 @@ void main() { 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); + expect(lib.getBlockRetainCount(inputBlock), 2); + expect(lib.getBlockRetainCount(blockBlock.pointer.cast()), 1); + expect(lib.getBlockRetainCount(outputBlock.pointer.cast()), 1); return ( inputBlock, blockBlock.pointer.cast(), @@ -335,10 +330,10 @@ void main() { // 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(lib.getBlockRetainCount(inputBlock), 2); - expect(BlockTester.getBlockRetainCount_(lib, blockBlock), 0); - expect(BlockTester.getBlockRetainCount_(lib, outputBlock), 0); + expect(lib.getBlockRetainCount(blockBlock), 0); + expect(lib.getBlockRetainCount(outputBlock), 0); }); (Pointer, Pointer, Pointer) @@ -353,13 +348,10 @@ void main() { // 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(lib.getBlockRetainCount(inputBlock.pointer.cast()), 2); - expect( - BlockTester.getBlockRetainCount_(lib, blockBlock.pointer.cast()), 1); - expect( - BlockTester.getBlockRetainCount_(lib, outputBlock.pointer.cast()), 1); + expect(lib.getBlockRetainCount(blockBlock.pointer.cast()), 1); + expect(lib.getBlockRetainCount(outputBlock.pointer.cast()), 1); return ( inputBlock.pointer.cast(), blockBlock.pointer.cast(), @@ -371,9 +363,9 @@ void main() { final (inputBlock, blockBlock, outputBlock) = nativeBlockBlockDartCallRefCountTest(); doGC(); - expect(BlockTester.getBlockRetainCount_(lib, inputBlock), 0); - expect(BlockTester.getBlockRetainCount_(lib, blockBlock), 0); - expect(BlockTester.getBlockRetainCount_(lib, outputBlock), 0); + expect(lib.getBlockRetainCount(inputBlock), 0); + expect(lib.getBlockRetainCount(blockBlock), 0); + expect(lib.getBlockRetainCount(outputBlock), 0); }); (Pointer, Pointer) nativeBlockBlockObjCCallRefCountTest() { @@ -382,18 +374,16 @@ void main() { expect(outputBlock(1), 14); doGC(); - expect( - BlockTester.getBlockRetainCount_(lib, blockBlock.pointer.cast()), 1); - expect( - BlockTester.getBlockRetainCount_(lib, outputBlock.pointer.cast()), 1); + expect(lib.getBlockRetainCount(blockBlock.pointer.cast()), 1); + expect(lib.getBlockRetainCount(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); + expect(lib.getBlockRetainCount(blockBlock), 0); + expect(lib.getBlockRetainCount(outputBlock), 0); }); (Pointer, Pointer) objectBlockRefCountTest(Allocator alloc) { diff --git a/test/native_objc_test/block_test.m b/test/native_objc_test/block_test.m index 3dd4289d..d27a97c8 100644 --- a/test/native_objc_test/block_test.m +++ b/test/native_objc_test/block_test.m @@ -5,6 +5,8 @@ #import #import +#include "util.h" + typedef struct { double x; double y; @@ -61,7 +63,6 @@ @interface BlockTester : NSObject { } + (BlockTester*)makeFromBlock:(IntBlock)block; + (BlockTester*)makeFromMultiplier:(int32_t)mult; -+ (uint64_t)getBlockRetainCount:(void*)block; - (int32_t)call:(int32_t)x; - (IntBlock)getBlock; - (void)pokeBlock; @@ -91,33 +92,6 @@ + (BlockTester*)makeFromMultiplier:(int32_t)mult { return bt; } -typedef struct { - void* isa; - int flags; - // There are other fields, but we just need the flags and isa. -} BlockRefCountExtractor; - -void* valid_block_isa = NULL; -+ (uint64_t)getBlockRetainCount:(void*)block { - BlockRefCountExtractor* b = (BlockRefCountExtractor*)block; - // HACK: The only way I can find to reliably figure out that a block has been - // deleted is to check the isa field (the lower bits of the flags field seem - // to be randomized, not just set to 0). But we also don't know the value this - // field has when it's constructed (copying the block changes it from - // _NSConcreteGlobalBlock to an internal value). So we assume that the first - // time this function is called, we have a valid block, and on subsequent - // calls we check to see if the isa field changed. - if (valid_block_isa == NULL) { - valid_block_isa = b->isa; - } - if (b->isa != valid_block_isa) { - return 0; - } - // The ref count is stored in the lower bits of the flags field, but skips the - // 0x1 bit. - return (b->flags & 0xFFFF) >> 1; -} - - (int32_t)call:(int32_t)x { return myBlock(x); } diff --git a/test/native_objc_test/static_func_config.yaml b/test/native_objc_test/static_func_config.yaml index 72044198..ba11da16 100644 --- a/test/native_objc_test/static_func_config.yaml +++ b/test/native_objc_test/static_func_config.yaml @@ -5,6 +5,10 @@ output: 'static_func_bindings.dart' exclude-all-by-default: true functions: include: + - getBlockRetainCount + - staticFuncOfObject + - staticFuncOfNullableObject + - staticFuncOfBlock - staticFuncReturningNSString headers: entry-points: diff --git a/test/native_objc_test/static_func_native_config.yaml b/test/native_objc_test/static_func_native_config.yaml new file mode 100644 index 00000000..b5a5b309 --- /dev/null +++ b/test/native_objc_test/static_func_native_config.yaml @@ -0,0 +1,18 @@ +name: StaticFuncTestObjCLibrary +description: 'Test ObjC static functions using @Native' +language: objc +output: 'static_func_native_bindings.dart' +exclude-all-by-default: true +ffi-native: +functions: + include: + - getBlockRetainCount + - staticFuncOfObject + - staticFuncOfNullableObject + - staticFuncOfBlock + - staticFuncReturningNSString +headers: + entry-points: + - 'static_func_test.m' +preamble: | + // ignore_for_file: camel_case_types, non_constant_identifier_names, unused_element, unused_field diff --git a/test/native_objc_test/static_func_native_test.dart b/test/native_objc_test/static_func_native_test.dart new file mode 100644 index 00000000..a2b0066a --- /dev/null +++ b/test/native_objc_test/static_func_native_test.dart @@ -0,0 +1,122 @@ +// Copyright (c) 2023, 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. + +// Objective C support is only available on mac. +@TestOn('mac-os') + +// Keep in sync with static_func_test.dart. These are the same tests, but using +// @Native. + +import 'dart:ffi'; +import 'dart:io'; + +import 'package:test/test.dart'; +import 'package:ffi/ffi.dart'; +import '../test_utils.dart'; +import 'static_func_native_bindings.dart'; +import 'util.dart'; + +typedef IntBlock = ObjCBlock_Int32_Int32; + +void main() { + late StaticFuncTestObjCLibrary lib; + late void Function(Pointer, Pointer) executeInternalCommand; + + group('static functions', () { + setUpAll(() { + logWarnings(); + final dylib = File('test/native_objc_test/static_func_test.dylib'); + verifySetupFile(dylib); + lib = + StaticFuncTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path)); + + executeInternalCommand = DynamicLibrary.process().lookupFunction< + Void Function(Pointer, Pointer), + void Function( + Pointer, Pointer)>('Dart_ExecuteInternalCommand'); + + generateBindingsForCoverage('static_func'); + }); + + doGC() { + final gcNow = "gc-now".toNativeUtf8(); + executeInternalCommand(gcNow.cast(), nullptr); + calloc.free(gcNow); + } + + test('Static function returning NSString', () { + expect(staticFuncReturningNSString(lib).length, 12); + expect(staticFuncReturningNSString(lib).toString(), "Hello World!"); + }); + + Pointer staticFuncOfObjectRefCountTest(Allocator alloc) { + final counter = alloc(); + counter.value = 0; + + final obj = StaticFuncTestObj.newWithCounter_(lib, counter); + expect(counter.value, 1); + + final outputObj = staticFuncOfObject(lib, obj); + expect(obj, outputObj); + expect(counter.value, 1); + + return counter; + } + + test( + 'Objects passed through static functions have correct ref counts', + () { + using((Arena arena) { + final (counter) = staticFuncOfObjectRefCountTest(arena); + doGC(); + expect(counter.value, 0); + }); + }); + + Pointer staticFuncOfNullableObjectRefCountTest(Allocator alloc) { + final counter = alloc(); + counter.value = 0; + + final obj = StaticFuncTestObj.newWithCounter_(lib, counter); + expect(counter.value, 1); + + final outputObj = staticFuncOfNullableObject(lib, obj); + expect(obj, outputObj); + expect(counter.value, 1); + + return counter; + } + + test( + 'Nullables passed through static functions have correct ref counts', + () { + using((Arena arena) { + final (counter) = staticFuncOfNullableObjectRefCountTest(arena); + doGC(); + expect(counter.value, 0); + + expect(staticFuncOfNullableObject(lib, null), isNull); + }); + }); + + Pointer staticFuncOfBlockRefCountTest() { + final block = IntBlock.fromFunction(lib, (int x) => 2 * x); + expect(getBlockRetainCount(block.pointer.cast()), 1); + + final outputBlock = staticFuncOfBlock(lib, block); + expect(block, outputBlock); + expect(getBlockRetainCount(block.pointer.cast()), 2); + + return block.pointer.cast(); + } + + test( + 'Blocks passed through static functions have correct ref counts', + () { + final (rawBlock) = staticFuncOfBlockRefCountTest(); + doGC(); + expect(getBlockRetainCount(rawBlock), 0); + }); + }); +} diff --git a/test/native_objc_test/static_func_test.dart b/test/native_objc_test/static_func_test.dart index ba762473..e05c23bc 100644 --- a/test/native_objc_test/static_func_test.dart +++ b/test/native_objc_test/static_func_test.dart @@ -5,30 +5,112 @@ // Objective C support is only available on mac. @TestOn('mac-os') +// Keep in sync with static_func_test.dart. These are the same tests, but +// without using @Native. + import 'dart:ffi'; import 'dart:io'; import 'package:test/test.dart'; +import 'package:ffi/ffi.dart'; import '../test_utils.dart'; import 'static_func_bindings.dart'; import 'util.dart'; +typedef IntBlock = ObjCBlock_Int32_Int32; + void main() { late StaticFuncTestObjCLibrary lib; + late void Function(Pointer, Pointer) executeInternalCommand; group('static functions', () { setUpAll(() { logWarnings(); final dylib = File('test/native_objc_test/static_func_test.dylib'); verifySetupFile(dylib); - lib = - StaticFuncTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path)); + lib = StaticFuncTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path)); + + executeInternalCommand = DynamicLibrary.process().lookupFunction< + Void Function(Pointer, Pointer), + void Function( + Pointer, Pointer)>('Dart_ExecuteInternalCommand'); + generateBindingsForCoverage('static_func'); }); - test('Static function involving ObjC objects', () { + doGC() { + final gcNow = "gc-now".toNativeUtf8(); + executeInternalCommand(gcNow.cast(), nullptr); + calloc.free(gcNow); + } + + test('Static function returning NSString', () { expect(lib.staticFuncReturningNSString().length, 12); expect(lib.staticFuncReturningNSString().toString(), "Hello World!"); }); + + Pointer staticFuncOfObjectRefCountTest(Allocator alloc) { + final counter = alloc(); + counter.value = 0; + + final obj = StaticFuncTestObj.newWithCounter_(lib, counter); + expect(counter.value, 1); + + final outputObj = lib.staticFuncOfObject(obj); + expect(obj, outputObj); + expect(counter.value, 1); + + return counter; + } + + test('Objects passed through static functions have correct ref counts', () { + using((Arena arena) { + final (counter) = staticFuncOfObjectRefCountTest(arena); + doGC(); + expect(counter.value, 0); + }); + }); + + Pointer staticFuncOfNullableObjectRefCountTest(Allocator alloc) { + final counter = alloc(); + counter.value = 0; + + final obj = StaticFuncTestObj.newWithCounter_(lib, counter); + expect(counter.value, 1); + + final outputObj = lib.staticFuncOfNullableObject(obj); + expect(obj, outputObj); + expect(counter.value, 1); + + return counter; + } + + test('Nullables passed through static functions have correct ref counts', + () { + using((Arena arena) { + final (counter) = staticFuncOfNullableObjectRefCountTest(arena); + doGC(); + expect(counter.value, 0); + + expect(lib.staticFuncOfNullableObject(null), isNull); + }); + }); + + Pointer staticFuncOfBlockRefCountTest() { + final block = IntBlock.fromFunction(lib, (int x) => 2 * x); + expect(lib.getBlockRetainCount(block.pointer.cast()), 1); + + final outputBlock = lib.staticFuncOfBlock(block); + expect(block, outputBlock); + expect(lib.getBlockRetainCount(block.pointer.cast()), 2); + + return block.pointer.cast(); + } + + test('Blocks passed through static functions have correct ref counts', () { + final (rawBlock) = staticFuncOfBlockRefCountTest(); + doGC(); + expect(lib.getBlockRetainCount(rawBlock), 0); + }); }); } diff --git a/test/native_objc_test/static_func_test.m b/test/native_objc_test/static_func_test.m index fcd3ef54..6287cb00 100644 --- a/test/native_objc_test/static_func_test.m +++ b/test/native_objc_test/static_func_test.m @@ -4,6 +4,57 @@ #import +#include "util.h" + +// Take and return object, nullable, and block +// ref counting tests + +@interface StaticFuncTestObj : NSObject { + int32_t* counter; +} ++ (instancetype)newWithCounter:(int32_t*) _counter; +- (instancetype)initWithCounter:(int32_t*) _counter; +- (void)setCounter:(int32_t*) _counter; +- (void)dealloc; +@end +@implementation StaticFuncTestObj + ++ (instancetype)newWithCounter:(int32_t*) _counter { + return [[StaticFuncTestObj 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 + +StaticFuncTestObj* staticFuncOfObject(StaticFuncTestObj* a) { + return a; +} + +StaticFuncTestObj* _Nullable staticFuncOfNullableObject( + StaticFuncTestObj* _Nullable a) { + return a; +} + +typedef int32_t (^IntBlock)(int32_t); +IntBlock staticFuncOfBlock(IntBlock a) { + return a; +} + NSString* staticFuncReturningNSString() { return @"Hello World!"; } diff --git a/test/native_objc_test/util.h b/test/native_objc_test/util.h new file mode 100644 index 00000000..0694aea5 --- /dev/null +++ b/test/native_objc_test/util.h @@ -0,0 +1,35 @@ +// Copyright (c) 2023, 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. + +#ifndef _TEST_UTIL_H_ +#define _TEST_UTIL_H_ + +typedef struct { + void* isa; + int flags; + // There are other fields, but we just need the flags and isa. +} BlockRefCountExtractor; + +static void* valid_block_isa = NULL; +uint64_t getBlockRetainCount(void* block) { + BlockRefCountExtractor* b = (BlockRefCountExtractor*)block; + // HACK: The only way I can find to reliably figure out that a block has been + // deleted is to check the isa field (the lower bits of the flags field seem + // to be randomized, not just set to 0). But we also don't know the value this + // field has when it's constructed (copying the block changes it from + // _NSConcreteGlobalBlock to an internal value). So we assume that the first + // time this function is called, we have a valid block, and on subsequent + // calls we check to see if the isa field changed. + if (valid_block_isa == NULL) { + valid_block_isa = b->isa; + } + if (b->isa != valid_block_isa) { + return 0; + } + // The ref count is stored in the lower bits of the flags field, but skips the + // 0x1 bit. + return (b->flags & 0xFFFF) >> 1; +} + +#endif // _TEST_UTIL_H_ From 63bc6a717d002b13d2afda5beea81b8627386d66 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 31 Oct 2023 15:42:36 +1300 Subject: [PATCH 07/10] fmt --- test/native_objc_test/static_func_native_test.dart | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/test/native_objc_test/static_func_native_test.dart b/test/native_objc_test/static_func_native_test.dart index a2b0066a..93ace899 100644 --- a/test/native_objc_test/static_func_native_test.dart +++ b/test/native_objc_test/static_func_native_test.dart @@ -28,8 +28,7 @@ void main() { logWarnings(); final dylib = File('test/native_objc_test/static_func_test.dylib'); verifySetupFile(dylib); - lib = - StaticFuncTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path)); + lib = StaticFuncTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path)); executeInternalCommand = DynamicLibrary.process().lookupFunction< Void Function(Pointer, Pointer), @@ -64,9 +63,7 @@ void main() { return counter; } - test( - 'Objects passed through static functions have correct ref counts', - () { + test('Objects passed through static functions have correct ref counts', () { using((Arena arena) { final (counter) = staticFuncOfObjectRefCountTest(arena); doGC(); @@ -88,8 +85,7 @@ void main() { return counter; } - test( - 'Nullables passed through static functions have correct ref counts', + test('Nullables passed through static functions have correct ref counts', () { using((Arena arena) { final (counter) = staticFuncOfNullableObjectRefCountTest(arena); @@ -111,9 +107,7 @@ void main() { return block.pointer.cast(); } - test( - 'Blocks passed through static functions have correct ref counts', - () { + test('Blocks passed through static functions have correct ref counts', () { final (rawBlock) = staticFuncOfBlockRefCountTest(); doGC(); expect(getBlockRetainCount(rawBlock), 0); From d22cf202798fb2e60be4a52a1a4880aa938c4074 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 1 Nov 2023 15:39:00 +1300 Subject: [PATCH 08/10] Fix autorelease pool test --- test/native_objc_test/automated_ref_count_test.m | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/native_objc_test/automated_ref_count_test.m b/test/native_objc_test/automated_ref_count_test.m index d2493896..cad84af2 100644 --- a/test/native_objc_test/automated_ref_count_test.m +++ b/test/native_objc_test/automated_ref_count_test.m @@ -89,12 +89,12 @@ - (ArcTestObject*)returnsRetained NS_RETURNS_RETAINED { @end -id createAutoreleasePool() { - return [NSAutoreleasePool new]; +void* createAutoreleasePool() { + return (void*)[NSAutoreleasePool new]; } -void destroyAutoreleasePool(id pool) { - [pool release]; +void destroyAutoreleasePool(void* pool) { + [((NSAutoreleasePool*)pool) release]; } @implementation RefCounted From 610d1dc3765bf60694e10e283fb5533347257b76 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 1 Nov 2023 17:21:08 +1300 Subject: [PATCH 09/10] Handle NS_RETURNS_RETAINED --- lib/src/code_generator/func.dart | 4 +- .../sub_parsers/functiondecl_parser.dart | 27 ++++++++++ .../automated_ref_count_test.m | 2 + test/native_objc_test/static_func_config.yaml | 3 +- .../static_func_native_config.yaml | 3 +- .../static_func_native_test.dart | 50 +++++++++++++++++-- test/native_objc_test/static_func_test.dart | 50 +++++++++++++++++-- test/native_objc_test/static_func_test.m | 14 ++++-- 8 files changed, 135 insertions(+), 18 deletions(-) diff --git a/lib/src/code_generator/func.dart b/lib/src/code_generator/func.dart index 3a86de8f..e79a3766 100644 --- a/lib/src/code_generator/func.dart +++ b/lib/src/code_generator/func.dart @@ -42,6 +42,7 @@ class Func extends LookUpBinding { final bool exposeSymbolAddress; final bool exposeFunctionTypedefs; final bool isLeaf; + final bool objCReturnsRetained; final FfiNativeConfig ffiNativeConfig; late final String funcPointerName; @@ -61,6 +62,7 @@ class Func extends LookUpBinding { this.exposeSymbolAddress = false, this.exposeFunctionTypedefs = false, this.isLeaf = false, + this.objCReturnsRetained = false, super.isInternal, this.ffiNativeConfig = const FfiNativeConfig(enabled: false), }) : functionType = FunctionType( @@ -133,7 +135,7 @@ class Func extends LookUpBinding { w, '$funcVarName($argString)', ffiNativeConfig.enabled ? 'lib' : 'this', - objCRetain: true, + objCRetain: !objCReturnsRetained, ); } else { dartReturnType = ffiReturnType; diff --git a/lib/src/header_parser/sub_parsers/functiondecl_parser.dart b/lib/src/header_parser/sub_parsers/functiondecl_parser.dart index f9d1121d..c2291b4a 100644 --- a/lib/src/header_parser/sub_parsers/functiondecl_parser.dart +++ b/lib/src/header_parser/sub_parsers/functiondecl_parser.dart @@ -2,6 +2,8 @@ // 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 'dart:ffi'; + import 'package:ffigen/src/code_generator.dart'; import 'package:ffigen/src/config_provider/config_types.dart'; import 'package:ffigen/src/header_parser/data.dart'; @@ -20,6 +22,7 @@ class _ParserFunc { List funcs = []; bool incompleteStructParameter = false; bool unimplementedParameterType = false; + bool objCReturnsRetained = false; _ParserFunc(); } @@ -69,6 +72,13 @@ List? parseFunctionDeclaration(clang_types.CXCursor cursor) { return _stack.pop().funcs; } + // Look for any annotations on the function. + clang.clang_visitChildren( + cursor, + _parseAnnotationVisitorPtr ??= Pointer.fromFunction( + _parseAnnotationVisitor, exceptional_visitor_return), + nullptr); + // Initialized with a single value with no prefix and empty var args. var varArgFunctions = [VarArgFunction('', [])]; if (config.varArgFunctions.containsKey(funcName)) { @@ -97,6 +107,7 @@ List? parseFunctionDeclaration(clang_types.CXCursor cursor) { exposeFunctionTypedefs: config.exposeFunctionTypedefs.shouldInclude(funcName), isLeaf: config.leafFunctions.shouldInclude(funcName), + objCReturnsRetained: _stack.top.objCReturnsRetained, ffiNativeConfig: config.ffiNativeConfig, )); } @@ -147,3 +158,19 @@ List _getParameters(clang_types.CXCursor cursor, String funcName) { Type _getParameterType(clang_types.CXCursor cursor) { return cursor.toCodeGenType(); } + +Pointer< + NativeFunction< + Int32 Function( + clang_types.CXCursor, clang_types.CXCursor, Pointer)>>? + _parseAnnotationVisitorPtr; +int _parseAnnotationVisitor(clang_types.CXCursor cursor, + clang_types.CXCursor parent, Pointer clientData) { + switch (cursor.kind) { + case clang_types.CXCursorKind.CXCursor_NSReturnsRetained: + _stack.top.objCReturnsRetained = true; + break; + default: + } + return clang_types.CXChildVisitResult.CXChildVisit_Continue; +} diff --git a/test/native_objc_test/automated_ref_count_test.m b/test/native_objc_test/automated_ref_count_test.m index cad84af2..cf8ab3a1 100644 --- a/test/native_objc_test/automated_ref_count_test.m +++ b/test/native_objc_test/automated_ref_count_test.m @@ -89,6 +89,8 @@ - (ArcTestObject*)returnsRetained NS_RETURNS_RETAINED { @end +// Pass around the NSAutoreleasePool as a void* to bypass the Dart wrappers so +// that we can precisely control the life cycle. void* createAutoreleasePool() { return (void*)[NSAutoreleasePool new]; } diff --git a/test/native_objc_test/static_func_config.yaml b/test/native_objc_test/static_func_config.yaml index ba11da16..bbafeaa7 100644 --- a/test/native_objc_test/static_func_config.yaml +++ b/test/native_objc_test/static_func_config.yaml @@ -9,7 +9,8 @@ functions: - staticFuncOfObject - staticFuncOfNullableObject - staticFuncOfBlock - - staticFuncReturningNSString + - staticFuncReturnsRetained + - staticFuncReturnsRetainedArg headers: entry-points: - 'static_func_test.m' diff --git a/test/native_objc_test/static_func_native_config.yaml b/test/native_objc_test/static_func_native_config.yaml index b5a5b309..2a591c9f 100644 --- a/test/native_objc_test/static_func_native_config.yaml +++ b/test/native_objc_test/static_func_native_config.yaml @@ -10,7 +10,8 @@ functions: - staticFuncOfObject - staticFuncOfNullableObject - staticFuncOfBlock - - staticFuncReturningNSString + - staticFuncReturnsRetained + - staticFuncReturnsRetainedArg headers: entry-points: - 'static_func_test.m' diff --git a/test/native_objc_test/static_func_native_test.dart b/test/native_objc_test/static_func_native_test.dart index 93ace899..93cbcad7 100644 --- a/test/native_objc_test/static_func_native_test.dart +++ b/test/native_objc_test/static_func_native_test.dart @@ -44,11 +44,6 @@ void main() { calloc.free(gcNow); } - test('Static function returning NSString', () { - expect(staticFuncReturningNSString(lib).length, 12); - expect(staticFuncReturningNSString(lib).toString(), "Hello World!"); - }); - Pointer staticFuncOfObjectRefCountTest(Allocator alloc) { final counter = alloc(); counter.value = 0; @@ -112,5 +107,50 @@ void main() { doGC(); expect(getBlockRetainCount(rawBlock), 0); }); + + Pointer staticFuncReturnsRetainedRefCountTest(Allocator alloc) { + final counter = alloc(); + counter.value = 0; + + final outputObj = staticFuncReturnsRetained(lib, counter); + expect(counter.value, 1); + + return counter; + } + + test( + 'Objects returned from static functions with NS_RETURNS_RETAINED ' + 'have correct ref counts', () { + using((Arena arena) { + final (counter) = staticFuncReturnsRetainedRefCountTest(arena); + doGC(); + expect(counter.value, 0); + }); + }); + + Pointer staticFuncOfObjectReturnsRetainedRefCountTest( + Allocator alloc) { + final counter = alloc(); + counter.value = 0; + + final obj = StaticFuncTestObj.newWithCounter_(lib, counter); + expect(counter.value, 1); + + final outputObj = staticFuncReturnsRetainedArg(lib, obj); + expect(obj, outputObj); + expect(counter.value, 1); + + return counter; + } + + test( + 'Objects passed through static functions with NS_RETURNS_RETAINED ' + 'have correct ref counts', () { + using((Arena arena) { + final (counter) = staticFuncOfObjectReturnsRetainedRefCountTest(arena); + doGC(); + expect(counter.value, 0); + }); + }); }); } diff --git a/test/native_objc_test/static_func_test.dart b/test/native_objc_test/static_func_test.dart index e05c23bc..f3d9c01b 100644 --- a/test/native_objc_test/static_func_test.dart +++ b/test/native_objc_test/static_func_test.dart @@ -44,11 +44,6 @@ void main() { calloc.free(gcNow); } - test('Static function returning NSString', () { - expect(lib.staticFuncReturningNSString().length, 12); - expect(lib.staticFuncReturningNSString().toString(), "Hello World!"); - }); - Pointer staticFuncOfObjectRefCountTest(Allocator alloc) { final counter = alloc(); counter.value = 0; @@ -112,5 +107,50 @@ void main() { doGC(); expect(lib.getBlockRetainCount(rawBlock), 0); }); + + Pointer staticFuncReturnsRetainedRefCountTest(Allocator alloc) { + final counter = alloc(); + counter.value = 0; + + final outputObj = lib.staticFuncReturnsRetained(counter); + expect(counter.value, 1); + + return counter; + } + + test( + 'Objects returned from static functions with NS_RETURNS_RETAINED ' + 'have correct ref counts', () { + using((Arena arena) { + final (counter) = staticFuncReturnsRetainedRefCountTest(arena); + doGC(); + expect(counter.value, 0); + }); + }); + + Pointer staticFuncOfObjectReturnsRetainedRefCountTest( + Allocator alloc) { + final counter = alloc(); + counter.value = 0; + + final obj = StaticFuncTestObj.newWithCounter_(lib, counter); + expect(counter.value, 1); + + final outputObj = lib.staticFuncReturnsRetainedArg(obj); + expect(obj, outputObj); + expect(counter.value, 1); + + return counter; + } + + test( + 'Objects passed through static functions with NS_RETURNS_RETAINED ' + 'have correct ref counts', () { + using((Arena arena) { + final (counter) = staticFuncOfObjectReturnsRetainedRefCountTest(arena); + doGC(); + expect(counter.value, 0); + }); + }); }); } diff --git a/test/native_objc_test/static_func_test.m b/test/native_objc_test/static_func_test.m index 6287cb00..a297053a 100644 --- a/test/native_objc_test/static_func_test.m +++ b/test/native_objc_test/static_func_test.m @@ -1,3 +1,4 @@ + // Copyright (c) 2023, 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. @@ -6,9 +7,6 @@ #include "util.h" -// Take and return object, nullable, and block -// ref counting tests - @interface StaticFuncTestObj : NSObject { int32_t* counter; } @@ -55,6 +53,12 @@ IntBlock staticFuncOfBlock(IntBlock a) { return a; } -NSString* staticFuncReturningNSString() { - return @"Hello World!"; +NS_RETURNS_RETAINED StaticFuncTestObj* staticFuncReturnsRetained( + int32_t* counter) { + return [StaticFuncTestObj newWithCounter: counter]; +} + +NS_RETURNS_RETAINED StaticFuncTestObj* staticFuncReturnsRetainedArg( + StaticFuncTestObj* a) { + return [a retain]; } From 2bdfda33be23482da4b4d8f5a8c7c97afb91f053 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Thu, 2 Nov 2023 13:30:38 +1300 Subject: [PATCH 10/10] Daco's comments --- test/native_objc_test/static_func_test.m | 47 ++++++++++++------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/test/native_objc_test/static_func_test.m b/test/native_objc_test/static_func_test.m index a297053a..5fc3a14d 100644 --- a/test/native_objc_test/static_func_test.m +++ b/test/native_objc_test/static_func_test.m @@ -1,4 +1,3 @@ - // Copyright (c) 2023, 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. @@ -14,29 +13,6 @@ + (instancetype)newWithCounter:(int32_t*) _counter; - (instancetype)initWithCounter:(int32_t*) _counter; - (void)setCounter:(int32_t*) _counter; - (void)dealloc; -@end -@implementation StaticFuncTestObj - -+ (instancetype)newWithCounter:(int32_t*) _counter { - return [[StaticFuncTestObj 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 StaticFuncTestObj* staticFuncOfObject(StaticFuncTestObj* a) { @@ -62,3 +38,26 @@ IntBlock staticFuncOfBlock(IntBlock a) { StaticFuncTestObj* a) { return [a retain]; } + + +@implementation StaticFuncTestObj ++ (instancetype)newWithCounter:(int32_t*) _counter { + return [[StaticFuncTestObj 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