Skip to content

Commit 96d37c4

Browse files
madsodgaardmadsodgaard
committed
PR feedback
1 parent e97220b commit 96d37c4

File tree

13 files changed

+60
-92
lines changed

13 files changed

+60
-92
lines changed

‎Plugins/JExtractSwiftPlugin/JExtractSwiftPlugin.swift‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,19 @@ struct JExtractSwiftBuildToolPlugin: SwiftJavaPluginProtocol, BuildToolPlugin {
128128

129129
var jextractOutputFiles = outputSwiftFiles
130130

131-
let runJavaCallbacksPhases = configuration?.enableJavaCallbacks ?? false && configuration?.effectiveMode == .jni
131+
// If the developer has enabled java callbacks in the configuration (default is false)
132+
// and we are running in JNI mode, we will run additional phases in this build plugin
133+
// to generate Swift wrappers using wrap-java that can be used to callback to Java.
134+
let shouldRunJavaCallbacksPhases = if let configuration, configuration.enableJavaCallbacks ?? false, configuration.effectiveMode == .jni {
135+
true
136+
} else {
137+
false
138+
}
132139

133140
// Extract list of all sources
134141
let javaSourcesListFileName = "jextract-generated-sources.txt"
135142
let javaSourcesFile = outputJavaDirectory.appending(path: javaSourcesListFileName)
136-
if runJavaCallbacksPhases {
143+
if shouldRunJavaCallbacksPhases {
137144
arguments += [
138145
"--generated-java-sources-list-file-output", javaSourcesListFileName
139146
]
@@ -151,20 +158,19 @@ struct JExtractSwiftBuildToolPlugin: SwiftJavaPluginProtocol, BuildToolPlugin {
151158
]
152159

153160
// If we do not need Java callbacks, we can skip the remaining steps.
154-
guard runJavaCallbacksPhases else {
161+
guard shouldRunJavaCallbacksPhases else {
155162
return commands
156163
}
157164

158165
// The URL of the compiled Java sources
159-
let javaClassFileURL = context.pluginWorkDirectoryURL
166+
let javaCompiledClassesURL = context.pluginWorkDirectoryURL
160167
.appending(path: "compiled-java-output")
161168

162169
// Build SwiftKitCore and get the classpath
163170
// as the jextracted sources will depend on that
164171

165172
guard let swiftJavaDirectory = findSwiftJavaDirectory(for: target) else {
166-
// FIXME: Error
167-
fatalError()
173+
fatalError("Unable to find the path to the swift-java sources, please file an issue.")
168174
}
169175
log("Found swift-java at \(swiftJavaDirectory)")
170176

@@ -204,12 +210,12 @@ struct JExtractSwiftBuildToolPlugin: SwiftJavaPluginProtocol, BuildToolPlugin {
204210
.appending(path: self.javacName),
205211
arguments: [
206212
"@\(javaSourcesFile.path(percentEncoded: false))",
207-
"-d", javaClassFileURL.path(percentEncoded: false),
213+
"-d", javaCompiledClassesURL.path(percentEncoded: false),
208214
"-parameters",
209215
"-classpath", swiftKitCoreClassPath.path(percentEncoded: false)
210216
],
211217
inputFiles: [javaSourcesFile, swiftKitCoreClassPath],
212-
outputFiles: [javaClassFileURL]
218+
outputFiles: [javaCompiledClassesURL]
213219
)
214220
]
215221

@@ -223,11 +229,11 @@ struct JExtractSwiftBuildToolPlugin: SwiftJavaPluginProtocol, BuildToolPlugin {
223229
arguments: [
224230
"configure",
225231
"--output-directory", context.pluginWorkDirectoryURL.path(percentEncoded: false),
226-
"--cp", javaClassFileURL.path(percentEncoded: false),
232+
"--cp", javaCompiledClassesURL.path(percentEncoded: false),
227233
"--swift-module", sourceModule.name,
228234
"--swift-type-prefix", "Java"
229235
],
230-
inputFiles: [javaClassFileURL],
236+
inputFiles: [javaCompiledClassesURL],
231237
outputFiles: [swiftJavaConfigURL]
232238
)
233239
]

‎Samples/SwiftJavaExtractJNISampleApp/src/test/java/com/example/swift/NestedTypesTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,12 @@ void testClassesAndStructs() {
3434
}
3535
}
3636

37-
/*@Test
37+
@Test
3838
void testStructInEnum() {
3939
try (var arena = SwiftArena.ofConfined()) {
4040
var obj = NestedEnum.one(NestedEnum.OneStruct.init(arena), arena);
4141
var one = obj.getAsOne(arena);
4242
assertTrue(one.isPresent());
4343
}
44-
}*/
44+
}
4545
}

‎Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+InterfaceWrapperGeneration.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ extension JNISwift2JavaGenerator {
8787
}
8888

8989
let variables = try Dictionary(grouping: type.variables, by: { $0.swiftDecl.id }).map { (id, funcs) in
90-
assert(funcs.count > 0 && funcs.count <= 2, "Variables must contain a getter and optionally a setter")
90+
precondition(funcs.count > 0 && funcs.count <= 2, "Variables must contain a getter and optionally a setter")
9191
guard let getter = funcs.first(where: { $0.apiKind == .getter }) else {
92-
fatalError("")
92+
fatalError("Getter not found for variable with imported funcs: \(funcs)")
9393
}
9494
let setter = funcs.first(where: { $0.apiKind == .setter })
9595

‎Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift‎

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import Foundation
1616
import JavaTypes
17+
import OrderedCollections
1718

1819
// MARK: Defaults
1920

@@ -41,7 +42,7 @@ extension JNISwift2JavaGenerator {
4142
package func writeExportedJavaSources(_ printer: inout CodePrinter) throws {
4243
let importedTypes = analysis.importedTypes.sorted(by: { (lhs, rhs) in lhs.key < rhs.key })
4344

44-
var exportedFileNames: Set<String> = []
45+
var exportedFileNames: OrderedSet<String> = []
4546

4647
// Each parent type goes into its own file
4748
// any nested types are printed inside the body as `static class`
@@ -55,7 +56,7 @@ extension JNISwift2JavaGenerator {
5556
javaPackagePath: javaPackagePath,
5657
filename: filename
5758
) {
58-
exportedFileNames.insert(outputFile.path(percentEncoded: false))
59+
exportedFileNames.append(outputFile.path(percentEncoded: false))
5960
logger.info("[swift-java] Generated: \(ty.swiftNominal.name.bold).java (at \(outputFile))")
6061
}
6162
}
@@ -69,7 +70,7 @@ extension JNISwift2JavaGenerator {
6970
javaPackagePath: javaPackagePath,
7071
filename: filename
7172
) {
72-
exportedFileNames.insert(outputFile.path(percentEncoded: false))
73+
exportedFileNames.append(outputFile.path(percentEncoded: false))
7374
logger.info("[swift-java] Generated: \(self.swiftModuleName).java (at \(outputFile))")
7475
}
7576

@@ -143,17 +144,17 @@ extension JNISwift2JavaGenerator {
143144

144145
printer.printBraceBlock("public interface \(decl.swiftNominal.name)\(extendsString)") { printer in
145146
for initializer in decl.initializers {
146-
printFunctionDowncallMethods(&printer, initializer, signaturesOnly: true, ignoresArenas: true)
147+
printFunctionDowncallMethods(&printer, initializer, skipMethodBody: true, skipArenas: true)
147148
printer.println()
148149
}
149150

150151
for method in decl.methods {
151-
printFunctionDowncallMethods(&printer, method, signaturesOnly: true, ignoresArenas: true)
152+
printFunctionDowncallMethods(&printer, method, skipMethodBody: true, skipArenas: true)
152153
printer.println()
153154
}
154155

155156
for variable in decl.variables {
156-
printFunctionDowncallMethods(&printer, variable, signaturesOnly: true, ignoresArenas: true)
157+
printFunctionDowncallMethods(&printer, variable, skipMethodBody: true, skipArenas: true)
157158
printer.println()
158159
}
159160
}
@@ -393,16 +394,16 @@ extension JNISwift2JavaGenerator {
393394
printer.print("record _NativeParameters(\(nativeParameters.joined(separator: ", "))) {}")
394395
}
395396

396-
self.printJavaBindingWrapperMethod(&printer, translatedCase.getAsCaseFunction, signaturesOnly: false, ignoresArenas: false)
397+
self.printJavaBindingWrapperMethod(&printer, translatedCase.getAsCaseFunction, skipMethodBody: false, skipArenas: false)
397398
printer.println()
398399
}
399400
}
400401

401402
private func printFunctionDowncallMethods(
402403
_ printer: inout CodePrinter,
403404
_ decl: ImportedFunc,
404-
signaturesOnly: Bool = false,
405-
ignoresArenas: Bool = false
405+
skipMethodBody: Bool = false,
406+
skipArenas: Bool = false
406407
) {
407408
guard translatedDecl(for: decl) != nil else {
408409
// Failed to translate. Skip.
@@ -413,7 +414,7 @@ extension JNISwift2JavaGenerator {
413414

414415
printJavaBindingWrapperHelperClass(&printer, decl)
415416

416-
printJavaBindingWrapperMethod(&printer, decl, signaturesOnly: signaturesOnly, ignoresArenas: ignoresArenas)
417+
printJavaBindingWrapperMethod(&printer, decl, skipMethodBody: skipMethodBody, skipArenas: skipArenas)
417418
}
418419

419420
/// Print the helper type container for a user-facing Java API.
@@ -459,21 +460,21 @@ extension JNISwift2JavaGenerator {
459460
private func printJavaBindingWrapperMethod(
460461
_ printer: inout CodePrinter,
461462
_ decl: ImportedFunc,
462-
signaturesOnly: Bool,
463-
ignoresArenas: Bool
463+
skipMethodBody: Bool,
464+
skipArenas: Bool
464465
) {
465466
guard let translatedDecl = translatedDecl(for: decl) else {
466467
fatalError("Decl was not translated, \(decl)")
467468
}
468-
printJavaBindingWrapperMethod(&printer, translatedDecl, importedFunc: decl, signaturesOnly: signaturesOnly, ignoresArenas: ignoresArenas)
469+
printJavaBindingWrapperMethod(&printer, translatedDecl, importedFunc: decl, skipMethodBody: skipMethodBody, skipArenas: skipArenas)
469470
}
470471

471472
private func printJavaBindingWrapperMethod(
472473
_ printer: inout CodePrinter,
473474
_ translatedDecl: TranslatedFunctionDecl,
474475
importedFunc: ImportedFunc? = nil,
475-
signaturesOnly: Bool,
476-
ignoresArenas: Bool
476+
skipMethodBody: Bool,
477+
skipArenas: Bool
477478
) {
478479
var modifiers = ["public"]
479480
if translatedDecl.isStatic {
@@ -521,14 +522,14 @@ extension JNISwift2JavaGenerator {
521522
printer.println()
522523
}
523524

524-
if translatedSignature.requiresSwiftArena, !ignoresArenas {
525+
if translatedSignature.requiresSwiftArena, !skipArenas {
525526
parameters.append("SwiftArena swiftArena$")
526527
}
527528
if let importedFunc {
528529
printDeclDocumentation(&printer, importedFunc)
529530
}
530531
let signature = "\(annotationsStr)\(modifiers.joined(separator: " ")) \(resultType) \(translatedDecl.name)(\(parameters.joined(separator: ", ")))\(throwsClause)"
531-
if signaturesOnly {
532+
if skipMethodBody {
532533
printer.print("\(signature);")
533534
} else {
534535
printer.printBraceBlock(signature) { printer in

‎Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaTranslation.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,6 +1326,7 @@ extension JNISwift2JavaGenerator {
13261326
/// a java class.
13271327
case wrappedJavaClassTranslationNotProvided(SwiftType)
13281328

1329+
// FIXME: Remove once we support protocol variables
13291330
case protocolVariablesNotSupported
13301331
}
13311332
}

‎Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+SwiftThunkPrinting.swift‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,6 @@ extension JNISwift2JavaGenerator {
113113
guard let translatedCase = translatedEnumCase(for: enumCase) else { continue }
114114
printer.print("static let \(JNICaching.cacheMemberName(for: enumCase)) = \(renderEnumCaseCacheInit(translatedCase))")
115115
}
116-
117-
if type.swiftNominal.kind == .protocol {
118-
// let methodSignatures = type.methods.map {
119-
// MethodSignature(resultType: <#T##JavaType#>, parameterTypes: <#T##[JavaType]#>)
120-
// }
121-
// printer.print("static let cache = \(renderJNICacheInit(className: type.qualifiedName, methods: []))")
122-
}
123116
}
124117
}
125118

‎Sources/JExtractSwiftLib/SwiftTypes/SwiftKnownTypeDecls.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ enum SwiftKnownTypeDeclKind: String, Hashable {
6666
}
6767
}
6868

69+
/// Indicates whether this known type is translated by `wrap-java`
70+
/// into the same type as `jextract`.
71+
///
72+
/// This means we do not have to perform any mapping when passing
73+
/// this type between jextract and wrap-java
6974
var isDirectlyTranslatedToWrapJava: Bool {
7075
switch self {
7176
case .bool, .int, .uint, .int8, .uint8, .int16, .uint16, .int32, .uint32, .int64, .uint64, .float, .double, .string, .void:

‎Sources/JavaStdlib/JavaLangReflect/Constructor+Utilities.swift‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,6 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
extension Constructor {
16-
/// Whether this is a 'public' constructor.
17-
public var isPublic: Bool {
18-
return (getModifiers() & 1) != 0
19-
}
20-
2116
/// Whether this is a 'native' constructor.
2217
public var isNative: Bool {
2318
return (getModifiers() & 256) != 0

‎Sources/JavaStdlib/JavaLangReflect/JavaClass+Utilities.swift‎ renamed to ‎Sources/JavaStdlib/JavaLangReflect/HasJavaModifiers.swift‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
//
33
// This source file is part of the Swift.org open source project
44
//
5-
// Copyright (c) 2024 Apple Inc. and the Swift.org project authors
5+
// Copyright (c) 2025 Apple Inc. and the Swift.org project authors
66
// Licensed under Apache License v2.0
77
//
88
// See LICENSE.txt for license information
@@ -14,26 +14,34 @@
1414

1515
import SwiftJava
1616

17-
extension JavaClass {
18-
/// Whether this is a 'public' class.
17+
public protocol HasJavaModifiers {
18+
func getModifiers() -> Int32
19+
}
20+
21+
extension HasJavaModifiers {
22+
/// Whether the modifiers contain 'public'.
1923
public var isPublic: Bool {
2024
return (getModifiers() & 0x00000001) != 0
2125
}
2226

23-
/// Whether this is a 'private' class.
27+
/// Whether the modifiers contain 'private'.
2428
public var isPrivate: Bool {
2529
return (getModifiers() & 0x00000002) != 0
2630
}
2731

28-
/// Whether this is a 'protected' class.
32+
/// Whether the modifiers contain 'protected'.
2933
public var isProtected: Bool {
3034
return (getModifiers() & 0x00000004) != 0
3135
}
3236

33-
/// Whether this is a 'package' method.
37+
/// Whether the modifiers is equivelant to 'package'..
3438
///
3539
/// The "default" access level in Java is 'package', it is signified by lack of a different access modifier.
3640
public var isPackage: Bool {
3741
return !isPublic && !isPrivate && !isProtected
3842
}
3943
}
44+
45+
extension Constructor: HasJavaModifiers {}
46+
extension JavaClass: HasJavaModifiers {}
47+
extension Method: HasJavaModifiers {}

‎Sources/JavaStdlib/JavaLangReflect/Method+Utilities.swift‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,6 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
extension Method {
16-
17-
/// Whether this is a 'public' method.
18-
public var isPublic: Bool {
19-
return (getModifiers() & 0x00000001) != 0
20-
}
21-
22-
/// Whether this is a 'private' method.
23-
public var isPrivate: Bool {
24-
return (getModifiers() & 0x00000002) != 0
25-
}
26-
27-
/// Whether this is a 'protected' method.
28-
public var isProtected: Bool {
29-
return (getModifiers() & 0x00000004) != 0
30-
}
31-
32-
/// Whether this is a 'package' method.
33-
///
34-
/// The "default" access level in Java is 'package', it is signified by lack of a different access modifier.
35-
public var isPackage: Bool {
36-
return !isPublic && !isPrivate && !isProtected
37-
}
3816

3917
/// Whether this is a 'static' method.
4018
public var isStatic: Bool {

0 commit comments

Comments
 (0)