Fixes and tests for listener block native bindings (#1250)

liamappelbe · web-flow · commit 249d66549e8f · 2024-07-02T16:48:51.000-07:00
* Fixes and tests for listener block native bindings

* Rename entryPoints field to nativeEntryPoints.
diff --git a/pkgs/ffigen/lib/src/code_generator/compound.dart b/pkgs/ffigen/lib/src/code_generator/compound.dart
@@ -36,6 +36,11 @@ abstract class Compound extends BindingType {
 
   ObjCBuiltInFunctions? objCBuiltInFunctions;
 
+  /// The way the native type is written in C source code. This isn't always the
+  /// same as the originalName, because the type may need to be prefixed with
+  /// `struct` or `union`, depending on whether the declaration is a typedef.
+  final String nativeType;
+
   Compound({
     super.usr,
     super.originalName,
@@ -47,7 +52,9 @@ abstract class Compound extends BindingType {
     List<Member>? members,
     super.isInternal,
     this.objCBuiltInFunctions,
-  }) : members = members ?? [];
+    String? nativeType,
+  })  : members = members ?? [],
+        nativeType = nativeType ?? originalName ?? name;
 
   factory Compound.fromType({
     required CompoundType type,
@@ -59,6 +66,7 @@ abstract class Compound extends BindingType {
     String? dartDoc,
     List<Member>? members,
     ObjCBuiltInFunctions? objCBuiltInFunctions,
+    String? nativeType,
   }) {
     switch (type) {
       case CompoundType.struct:
@@ -71,6 +79,7 @@ abstract class Compound extends BindingType {
           dartDoc: dartDoc,
           members: members,
           objCBuiltInFunctions: objCBuiltInFunctions,
+          nativeType: nativeType,
         );
       case CompoundType.union:
         return Union(
@@ -82,6 +91,7 @@ abstract class Compound extends BindingType {
           dartDoc: dartDoc,
           members: members,
           objCBuiltInFunctions: objCBuiltInFunctions,
+          nativeType: nativeType,
         );
     }
   }
@@ -170,7 +180,7 @@ abstract class Compound extends BindingType {
   String getCType(Writer w) => _isBuiltIn ? '${w.objcPkgPrefix}.$name' : name;
 
   @override
-  String getNativeType({String varName = ''}) => '$originalName $varName';
+  String getNativeType({String varName = ''}) => '$nativeType $varName';
 
   @override
   bool get sameFfiDartAndCType => true;
diff --git a/pkgs/ffigen/lib/src/code_generator/library.dart b/pkgs/ffigen/lib/src/code_generator/library.dart
@@ -33,6 +33,7 @@ class Library {
     StructPackingOverride? packingOverride,
     Set<LibraryImport>? libraryImports,
     bool silenceEnumWarning = false,
+    List<String> nativeEntryPoints = const <String>[],
   }) {
     _findBindings(bindings, sort);
 
@@ -86,6 +87,7 @@ class Library {
       additionalImports: libraryImports,
       generateForPackageObjectiveC: generateForPackageObjectiveC,
       silenceEnumWarning: silenceEnumWarning,
+      nativeEntryPoints: nativeEntryPoints,
     );
   }
 
@@ -141,7 +143,7 @@ class Library {
   ///
   /// Returns whether bindings were generated.
   bool generateObjCFile(File file) {
-    final bindings = writer.generateObjC();
+    final bindings = writer.generateObjC(file.path);
 
     if (bindings == null) {
       // No ObjC code needed. If there's already a file (eg from an earlier
diff --git a/pkgs/ffigen/lib/src/code_generator/objc_block.dart b/pkgs/ffigen/lib/src/code_generator/objc_block.dart
@@ -243,6 +243,7 @@ pointer.ref.invoke.cast<$natTrampFnType>().asFunction<$trampFuncFfiDartType>()(
 
     final s = StringBuffer();
     s.write('''
+
 typedef ${getNativeType(varName: blockTypedef)};
 $blockTypedef $fnName($blockTypedef block) {
   $blockTypedef wrapper = [^void(${argsReceived.join(', ')}) {
diff --git a/pkgs/ffigen/lib/src/code_generator/objc_nullable.dart b/pkgs/ffigen/lib/src/code_generator/objc_nullable.dart
@@ -40,7 +40,7 @@ class ObjCNullable extends Type {
 
   @override
   String getNativeType({String varName = ''}) =>
-      '${child.getNativeType(varName: varName)} _Nullable';
+      '${child.getNativeType()} _Nullable $varName';
 
   @override
   bool get sameFfiDartAndCType => child.sameFfiDartAndCType;
diff --git a/pkgs/ffigen/lib/src/code_generator/struct.dart b/pkgs/ffigen/lib/src/code_generator/struct.dart
@@ -39,5 +39,6 @@ class Struct extends Compound {
     super.members,
     super.isInternal,
     super.objCBuiltInFunctions,
+    super.nativeType,
   }) : super(compoundType: CompoundType.struct);
 }
diff --git a/pkgs/ffigen/lib/src/code_generator/union.dart b/pkgs/ffigen/lib/src/code_generator/union.dart
@@ -37,5 +37,6 @@ class Union extends Compound {
     super.dartDoc,
     super.members,
     super.objCBuiltInFunctions,
+    super.nativeType,
   }) : super(compoundType: CompoundType.union);
 }
diff --git a/pkgs/ffigen/lib/src/code_generator/writer.dart b/pkgs/ffigen/lib/src/code_generator/writer.dart
@@ -5,6 +5,7 @@
 import 'package:ffigen/src/code_generator.dart';
 import 'package:ffigen/src/code_generator/utils.dart';
 import 'package:logging/logging.dart';
+import 'package:path/path.dart' as p;
 
 import '../strings.dart' as strings;
 
@@ -36,6 +37,8 @@ class Writer {
 
   final bool generateForPackageObjectiveC;
 
+  final List<String> nativeEntryPoints;
+
   /// Tracks where enumType.getCType is called. Reset everytime [generate] is
   /// called.
   bool usedEnumCType = false;
@@ -132,6 +135,7 @@ class Writer {
     this.header,
     required this.generateForPackageObjectiveC,
     required this.silenceEnumWarning,
+    required this.nativeEntryPoints,
   }) {
     final globalLevelNameSet = noLookUpBindings.map((e) => e.name).toSet();
     final wrapperLevelNameSet = lookUpBindings.map((e) => e.name).toSet();
@@ -387,13 +391,23 @@ class Writer {
   }
 
   /// Writes the Objective C code needed for the bindings, if any. Returns null
-  /// if there are no bindings that need generated ObjC code.
-  String? generateObjC() {
+  /// if there are no bindings that need generated ObjC code. This function does
+  /// not generate the output file, but the [outFilename] does affect the
+  /// generated code.
+  String? generateObjC(String outFilename) {
+    final outDir = p.dirname(outFilename);
+
     final s = StringBuffer();
     s.write('''
 #include <stdint.h>
 
 ''');
+
+    for (final entryPoint in nativeEntryPoints) {
+      final path = p.relative(entryPoint, from: outDir);
+      s.write('#include "$path"\n');
+    }
+
     bool empty = true;
     for (final binding in _allBindings) {
       final bindingString = binding.toObjCBindingString(this);
diff --git a/pkgs/ffigen/lib/src/config_provider/spec_utils.dart b/pkgs/ffigen/lib/src/config_provider/spec_utils.dart
@@ -35,8 +35,9 @@ String _replaceSeparators(String path) {
 String _normalizePath(String path, String? configFilename) {
   final skipNormalization =
       (configFilename == null) || p.isAbsolute(path) || path.startsWith("**");
-  return _replaceSeparators(
-      skipNormalization ? path : p.join(p.dirname(configFilename), path));
+  return _replaceSeparators(skipNormalization
+      ? path
+      : p.absolute(p.join(p.dirname(configFilename), path)));
 }
 
 Map<String, LibraryImport> libraryImportsExtractor(
diff --git a/pkgs/ffigen/lib/src/header_parser/parser.dart b/pkgs/ffigen/lib/src/header_parser/parser.dart
@@ -34,6 +34,7 @@ Library parse(Config c) {
     packingOverride: c.structPackingOverride,
     libraryImports: c.libraryImports.values.toSet(),
     silenceEnumWarning: c.silenceEnumWarning,
+    nativeEntryPoints: c.headers.entryPoints,
   );
 
   return library;
diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/compounddecl_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/compounddecl_parser.dart
@@ -124,6 +124,7 @@ Compound? parseCompoundDeclaration(
         usr: declUsr,
         dartDoc: getCursorDocComment(cursor),
         objCBuiltInFunctions: objCBuiltInFunctions,
+        nativeType: cursor.type().spelling(),
       );
     } else {
       _logger.finest('unnamed $className declaration');
@@ -139,6 +140,7 @@ Compound? parseCompoundDeclaration(
       name: configDecl.renameUsingConfig(declName),
       dartDoc: getCursorDocComment(cursor),
       objCBuiltInFunctions: objCBuiltInFunctions,
+      nativeType: cursor.type().spelling(),
     );
   }
   return null;
diff --git a/pkgs/ffigen/test/native_objc_test/block_config.yaml b/pkgs/ffigen/test/native_objc_test/block_config.yaml
@@ -19,8 +19,12 @@ typedefs:
     - NullableObjectBlock
     - BlockBlock
     - ListenerBlock
+    - NullableListenerBlock
+    - StructListenerBlock
+    - NSStringListenerBlock
+    - NoTrampolineListenerBlock
 headers:
   entry-points:
-    - 'block_test.m'
+    - 'block_test.h'
 preamble: |
   // ignore_for_file: camel_case_types, non_constant_identifier_names, unnecessary_non_null_assertion, unused_element, unused_field
diff --git a/pkgs/ffigen/test/native_objc_test/block_test.dart b/pkgs/ffigen/test/native_objc_test/block_test.dart
@@ -189,6 +189,69 @@ void main() {
       expect(isCalled, isTrue);
     });
 
+    test('Nullable listener block', () async {
+      final hasRun = Completer<void>();
+      final block = DartNullableListenerBlock.listener((DummyObject? x) {
+        expect(x, isNull);
+        hasRun.complete();
+      });
+
+      BlockTester.callNullableListener_(block);
+      await hasRun.future;
+    });
+
+    test('Struct listener block', () async {
+      final hasRun = Completer<void>();
+      final block = DartStructListenerBlock.listener(
+          (Vec2 vec2, Vec4 vec4, NSObject dummy) {
+        expect(vec2.x, 100);
+        expect(vec2.y, 200);
+
+        expect(vec4.x, 1.2);
+        expect(vec4.y, 3.4);
+        expect(vec4.z, 5.6);
+        expect(vec4.w, 7.8);
+
+        expect(dummy, isNotNull);
+
+        hasRun.complete();
+      });
+
+      BlockTester.callStructListener_(block);
+      await hasRun.future;
+    });
+
+    test('NSString listener block', () async {
+      final hasRun = Completer<void>();
+      final block = DartNSStringListenerBlock.listener((NSString s) {
+        expect(s.toString(), "Foo 123");
+        hasRun.complete();
+      });
+
+      BlockTester.callNSStringListener_x_(block, 123);
+      await hasRun.future;
+    });
+
+    test('No trampoline listener block', () async {
+      final hasRun = Completer<void>();
+      final block = DartNoTrampolineListenerBlock.listener(
+          (int x, Vec4 vec4, Pointer<Char> charPtr) {
+        expect(x, 123);
+
+        expect(vec4.x, 1.2);
+        expect(vec4.y, 3.4);
+        expect(vec4.z, 5.6);
+        expect(vec4.w, 7.8);
+
+        expect(charPtr.cast<Utf8>().toDartString(), "Hello World");
+
+        hasRun.complete();
+      });
+
+      BlockTester.callNoTrampolineListener_(block);
+      await hasRun.future;
+    });
+
     test('Block block', () {
       final blockBlock = DartBlockBlock.fromFunction((DartIntBlock intBlock) {
         return DartIntBlock.fromFunction((int x) {
diff --git a/pkgs/ffigen/test/native_objc_test/block_test.h b/pkgs/ffigen/test/native_objc_test/block_test.h
@@ -0,0 +1,70 @@
+// Copyright (c) 2024, 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 <Foundation/NSObject.h>
+#import <Foundation/NSString.h>
+#import <Foundation/NSThread.h>
+
+struct Vec2 {
+  double x;
+  double y;
+};
+
+typedef struct {
+  double x;
+  double y;
+  double z;
+  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
+
+
+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);
+typedef void (^ListenerBlock)(IntBlock);
+typedef void (^NullableListenerBlock)(DummyObject* _Nullable);
+typedef void (^StructListenerBlock)(struct Vec2, Vec4, NSObject*);
+typedef void (^NSStringListenerBlock)(NSString*);
+typedef void (^NoTrampolineListenerBlock)(int32_t, Vec4, const char*);
+
+// Wrapper around a block, so that our Dart code can test creating and invoking
+// blocks in Objective C code.
+@interface BlockTester : NSObject {
+  IntBlock myBlock;
+}
++ (BlockTester*)makeFromBlock:(IntBlock)block;
++ (BlockTester*)makeFromMultiplier:(int32_t)mult;
+- (int32_t)call:(int32_t)x;
+- (IntBlock)getBlock;
+- (void)pokeBlock;
++ (void)callOnSameThread:(VoidBlock)block;
++ (NSThread*)callOnNewThread:(VoidBlock)block;
++ (NSThread*)callWithBlockOnNewThread:(ListenerBlock)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;
++ (void)callListener:(ListenerBlock)block;
++ (void)callNullableListener:(NullableListenerBlock)block;
++ (void)callStructListener:(StructListenerBlock)block;
++ (void)callNSStringListener:(NSStringListenerBlock)block x:(int32_t)x;
++ (void)callNoTrampolineListener:(NoTrampolineListenerBlock)block;
++ (IntBlock)newBlock:(BlockBlock)block withMult:(int)mult;
++ (BlockBlock)newBlockBlock:(int)mult;
+@end
diff --git a/pkgs/ffigen/test/native_objc_test/block_test.m b/pkgs/ffigen/test/native_objc_test/block_test.m

Original file line number	Diff line number	Diff line change
`@@ -39,5 +39,6 @@ class Struct extends Compound {`
`39`	`39`	`super.members,`
`40`	`40`	`super.isInternal,`
`41`	`41`	`super.objCBuiltInFunctions,`
	`42`	`+ super.nativeType,`
`42`	`43`	`}) : super(compoundType: CompoundType.struct);`
`43`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,5 +37,6 @@ class Union extends Compound {`
`37`	`37`	`super.dartDoc,`
`38`	`38`	`super.members,`
`39`	`39`	`super.objCBuiltInFunctions,`
	`40`	`+ super.nativeType,`
`40`	`41`	`}) : super(compoundType: CompoundType.union);`
`41`	`42`	`}`