Skip to content

JavaKit: Rework @ImplementsJava to be more like @implements language feature #105

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 13 commits into from
Oct 24, 2024

Conversation

DougGregor
Copy link
Member

@DougGregor DougGregor commented Oct 23, 2024

The @ImplementsJava macro had an odd formulation where it was used similarly to @JavaMethod, but was actually interpreted by the outer @JavaClass macro. Completely rework the way @ImplementsJava is used to make it more like the @implements feature introduced in SE-0436, and rename it to @JavaImplementation to more closely match the canonical spelling for @implementation with a language and make use of the @Java... as a common prefix.

In this new design, one places @JavaImplementation on an extension of the Swift type, conforms to the corresponding protocol, and marks the implementing methods with @JavaMethod. For example:

@JavaImplementation("org.swift.javakit.HelloSwift")
extension Hello: HelloNativeMethods {
  @JavaMethod
  func reportStatistics(_ meaning: String, _ numbers: [Double]) -> String {
    let average = numbers.isEmpty ? 0.0 : numbers.reduce(0.0) { $0 + $1 } / Double(numbers.count)
    return "Average of \(meaning) is \(average)"
  }
}

This also means that we can generate most of the @JavaClass definition for a given Java class, and leave the native methods to be implemented by an @JavaImplementation extension. The NativeMethods protocol provides declarations for all of the native methods that are expected to be implemented in Swift, making it easier to get all of
the signatures right.

Update the JavaKitSampleApp to use the Java compilation plugin to compile the Java files in the target. Automatically feed those into the Java2Swift plugin to generate the Swift representations of the Java classes, skipping the native methods (since they'll be implemented here).

The user guide / tutorial is a bit behind the implementation here as I smooth out more corners.

…age feature

The `@ImplementsJava` macro had an odd formulation where it was used similarly
to `@JavaMethod`, but was actually interpreted by the outer `@JavaClass` macro.
Completely rework the way `@ImplementsJava` is used to make it more like the
`@implements` feature introduced in SE-0436, and rename it to `@JavaImplements`
to more closely match the canonical spelling for `@implementation` with a
language and make use of the `@Java...` as a common prefix.

In this new design, one places `@JavaImplements` on an extension of the Swift
type and marks the implementing methods with `@JavaMethod`. For example:

    @JavaImplements("org.swift.javakit.HelloSwift")
    extension Hello {
      @JavaMethod
      func reportStatistics(_ meaning: String, _ numbers: [Double]) -> String {
        let average = numbers.isEmpty ? 0.0 : numbers.reduce(0.0) { $0 + $1 } / Double(numbers.count)
        return "Average of \(meaning) is \(average)"
      }
    }

This also means that we can generate most of the `@JavaClass` definition
for a given Java class, and leave the native methods to be implemented
by an `@JavaImplements` extension. Update the JavaKitSampleApp to use the
Java compilation plugin to compile the Java files in the target. Automatically
feed those into the Java2Swift plugin to generate the Swift
representations of the Java classes, skipping the native methods
(since they'll be implemented here).

The user guide / tutorial is a bit behind the implementation here as I
smooth out more corners.
When an imported Java class has native methods that we expect to implement
from Swift, create a protocol names <Swift Type>NativeMethods that declares
all of the methods that need to be implemented.

From the user perspective, one should make the `@JavaImplements` extension
conform to this protocol. Then, the compiler will ensure that the right
Swift methods exist with the right signatures. Note that the user is
still responsible for ensuring that the appropriate `@JavaImplements` and
`@JavaMethod` annotations are present.
Implementations of `native` methods are written in an extension of the Swift type that has been marked with `@JavaImplementation`. The methods themselves must be marked with `@JavaMethod`, indicating that they are available to Java as well. To help ensure that the Swift code implements all of the `native` methods with the right signatures, JavaKit produces a protocol with the Swift type name suffixed by `NativeMethods`. Declare conformance to that protocol and implement its requirements, for example:

```swift
@JavaImplementation("org.swift.javakit.HelloSwift")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a thought on the names matching dance here: can we do better than that? This currently needs to match up the java file, the config file, and the swift file. Can the config file eventually become an implementation detail? It seems like a moving piece that folks are bound to forget about when e.g. moving the java type to some other package 🤔

Perhaps just more good diagnostics would be a way to address this as well.

Occurred to me now while reviewing and comparing with how much of a hassle this is vs integrating Java/JNI/C 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it depends on how much work we're willing to do in the plugin itself. The config file lets us predetermine all of the .swift files that will be generated by reading just the one file. If we want to extract that information from the Swift or Java sources, we have to do a lot more digging just to set up the build graph.

The *NativeMethods protocol should help a bit with the names-matching, though. You write the extension to conform to protocol, and the compiler makes sure that all of the names and types match the native function.

Good diagnostics would definitely help here, and we should think more about whether it's okay to generate more of the config file to save steps.

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.

Overall looking good, let's keep polishing this up :)

We can figure out what the problem with necessitating public was here, it should not be a requirement for native AFAIR

@DougGregor
Copy link
Member Author

We can figure out what the problem with necessitating public was here, it should not be a requirement for native AFAIR

Yeah, I think I know the problem here. See #106

@DougGregor DougGregor force-pushed the java-implements-rework branch from fa99f2e to 8173b71 Compare October 24, 2024 05:07
@DougGregor DougGregor force-pushed the java-implements-rework branch from 5409b2e to 5ee3126 Compare October 24, 2024 05:54
@DougGregor
Copy link
Member Author

e87c3bf addressed the crash in the tests that were causing me trouble on Linux. I suspect the development toolchain might be miscompiling closure captures of parameter packs in some cases.

@DougGregor DougGregor merged commit b1a82db into swiftlang:main Oct 24, 2024
11 checks passed
@DougGregor DougGregor deleted the java-implements-rework branch October 25, 2024 02:02
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