Repository navigation
[jni] Add deep conversion utils - #3635
Conversation
cefc389 to
90bb7ec
Compare
|
Fixed the directives_ordering analyzer failure. I'm not sure about the runtime_test.dart failure. |
It's a known flake. I really need to deflake all these tests, but I haven't been able to repro them locally. |
|
Added the required jni 1.0.3 changelog entry. |
|
Do you see any logical/API changes needed here? Also, should we support DateTime conversion like package:objective_c? |
liamappelbe
left a comment
There was a problem hiding this comment.
Also, should we support DateTime conversion like package:objective_c?
Not in this PR.
|
|
||
| extension DartListToJList on List<Object?> { | ||
| /// Deep converts this Dart [List] to a [JList]. | ||
| JList<JObject?> toJListDeep({ |
There was a problem hiding this comment.
Can you delete toJListDeep and instead do this deep conversion inside toJList? It doesn't really make sense to have a distinction between deep conversion and shallow conversion, because whether or not the conversion is deep is determined by the type of the element:
- If the element is a
JObject, thentoJObjectis a no-op, sotoJListDeepis a shallow conversion. - If the element is not a
JObjectthen it's impossible to do a shallow conversion.
So ToJavaList could check the type of the element and either shallow convert or not. But even that is just an optimisation (avoiding an unnecessary .map() call if the element is a JObject). I think even if you ignored that distinction, and always do .map(toNullableJObject) in toJList, it would be behaviorally identical.
Same for the other collections.
| ## 1.0.3 | ||
|
|
||
| - Fix JNI build errors on Linux arm64. | ||
| - Add deep conversion utilities for converting between Dart values and Java |
There was a problem hiding this comment.
1.0.3 is already published, so start a new version. This is adding new APIs, so it'll need to be a minor version bump to 1.1.0-wip. Update here and the pubspec.
|
Updated the collection conversion APIs to integrate deep conversion into toJList, toJSet, and toJMap, and bumped the package version to 1.1.0-wip. |
|
@liamappelbe, can you run the CI tests when you get the chance? |
|
I hit the Updating it to Should I include those regenerated bindings here, or is there a better way to handle the JNI version bump? |
To avoid the churn, maybe just don't run |
|
Fixed it by updating jniMinorVersion to 1 and leaving the generated bindings unchanged. The version test passes locally now. Could you rerun the CI tests? |
|
Looks like CI is now failing because the freshly generated bindings expect |
|
We can use this PR as a test case to fix the remaining version test relaxation issues. Ideally a version bump like this (one that doesn't affect the generated code) should just require bumping a few constants in monorepo, not regenerating dozens of bindings files. For example, this failure looks legit. It's another case that could be relaxed to not check the JNI version. This failure also looks legit. There's probably some hard coded version constants that need to be bumped. You should be able to repro both failures locally. |
|
Sounds good, I'll reproduce both failures locally and work on fixing them in this PR. |
|
Quick update on the failures:
The previously failing tests now pass locally without regenerating the bindings. Architectural question: the JNI major/minor version is hardcoded in a few places and manually kept in sync with jni/pubspec.yaml. Would it make sense to derive these from a single source of truth, or is keeping them explicit intentional? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3635 +/- ##
=======================================
Coverage ? 77.29%
=======================================
Files ? 53
Lines ? 3718
Branches ? 0
=======================================
Hits ? 2874
Misses ? 844
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| ); | ||
| } | ||
|
|
||
| extension DartIterableToJList on Iterable<Object?> { |
There was a problem hiding this comment.
Do we still need the converter in jlist.dart? Seems redundant.
Same for the ones in jmap.dart and jset.dart.
There was a problem hiding this comment.
The existing converters preserve the element types (Iterable.toJList() returns JList, and similarly for sets/maps), while the new converters accept Dart values and return JList<JObject?>/JSet<JObject?>/JMap<JObject?, JObject?>. I kept both to preserve the existing typed API.
Revisions updated by `dart tools/rev_sdk_deps.dart`. native (https://github.com/dart-lang/native/compare/f628b26..27f7678): 27f767827 Mon Sep 28 14:51:28 2026 +0200 Daco Harkes [hooks_runner] Remove `KernelAssets` and `Target` (dart-lang/native#3703) ddc7d6deb Mon Sep 28 05:40:03 2026 +0200 Jakob Kordež [ffigen] Skip typedefs to function pointers with unsupported parameter types (dart-lang/native#3694) 177d283d9 Mon Sep 28 04:30:28 2026 +0200 Jakob Kordež [ffigen] Generate bindings for C++ enums, structs and unions nested in namespaces and records (dart-lang/native#3644) 5ee8f67d8 Mon Sep 28 11:39:14 2026 +1000 Liam Appelbe [objective_c] Clarify block isolate scoping (dart-lang/native#3692) 25f090116 Fri Sep 25 12:09:12 2026 +1000 Liam Appelbe [ffigen][jnigen] Deprecate YAML config (dart-lang/native#3654) 8364413eb Fri Sep 25 10:09:50 2026 +1000 Liam Appelbe [jnigen] Document nullability (dart-lang/native#3690) e8d518574 Thu Sep 24 18:02:46 2026 +1000 Liam Appelbe [ffigen] Code snippet-ify ffigen and objective_c (dart-lang/native#3685) 663385955 Thu Sep 24 03:21:53 2026 +0300 Manar Elhabbal [ffigen] prevent setting both cpp and objectiveC in ffigenerator (dart-lang/native#3687) 1cf40952a Thu Sep 24 09:27:09 2026 +1000 Liam Appelbe [objective_c] Remove arm64e build hack (dart-lang/native#3680) c44550b6e Wed Sep 23 13:01:33 2026 +0200 Simon Binder [web_assets] Scaffold package (dart-lang/native#3610) 57b4bad69 Wed Sep 23 16:21:27 2026 +1000 Liam Appelbe [swift2objc] Code snippet-ify swift2objc (dart-lang/native#3684) 5cee1d0be Wed Sep 23 10:16:29 2026 +0530 Rachit Vikram Singh [ffigen] Add Input.appendCompilerOptions to append clang args to defaults (`#3630`) (dart-lang/native#3676) 225a648df Wed Sep 23 14:37:13 2026 +1000 Liam Appelbe [jnigen] Code snippet-ify jnigen and co (dart-lang/native#3641) 2030decf9 Wed Sep 23 13:51:24 2026 +1000 Liam Appelbe [infra] Move snippets_tool to its own package (dart-lang/native#3683) 6fa2beea2 Wed Sep 23 12:14:28 2026 +1000 Liam Appelbe [objective_c] Revert `#3664` (dart-lang/native#3681) ed7605f8a Tue Sep 22 12:03:22 2026 +0530 Sivapriya [jni] Add deep conversion utils (dart-lang/native#3635) 8574b7422 Tue Sep 22 08:31:11 2026 +0200 Lukas Klingsbo [ffigen] Allow package:cli_util 0.5.x and 0.6.x (dart-lang/native#3673) test (https://github.com/dart-lang/test/compare/9946c95..4f92d53): 4f92d530 Fri Sep 25 16:46:22 2026 -0700 Nate Bosch Unskip a test on windows (dart-lang/test#2772) 7fb84866 Fri Sep 25 14:52:39 2026 -0700 Nate Bosch Bundle skips at the end of the GitHub reporter (dart-lang/test#2768) 603e5ec5 Thu Sep 24 14:00:41 2026 -0700 Nate Bosch Clean up the runner's temporary directories (dart-lang/test#2763) 4bf04d90 Thu Sep 24 12:41:15 2026 -0700 Nate Bosch Refactor signal handling in executable.dart (dart-lang/test#2765) 2c660a84 Tue Sep 22 15:33:34 2026 -0700 Nate Bosch Regenerate mono_repo output (dart-lang/test#2767) tools (https://github.com/dart-lang/tools/compare/3614ce0..d87eaf7): d87eaf79 Thu Sep 24 08:31:33 2026 -0700 Kevin Moore fix(benchmark_harness): migrate subprocess spawning to package:cli_util (dart-lang/tools#2585) 8ccf8ccf Wed Sep 23 05:09:10 2026 +0530 Aman Maurya [string_scanner] Extract a mixin for LineScanner and deduplicate eager scanning logic (dart-lang/tools#2569) afcdd6d9 Tue Sep 22 15:38:07 2026 +0200 Morgan :) Document that watch events converge on filesystem state. (dart-lang/tools#2607) f2157cbc Tue Sep 22 04:52:58 2026 +0300 Hasan Armoush [code_builder] Add null-aware cascades, cascade index, and fluent cascade helpers (dart-lang/tools#2540) web (https://github.com/dart-lang/web/compare/e5c6c02..ec71a4c): ec71a4c Mon Sep 28 10:07:01 2026 -0700 Kevin Moore [js_interop_gen] Require `code_builder: ^4.12.0` and update golden headers (dart-lang/web#576) [email protected] Change-Id: I766bc5b2b1297e3251990821bb73387ba685e39b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/556320 Commit-Queue: Nate Bosch <[email protected]> Reviewed-by: Brian Quinlan <[email protected]>
Description
Adds deep conversion utilities to
package:jni, following the conversion API pattern inpackage:objective_c.This adds conversions between Dart values and JNI objects for primitive values, strings, lists, sets, and maps, with recursive conversion of nested collections and nullable values. It also adds conversion support for
JArrayand the primitive array variants.Java-to-Dart collection conversions return collections of
Object?, allowing callers to cast them to the expected types when needed. Unsupported types can be handled through theconvertOthercallback in either conversion direction.Related Issues
Fixes #3143
PR Checklist
dart tool/ci.dart --alllocally and resolved all issues identified. This ensures the PR is formatted, has no lint errors, and ran all code generators. This applies to the packages part of the toplevelpubspec.yamlworkspace.CHANGELOG.mdfor the relevant packages. (Not needed for small changes such as doc typos).