Use NSProxy to implement ObjC protocols #1144

liamappelbe · 2024-05-14T01:46:32Z

This is half the solution to protocol implementation. It adds a user visible class in package:objective_c called DartProxy, which is based on NSProxy. It stores a map from selector to block, so users can implement methods using blocks. DartProxy objects are built using DartProxyBuilder, which is just a helper to build that map. There are no changes to ffigen code generation in this PR.

The other half of the solution will be code-gen for protocols that will make things more ergonomic, by hiding the details of DartProxy. It may be the case that some advanced use cases (eg implementing multiple protocols) may still need to use DartProxy, but I can still make it a bit more ergonomic than it is in protocol_test.dart once that code-gen lands.

Overview:

Classes:
- SEL (aka selector) represents the name of a method. It does not encode the arg/return types. It's essentially a canonicalized string. We create it using sel_registerName.
- NSMethodSignature represents the type of a method. It doesn't include the name. We can construct it from a const char* that encodes the arg/return types. We don't actually have to implement these signature encoding rules though, because we can do protocol_getMethodDescription(proto, selector,...).types.
- NSInvocation represents a single method call. It contains a SEL, a NSMethodSignature, a target object (which we bypass using invokeWithTarget), and a list of arguments. It also has space for the result.
- NSProxy is a class that lets us respond to arbitrary methods. It checks whether we implement a method using respondsToSelector, checks the signature using methodSignatureForSelector, and then invokes the method using forwardInvocation.
- DartProxy is our implementation of NSProxy, and has a map from SEL to (NSMethodSignature, user's block). Since SELs are already canonicalzed, we can use these pointers directly as map keys.
- DartProxyBuilder is used to build DartProxys. It has the same sort of map as DartProxy, but it's mutable. When constructing a DartProxy, we make an immutable (shallow) copy of that map, so that DartProxy objects can be accessed from arbitrary threads without worrying about synchronization.
The design of this solution relies on the fact that ObjC blocks are objects that can receive messages. But there's a bit of a difference in the way they receive them. Instead of the selector selecting which method is invoked, it's passed as an ordinary argument.
- The NSInvocation args for an ordinary object look like [target, selector, first arg, second arg...], whereas for a block they look like [block, first arg, second arg...].
- So in DartProxy.forwardInvocation we could construct a new NSInvocation and copy across all the args except the selector, or we could just write blocks that take the SEL as an extra arg that they ignore. I went with the second option.
protocol_test.m current manually defines all the method blocks. Once code-gen support for protocols lands, this won't be necessary.
Now that we're bundling both C and ObjC into the package:objective_c dylib, I had to rewrite the build script a bit.
This PR doesn't support implementing class methods, only instance methods. See Does anyone need to implement protocol class methods? #1149

#1040

pkgs/objective_c/macos/Classes/objective_c.c

github-actions · 2024-05-14T01:48:32Z

PR Health

Coverage ⚠️

Details

File	Coverage
pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart	💔 Not covered
pkgs/objective_c/lib/objective_c.dart	💔 Not covered
pkgs/objective_c/lib/src/c_bindings_generated.dart	💔 Not covered
pkgs/objective_c/lib/src/internal.dart	💔 Not covered
pkgs/objective_c/lib/src/objective_c_bindings_generated.dart	💔 Not covered

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

License Headers ✔️

Details

// 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.

Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/ffi/example/main.dart
pkgs/ffigen/example/libclang-example/generated_bindings.dart
pkgs/ffigen/example/shared_bindings/generate.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/a_shared_b_gen.dart
pkgs/ffigen/example/shared_bindings/lib/generated/base_gen.dart
pkgs/ffigen/example/simple/generated_bindings.dart
pkgs/ffigen/lib/src/config_provider/config_spec.dart
pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_decl_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_symbol_address_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_decl_type_name_collision_bindings.dart
pkgs/ffigen/test/collision_tests/expected_bindings/_expected_reserved_keyword_collision_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_comment_markup_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_dart_handle_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_forward_decl_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_functions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_imported_types_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_native_func_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_opaque_dependencies_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_packed_structs_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_regress_384_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_struct_fptr_fields_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_typedef_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_unions_bindings.dart
pkgs/ffigen/test/header_parser_tests/expected_bindings/_expected_varargs_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_cjson_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_libclang_bindings.dart
pkgs/ffigen/test/large_integration_tests/_expected_sqlite_bindings.dart
pkgs/ffigen/test/native_test/_expected_native_test_bindings.dart
pkgs/jni/lib/src/lang/jcharacter.dart
pkgs/jni/lib/src/third_party/generated_bindings.dart
pkgs/jni/lib/src/third_party/global_env_extensions.dart
pkgs/jni/lib/src/third_party/jni_bindings_generated.dart
pkgs/jnigen/android_test_runner/lib/main.dart
pkgs/jnigen/example/in_app_java/lib/android_utils.dart
pkgs/jnigen/example/kotlin_plugin/example/lib/main.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_bindings.dart
pkgs/jnigen/example/kotlin_plugin/lib/kotlin_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/pdfbox_plugin.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocument.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/PDDocumentInformation.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/pdmodel/_package.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/PDFTextStripper.dart
pkgs/jnigen/example/pdfbox_plugin/lib/src/third_party/org/apache/pdfbox/text/_package.dart
pkgs/jnigen/lib/src/bindings/descriptor.dart
pkgs/jnigen/lib/src/elements/elements.g.dart
pkgs/jnigen/test/jackson_core_test/third_party/bindings/com/fasterxml/jackson/core/_package.dart
pkgs/jnigen/tool/command_runner.dart
pkgs/native_assets_cli/test/model/checksum_test.dart

Package publish validation ✔️

Details

Package	Version	Status
package:ffi	2.1.2	already published at pub.dev
package:ffigen	13.0.0-wip	WIP (no publish necessary)
package:jni	0.9.3-wip	WIP (no publish necessary)
package:jnigen	0.9.1	already published at pub.dev
package:native_assets_builder	0.7.0	already published at pub.dev
package:native_assets_cli	0.6.0	already published at pub.dev
package:native_toolchain_c	0.4.2	already published at pub.dev
package:objective_c	1.1.0-wip	WIP (no publish necessary)
package:swiftgen	0.0.1-wip	WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

coveralls · 2024-05-14T01:54:45Z

coverage: 91.167% (+0.02%) from 91.147%
when pulling 9374ffa on proto
into 413dd27 on main.

dcharkes · 2024-05-17T09:13:26Z

Let me see if I understand the high level picture:

protocols is the Objective-C terminology for what is an interface in Dart or Java. (With the exception that Objective-C protocols can also have static methods which have dynamic dispatch. Does anyone need to implement protocol class methods? #1149)
The NSProxy uses some kind of reflection to dispatch arbitrary method invocations on it. This is similar to the way we use proxies in JNIgen. cc @HosseinYousefi
When a DartProxy is passed to objective C, it holds on to blocks created from Dart methods. Once the DartProxy object reaches refcount 0 in objective C, it is GCed, as such the blocks are refcount decremented, and if the blocks ref count gets 0, they are deleted, which then informs Dart that the closure is no longer referenced (already implemented).
- We should be weary of cycles, similar to with JNIgen. If your Dart closure captures a Dart object that holds on to an objective C object that holds on to the Dart proxy that holds on to that closure.

I think it might be useful to add the code gen to this PR (or stack a PR on top of this), and look at some example code users would write with the code-genned code to ensure we don't accidentally create extra cyclic references.

liamappelbe · 2024-05-17T09:21:01Z

Let me see if I understand the high level picture:

Yep, that's all accurate.

I think it might be useful to add the code gen to this PR (or stack a PR on top of this), and look at some example code users would write with the code-genned code to ensure we don't accidentally create extra cyclic references.

Ok. I'll start working on that.

dcharkes

lgtm based on pkgs/ffigen/test/native_objc_test/protocol_test.dart

I'm thinking that there are likely going to be changes to some of this code once code gen is built on top and we know better what we want/need. But I'm fine landing this as some lower layer infra that's tested by itself. Feel free to land, or layer the code-gen PR on top and re-request review once the code-gen PR is ready.

dcharkes · 2024-05-17T08:33:06Z

pkgs/objective_c/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 1.1.0-wip
+
+- Add `DartProxy`, which is an implementation of `NSProxy` that allows


allows -> enables
(access control versus technically possible)

dcharkes · 2024-05-17T09:15:36Z

pkgs/ffigen/test/native_objc_test/protocol_test.dart

+            isRequired: true, isInstance: true);
+        final block = DartInstanceMethodBlock.fromFunction(
+            (Pointer<Void> p, NSString s, double x) {
+          return 'DartProxy: $s: $x'.toNSString();


Use p? Or what is the p for?

It's the method selector. I'll hide it in the code gen layer. The only way to hide it from this layer would be option 4 from the doc, which has technical issues.

dcharkes · 2024-05-17T09:16:48Z

pkgs/ffigen/test/native_objc_test/protocol_test.dart

+        final otherSignature = getProtocolMethodSignature(secondProto, otherSel,
+            isRequired: true, isInstance: true);
+        final otherBlock = DartOtherMethodBlock.fromFunction(
+            (Pointer<Void> p, int a, int b, int c, int d) {


Ditto, what is the p for?

liamappelbe

I'm going to land this one. We're still working out the details of the code gen layer, but none of what we're discussing there will affect this layer.

liamappelbe · 2024-05-27T23:23:25Z

pkgs/ffigen/test/native_objc_test/protocol_test.dart

+            isRequired: true, isInstance: true);
+        final block = DartInstanceMethodBlock.fromFunction(
+            (Pointer<Void> p, NSString s, double x) {
+          return 'DartProxy: $s: $x'.toNSString();


It's the method selector. I'll hide it in the code gen layer. The only way to hide it from this layer would be option 4 from the doc, which has technical issues.

liamappelbe · 2024-05-27T23:25:11Z

pkgs/objective_c/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 1.1.0-wip
+
+- Add `DartProxy`, which is an implementation of `NSProxy` that allows


Use NSProxy to implement protocols

3fe4c5c

github-actions bot added package:ffigen package:objective_c labels May 14, 2024

liamappelbe commented May 14, 2024

View reviewed changes

pkgs/objective_c/macos/Classes/objective_c.c Show resolved Hide resolved

liamappelbe added 8 commits May 14, 2024 14:34

fix analysis

c7115d5

Update pubspec

b652b04

Fix changelog

d533c94

Handle unimplemented methods

a636316

Merge branch 'main' into proto

819cc1a

Multiple protocol implementation

9dc396e

Fix tests

457406d

Fix and test memory management

a82a6ed

liamappelbe added the skip-changelog-check label May 16, 2024

Separate building into DartProxyBuilder and test multithreading

9e65a95

liamappelbe changed the title ~~WIP: Use NSProxy to implement protocols~~ Use NSProxy to implement ObjC protocols May 17, 2024

Fix analysis

d5a4825

liamappelbe marked this pull request as ready for review May 17, 2024 00:32

liamappelbe requested a review from dcharkes May 17, 2024 00:32

dcharkes approved these changes May 17, 2024

View reviewed changes

Merge branch 'main' into proto

45da2fb

liamappelbe mentioned this pull request May 24, 2024

Code generation for ObjC protocols #1175

Merged

Merge branch 'main' into proto

d9cf7bb

liamappelbe commented May 27, 2024

View reviewed changes

Update readme

9374ffa

liamappelbe merged commit e5dee44 into main May 28, 2024
19 checks passed

liamappelbe deleted the proto branch May 28, 2024 05:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use NSProxy to implement ObjC protocols #1144

Use NSProxy to implement ObjC protocols #1144

liamappelbe commented May 14, 2024 •

edited

Loading

github-actions bot commented May 14, 2024 •

edited

Loading

coveralls commented May 14, 2024 •

edited

Loading

dcharkes commented May 17, 2024

liamappelbe commented May 17, 2024

dcharkes left a comment

dcharkes May 17, 2024

liamappelbe May 27, 2024

dcharkes May 17, 2024

liamappelbe May 27, 2024

dcharkes May 17, 2024

liamappelbe left a comment

liamappelbe May 27, 2024

liamappelbe May 27, 2024

		@@ -1,3 +1,8 @@
		## 1.1.0-wip

		- Add `DartProxy`, which is an implementation of `NSProxy` that allows

Use NSProxy to implement ObjC protocols #1144

Use NSProxy to implement ObjC protocols #1144

Conversation

liamappelbe commented May 14, 2024 • edited Loading

github-actions bot commented May 14, 2024 • edited Loading

PR Health

Coverage ⚠️

License Headers ✔️

Package publish validation ✔️

coveralls commented May 14, 2024 • edited Loading

dcharkes commented May 17, 2024

liamappelbe commented May 17, 2024

dcharkes left a comment

Choose a reason for hiding this comment

dcharkes May 17, 2024

Choose a reason for hiding this comment

liamappelbe May 27, 2024

Choose a reason for hiding this comment

dcharkes May 17, 2024

Choose a reason for hiding this comment

liamappelbe May 27, 2024

Choose a reason for hiding this comment

dcharkes May 17, 2024

Choose a reason for hiding this comment

liamappelbe left a comment

Choose a reason for hiding this comment

liamappelbe May 27, 2024

Choose a reason for hiding this comment

liamappelbe May 27, 2024

Choose a reason for hiding this comment

liamappelbe commented May 14, 2024 •

edited

Loading

github-actions bot commented May 14, 2024 •

edited

Loading

coveralls commented May 14, 2024 •

edited

Loading