Skip to content

[JExtract/JNI] Add support for primitive non-escaping closures. #327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 22, 2025

Conversation

madsodgaard
Copy link
Contributor

@madsodgaard madsodgaard commented Jul 21, 2025

Adds support for importing non-escaping closures that only handle primitive types supported by JavaKit.

Just like FFM, each closure is imported as an @FunctionalInterface that gets passed down to JNI. Then we call the apply method using JNI.

@ktoso
Copy link
Collaborator

ktoso commented Jul 22, 2025

Overall looks good! I was looking at the last commit in here :)

@madsodgaard madsodgaard changed the title WIP: [JExtract/JNI] Add support for primitive non-escaping closures. [JExtract/JNI] Add support for primitive non-escaping closures. Jul 22, 2025
@madsodgaard madsodgaard marked this pull request as ready for review July 22, 2025 11:03
let methodID$ = environment!.interface.GetMethodID(environment, class$, "apply", "(JZ)J")!
let arguments$: [jvalue] = [_0.getJValue(in: environment!), _1.getJValue(in: environment!)]
return Int64(fromJNI: environment!.interface.CallLongMethodA(environment, closure, methodID$, arguments$), in: environment!)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd kinda want to do one guard for the environment like we did elsewhere but this is minor, let's revisit holistically someday 👍

Copy link
Collaborator

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ktoso ktoso merged commit f62a317 into swiftlang:main Jul 22, 2025
80 of 84 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants