Skip to content

Generate JavaKitFunction module #119

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 2 commits into from
Oct 28, 2024
Merged

Generate JavaKitFunction module #119

merged 2 commits into from
Oct 28, 2024

Conversation

iainsmith
Copy link
Contributor

@iainsmith iainsmith commented Oct 27, 2024

Add JavaKitFunction module & Makefile action as first steps for #84

Callsite Example

Java

public Predicate<Integer> lessThanTen() {
  Predicate<Integer> predicate = i -> (i < 10);
  return predicate;
}

Swift

let predicate: JavaPredicate<JavaInteger> = self.lessThanTen()! 
let value = predicate.test(JavaInteger(3).as(JavaObject.self)) // question about removing need for this cast below?
print("Running a predicate from swift 3 < 10 = \(value)")

Updated JavaKitExample available in this commit 7baa992

Questions

  • Is this roughly what you had in mind or do you want to go with a different direction?
  • For the callAsFunction implementation, should we add those by hand or is it worth updating the generator / config?
  • Are you happy if we update the existing JavaKitExample to import JavaKitCollection and JavaKitFunction so these get run through CI?
  • The generated API doesn't enforce the Generic parameter. Should we update the @JavaMethod macro to support handling the cast to JavaObject? That would let us move from:
@JavaInterface("java.util.function.Predicate")
public struct JavaPredicate<T: AnyJavaObject> {
  @JavaMethod
-  public func test(_ arg0: JavaObject?) -> Bool
+  public func test(_ arg0: T?) -> Bool

// Callsite

let predicate: JavaPredicate<JavaInteger> = self.lessThanTen()! 
- let value = predicate.test(JavaInteger(3).as(JavaObject.self))
+ let value = predicate.test(JavaInteger(3))

@ktoso
Copy link
Collaborator

ktoso commented Oct 27, 2024

Looks fine, PR doesn't seem to include changes to samples or a test though.

Would you be interested in helping out in the other direction as well? We need to support all these in jextract as well: #101

@iainsmith
Copy link
Contributor Author

PR doesn't seem to include changes to samples or a test though.

  • Do you want a new sample or to add a few functions to the existing sample in JavaKitExample/com/example/swift/HelloSwift.java?
  • What's the approach for testing generated modules like these, I couldn't see anything for JavaKitCollection?

We need to support all these in jextract as well: #101

I haven't been able to get make jextract-run to run yet, using either main-snapshot-2024-10-25 or 6.0-snapshot-2024-10-12, but I'd be keen to give it a go as time allows.

Comment on lines +48 to +50
let predicate: JavaPredicate<JavaInteger> = self.lessThanTen()!
let value = predicate.test(JavaInteger(3).as(JavaObject.self))
print("Running a JavaPredicate from swift 3 < 10 = \(value)")
Copy link
Contributor Author

@iainsmith iainsmith Oct 27, 2024

Choose a reason for hiding this comment

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

Not a particular compelling example, but illustrates the module working.

Copy link
Collaborator

@ktoso ktoso Oct 28, 2024

Choose a reason for hiding this comment

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

I think this is fine, just at least some soundness check that it works at all 👍

I'd put it into a test file in this project and we're good them IMHO.

@DougGregor
Copy link
Member

Add JavaKitFunction module & Makefile action as first steps for

Cool, thanks!

  • Is this roughly what you had in mind or do you want to go with a different direction?

Yes, this is what I had in mind.

  • For the callAsFunction implementation, should we add those by hand or is it worth updating the generator / config?

If we can update the generator, that would be ideal. Perhaps we could use the FunctionalInterface annotation in the generator to determine when we should add callAsFunction?

  • Are you happy if we update the existing JavaKitExample to import JavaKitCollection and JavaKitFunction so these get run through CI?

JavaKitExample is a little bit of a mess right now, and might not stick around as we get more focused and interesting samples. The ideal place to add tests for this kind of thing is in the Java2SwiftTests unit test target, where we translate individual classes and can check for the right output. It's a whole lot faster to iterate there than through the JavaKitExample project.

  • The generated API doesn't enforce the Generic parameter. Should we update the @JavaMethod macro to support handling the cast to JavaObject?

If we can improve the translator to provide this improved type information automatically, that would be wonderful! Ideally, it would be a separate change to this pull request, because it's likely to have an effect on many other translations into Swift.

@DougGregor
Copy link
Member

Let's go ahead and merge as-is, and any of the suggestions we made above can be follow-ups. Thanks!

@DougGregor DougGregor merged commit 633c03b into swiftlang:main Oct 28, 2024
11 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.

3 participants