native icon indicating copy to clipboard operation
native copied to clipboard

Use NSProxy to implement ObjC protocols

Open liamappelbe opened this issue 1 year ago • 4 comments
trafficstars

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 #1149

https://github.com/dart-lang/native/issues/1040

liamappelbe avatar May 14 '24 01:05 liamappelbe

PR Health

Coverage :warning:

Details
File Coverage
pkgs/ffigen/lib/src/code_generator/objc_built_in_functions.dart :broken_heart: Not covered
pkgs/objective_c/lib/objective_c.dart :broken_heart: Not covered
pkgs/objective_c/lib/src/c_bindings_generated.dart :broken_heart: Not covered
pkgs/objective_c/lib/src/internal.dart :broken_heart: Not covered
pkgs/objective_c/lib/src/objective_c_bindings_generated.dart :broken_heart: 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 :heavy_check_mark:

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 :heavy_check_mark:

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.

github-actions[bot] avatar May 14 '24 01:05 github-actions[bot]

Coverage Status

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

coveralls avatar May 14 '24 01:05 coveralls

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. https://github.com/dart-lang/native/issues/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.

dcharkes avatar May 17 '24 09:05 dcharkes

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.

liamappelbe avatar May 17 '24 09:05 liamappelbe