PR feedback

Author: madsodgaard

Plugins/JExtractSwiftPlugin/JExtractSwiftPlugin.swift

@@ -114,13 +114,6 @@ struct JExtractSwiftBuildToolPlugin: SwiftJavaPluginProtocol, BuildToolPlugin {
       outputSwiftDirectory.appending(path: "\(sourceModule.name)Module+SwiftJava.swift")
     ]
 
-    // Append any JNI cache files
-    if configuration?.mode == .jni {
-      outputSwiftFiles += [
-        outputSwiftDirectory.appending(path: "\(sourceModule.name)+JNICaches.swift")
-      ]
-    }
-
     // If the module uses 'Data' type, the thunk file is emitted as if 'Data' is declared
     // in that module. Declare the thunk file as the output.
     // FIXME: Make this conditional.

Samples/JExtractJNISampleApp/Sources/MySwiftLibrary/Alignment.swift renamed to Samples/JExtractJNISampleApp/Sources/MySwiftLibrary/EnumAlignment.swift

@@ -13,7 +13,19 @@
 //===----------------------------------------------------------------------===//
 
 enum JNICaching {
-  static func cacheName(for enumCase: ImportedEnumCase) -> String {
+  static func cacheName(for type: ImportedNominalType) -> String {
+    cacheName(for: type.swiftNominal.name)
+  }
+
+  static func cacheName(for type: SwiftNominalType) -> String {
+    cacheName(for: type.nominalTypeDecl.name)
+  }
+
+  private static func cacheName(for name: String) -> String {
+    "_JNI_\(name)"
+  }
+
+  static func cacheMemberName(for enumCase: ImportedEnumCase) -> String {
     "\(enumCase.enumType.nominalTypeDecl.name.firstCharacterLowercased)\(enumCase.name.firstCharacterUppercased)Cache"
   }
 }

Samples/JExtractJNISampleApp/Sources/MySwiftLibrary/Vehicle.swift renamed to Samples/JExtractJNISampleApp/Sources/MySwiftLibrary/EnumVehicle.swift

@@ -259,31 +259,14 @@ extension JNISwift2JavaGenerator {
 
       // Print record
       printer.printBraceBlock("public record \(caseName)(\(members.joined(separator: ", "))) implements Case") { printer in
-        printer.printBraceBlock("static class $JNI") { printer in
-          printer.print("private static native void $nativeInit();")
-        }
-
-        // Used to ensure static initializer has been calling to trigger caching.
-        printer.print("static void $ensureInitialized() {}")
-
-        printer.printBraceBlock("static") { printer in
-          printer.print("$JNI.$nativeInit();")
-        }
-
         let nativeParameters = zip(translatedCase.translatedValues, translatedCase.parameterConversions).flatMap { value, conversion in
           ["\(conversion.native.javaType) \(value.parameter.name)"]
         }
 
         printer.print("record $NativeParameters(\(nativeParameters.joined(separator: ", "))) {}")
       }
 
-      self.printJavaBindingWrapperMethod(
-        &printer,
-        translatedCase.getAsCaseFunction,
-        prefix: { printer in
-          printer.print("\(caseName).$ensureInitialized();")
-        }
-      )
+      self.printJavaBindingWrapperMethod(&printer, translatedCase.getAsCaseFunction)
       printer.println()
     }
   }
@@ -355,8 +338,7 @@ extension JNISwift2JavaGenerator {
   private func printJavaBindingWrapperMethod(
     _ printer: inout CodePrinter,
     _ translatedDecl: TranslatedFunctionDecl,
-    importedFunc: ImportedFunc? = nil,
-    prefix: (inout CodePrinter) -> Void = { _ in }
+    importedFunc: ImportedFunc? = nil
   ) {
     var modifiers = ["public"]
     if translatedDecl.isStatic {
@@ -402,7 +384,6 @@ extension JNISwift2JavaGenerator {
     printer.printBraceBlock(
       "\(annotationsStr)\(modifiers.joined(separator: " ")) \(resultType) \(translatedDecl.name)(\(parameters.joined(separator: ", ")))\(throwsClause)"
     ) { printer in
-      prefix(&printer)
       printDowncall(&printer, translatedDecl)
     }
 

Sources/JExtractSwiftLib/JNI/JNICaching.swift

@@ -56,8 +56,6 @@ extension JNISwift2JavaGenerator {
         self.expectedOutputSwiftFiles.remove(moduleFilename)
       }
 
-      try self.writeJNICacheSource(&printer)
-
       for (_, ty) in self.analysis.importedTypes.sorted(by: { (lhs, rhs) in lhs.key < rhs.key }) {
         let fileNameBase = "\(ty.swiftNominal.qualifiedName)+SwiftJava"
         let filename = "\(fileNameBase).swift"
@@ -82,38 +80,13 @@ extension JNISwift2JavaGenerator {
     }
   }
 
-  private func writeJNICacheSource(_ printer: inout CodePrinter) throws {
-    printer.print("import JavaKit")
-    printer.println()
-    printer.printBraceBlock("enum JNICaches") { printer in
-      let enumCases = self.analysis.importedTypes.values.filter { $0.swiftNominal.kind == .enum }.flatMap(\.cases)
-      for enumCase in enumCases {
-        printer.print("static var \(JNICaching.cacheName(for: enumCase)): _JNICache!")
-      }
-      printer.println()
-      printer.printBraceBlock("static func cleanup()") { printer in
-        for enumCase in enumCases {
-          printer.print("JNICaches.\(JNICaching.cacheName(for: enumCase)) = nil")
-        }
+  private func printJNICache(_ printer: inout CodePrinter, _ type: ImportedNominalType) {
+    printer.printBraceBlock("enum \(JNICaching.cacheName(for: type))") { printer in
+      for enumCase in type.cases {
+        guard let translatedCase = translatedEnumCase(for: enumCase) else { continue }
+        printer.print("static let \(JNICaching.cacheMemberName(for: enumCase)) = \(renderEnumCaseCacheInit(translatedCase))")
       }
     }
-
-    printer.println()
-    printer.print(#"@_cdecl("JNI_OnUnload")"#)
-    printer.printBraceBlock("func JNI_OnUnload(javaVM: UnsafeMutablePointer<JavaVM?>!, reserved: UnsafeMutableRawPointer!)") { printer in
-      printer.print("JNICaches.cleanup()")
-    }
-
-    let fileName = "\(self.swiftModuleName)+JNICaches.swift"
-
-    if let outputFile = try printer.writeContents(
-      outputDirectory: self.swiftOutputDirectory,
-      javaPackagePath: nil,
-      filename: fileName
-    ) {
-      print("[swift-java] Generated: \(fileName.bold) (at \(outputFile))")
-      self.expectedOutputSwiftFiles.remove(fileName)
-    }
   }
 
   private func printGlobalSwiftThunkSources(_ printer: inout CodePrinter) throws {
@@ -133,6 +106,9 @@ extension JNISwift2JavaGenerator {
   private func printNominalTypeThunks(_ printer: inout CodePrinter, _ type: ImportedNominalType) throws {
     printHeader(&printer)
 
+    printJNICache(&printer, type)
+    printer.println()
+
     for initializer in type.initializers {
       printSwiftFunctionThunk(&printer, initializer)
       printer.println()
@@ -192,30 +168,18 @@ extension JNISwift2JavaGenerator {
     printSwiftFunctionThunk(&printer, enumCase.caseFunction)
     printer.println()
 
-    // Print enum case native init
-    printEnumNativeInit(&printer, translatedCase)
-
     // Print getAsCase method
     if !translatedCase.translatedValues.isEmpty {
       printEnumGetAsCaseThunk(&printer, translatedCase)
     }
   }
 
-  private func printEnumNativeInit(_ printer: inout CodePrinter, _ enumCase: TranslatedEnumCase) {
-    printCDecl(
-      &printer,
-      javaMethodName: "$nativeInit",
-      parentName: "\(enumCase.original.enumType.nominalTypeDecl.name)$\(enumCase.name)$$JNI",
-      parameters: [],
-      resultType: .void
-    ) { printer in
-      // Setup caching
+  private func renderEnumCaseCacheInit(_ enumCase: TranslatedEnumCase) -> String {
       let nativeParametersClassName = "\(javaPackagePath)/\(enumCase.enumName)$\(enumCase.name)$$NativeParameters"
       let methodSignature = MethodSignature(resultType: .void, parameterTypes: enumCase.parameterConversions.map(\.native.javaType))
       let methods = #"[.init(name: "<init>", signature: "\#(methodSignature.mangledName)")]"#
 
-      printer.print(#"JNICaches.\#(JNICaching.cacheName(for: enumCase.original)) = _JNICache(environment: environment, className: "\#(nativeParametersClassName)", methods: \#(methods))"#)
-    }
+      return #"_JNIMethodIDCache(environment: try! JavaVirtualMachine.shared().environment(), className: "\#(nativeParametersClassName)", methods: \#(methods))"#
   }
 
   private func printEnumGetAsCaseThunk(
@@ -237,9 +201,9 @@ extension JNISwift2JavaGenerator {
         guard case .\(enumCase.original.name)(\(caseNamesWithLet.joined(separator: ", "))) = \(selfPointer).pointee else {
           fatalError("Expected enum case '\(enumCase.original.name)', but was '\\(\(selfPointer).pointee)'!")
         }
-        let cache$ = JNICaches.\(JNICaching.cacheName(for: enumCase.original))!
+        let cache$ = \(JNICaching.cacheName(for: enumCase.original.enumType)).\(JNICaching.cacheMemberName(for: enumCase.original))
         let class$ = cache$.javaClass
-        let method$ = _JNICache.Method(name: "<init>", signature: "\(methodSignature.mangledName)")
+        let method$ = _JNIMethodIDCache.Method(name: "<init>", signature: "\(methodSignature.mangledName)")
         let constructorID$ = cache$[method$]
         """
       )

Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift

@@ -72,7 +72,6 @@ package class JNISwift2JavaGenerator: Swift2JavaGenerator {
         return String(filePathPart.replacing(".swift", with: "+SwiftJava.swift"))
       })
       self.expectedOutputSwiftFiles.insert("\(translator.swiftModuleName)Module+SwiftJava.swift")
-      self.expectedOutputSwiftFiles.insert("\(translator.swiftModuleName)+JNICaches.swift")
 
       // FIXME: Can we avoid this?
       self.expectedOutputSwiftFiles.insert("Data+SwiftJava.swift")

Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+SwiftThunkPrinting.swift

@@ -16,12 +16,7 @@
 ///
 /// This type is used internally in by the outputted JExtract wrappers
 /// to improve performance of any JNI lookups.
-///
-/// - Warning: This type is inherently thread-unsafe.
-///            We assume that there exists **at most** one of these
-///            per Java `class`, and it must only be initialized and modified in the
-///            static initializer of that Java class.
-public final class _JNICache: @unchecked Sendable {
+public final class _JNIMethodIDCache: Sendable {
   public struct Method: Hashable {
     public let name: String
     public let signature: String
@@ -32,9 +27,8 @@ public final class _JNICache: @unchecked Sendable {
     }
   }
 
-  var _class: jclass?
-  let environment: JNIEnvironment
-  let methods: [Method: jmethodID]
+  nonisolated(unsafe) let _class: jclass?
+  nonisolated(unsafe) let methods: [Method: jmethodID]
 
   public var javaClass: jclass {
     self._class!
@@ -44,7 +38,6 @@ public final class _JNICache: @unchecked Sendable {
     guard let clazz = environment.interface.FindClass(environment, className) else {
       fatalError("Class \(className) could not be found!")
     }
-    self.environment = environment
     self._class = environment.interface.NewGlobalRef(environment, clazz)!
     self.methods = methods.reduce(into: [:]) { (result, method) in
       if let methodID = environment.interface.GetMethodID(environment, clazz, method.name, method.signature) {
@@ -60,12 +53,7 @@ public final class _JNICache: @unchecked Sendable {
     methods[method]
   }
 
-  func cleanup() {
+  public func cleanup(environment: UnsafeMutablePointer<JNIEnv?>!) {
     environment.interface.DeleteGlobalRef(environment, self._class)
   }
-
-  deinit {
-    cleanup()
-    self._class = nil
-  }
 }

Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator.swift

@@ -194,7 +194,7 @@ try (var arena = SwiftArena.ofConfined()) {
 }
 ```
 
-#### Switching
+#### Switching and pattern matching
 
 If you only need to switch on the case and not access any associated values,
 you can use the `getDiscriminator()` method:
@@ -246,7 +246,7 @@ if (case instanceof Vehicle.Bicycle) {
 }
 ```
 
-#### RawRepresentable
+#### RawRepresentable enums
 
 JExtract also supports extracting enums that conform to `RawRepresentable`
 by giving access to an optional initializer and the `rawValue` variable.