From af341a074589520d19e079187e292b948ceb66a7 Mon Sep 17 00:00:00 2001 From: Konrad 'ktoso' Malawski Date: Thu, 14 Aug 2025 14:09:18 +0900 Subject: [PATCH 1/2] jextract: hide memory location wrapping constructor, give it proper name The reason here is that it's too easy to accidentally autocomplete into new Thing(bla) which can often be incorrect because we probably intended to call init on the Thing. This popped up with Data a lot, because it is so easy to wrongly use new Data rather than Data.init which performs the proper initialization --- .../JExtractSwiftLib/FFM/ConversionStep.swift | 2 +- ...wift2JavaGenerator+JavaBindingsPrinting.swift | 13 +++++++++++-- .../FFMSwift2JavaGenerator+JavaTranslation.swift | 5 ++++- .../FFM/FFMSwift2JavaGenerator.swift | 15 ++++++++++++++- ...wift2JavaGenerator+JavaBindingsPrinting.swift | 15 ++++++++++++++- .../JNISwift2JavaGenerator+JavaTranslation.swift | 13 ++++++++++--- Sources/JavaKit/Helpers/_JNICache.swift | 0 Tests/JExtractSwiftTests/DataImportTests.swift | 7 ++++--- Tests/JExtractSwiftTests/JNI/JNIClassTests.swift | 16 +++++++++++----- .../JNI/JNIOptionalTests.swift | 2 +- .../JExtractSwiftTests/JNI/JNIStructTests.swift | 12 +++++++++--- .../MemoryManagementModeTests.swift | 4 ++-- Tests/JExtractSwiftTests/MethodImportTests.swift | 6 +++--- 13 files changed, 84 insertions(+), 26 deletions(-) create mode 100644 Sources/JavaKit/Helpers/_JNICache.swift diff --git a/Sources/JExtractSwiftLib/FFM/ConversionStep.swift b/Sources/JExtractSwiftLib/FFM/ConversionStep.swift index 656d9dd9..f59f739d 100644 --- a/Sources/JExtractSwiftLib/FFM/ConversionStep.swift +++ b/Sources/JExtractSwiftLib/FFM/ConversionStep.swift @@ -123,7 +123,7 @@ enum ConversionStep: Equatable { // of splatting out text. let renderedArgumentList = renderedArguments.joined(separator: ", ") return "\(raw: type.description)(\(raw: renderedArgumentList))" - + case .tuplify(let elements): let renderedElements: [String] = elements.enumerated().map { (index, element) in element.asExprSyntax(placeholder: "\(placeholder)_\(index)", bodyItems: &bodyItems)!.description diff --git a/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaBindingsPrinting.swift b/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaBindingsPrinting.swift index b8749299..84a90275 100644 --- a/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaBindingsPrinting.swift +++ b/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaBindingsPrinting.swift @@ -407,6 +407,7 @@ extension FFMSwift2JavaGenerator { "arena$" } + // FIXME: use trailing$ convention let varName = outParameter.name.isEmpty ? "_result" : "_result_" + outParameter.name printer.print( @@ -463,7 +464,7 @@ extension FFMSwift2JavaGenerator.JavaConversionStep { switch self { case .placeholder, .explodedName, .constant, .readMemorySegment: return false - case .constructSwiftValue: + case .constructSwiftValue, .wrapMemoryAddressUnsafe: return true case .call(let inner, _, _), .cast(let inner, _), .construct(let inner, _), @@ -482,7 +483,11 @@ extension FFMSwift2JavaGenerator.JavaConversionStep { return false case .readMemorySegment: return true - case .cast(let inner, _), .construct(let inner, _), .constructSwiftValue(let inner, _), .swiftValueSelfSegment(let inner): + case .cast(let inner, _), + .construct(let inner, _), + .constructSwiftValue(let inner, _), + .swiftValueSelfSegment(let inner), + .wrapMemoryAddressUnsafe(let inner, _): return inner.requiresSwiftArena case .call(let inner, _, let withArena): return withArena || inner.requiresTemporaryArena @@ -522,6 +527,10 @@ extension FFMSwift2JavaGenerator.JavaConversionStep { let inner = inner.render(&printer, placeholder) return "new \(javaType.className!)(\(inner), swiftArena$)" + case .wrapMemoryAddressUnsafe(let inner, let javaType): + let inner = inner.render(&printer, placeholder) + return "\(javaType.className!).wrapMemoryAddressUnsafe(\(inner), swiftArena$)" + case .construct(let inner, let javaType): let inner = inner.render(&printer, placeholder) return "new \(javaType)(\(inner))" diff --git a/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaTranslation.swift b/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaTranslation.swift index f8b2e7e8..06a4d171 100644 --- a/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaTranslation.swift +++ b/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaTranslation.swift @@ -684,7 +684,7 @@ extension FFMSwift2JavaGenerator { outParameters: [ JavaParameter(name: "", type: javaType) ], - conversion: .constructSwiftValue(.placeholder, javaType) + conversion: .wrapMemoryAddressUnsafe(.placeholder, javaType) ) case .tuple: @@ -732,6 +732,9 @@ extension FFMSwift2JavaGenerator { // Call 'new \(Type)(\(placeholder), swiftArena$)'. indirect case constructSwiftValue(JavaConversionStep, JavaType) + /// Call the `MyType.wrapMemoryAddressUnsafe` in order to wrap a memory address using the Java binding type + indirect case wrapMemoryAddressUnsafe(JavaConversionStep, JavaType) + // Construct the type using the placeholder as arguments. indirect case construct(JavaConversionStep, JavaType) diff --git a/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator.swift b/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator.swift index 3e60db14..c66b0029 100644 --- a/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator.swift +++ b/Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator.swift @@ -216,9 +216,22 @@ extension FFMSwift2JavaGenerator { printer.print( """ - public \(decl.swiftNominal.name)(MemorySegment segment, AllocatingSwiftArena arena) { + private \(decl.swiftNominal.name)(MemorySegment segment, AllocatingSwiftArena arena) { super(segment, arena); } + + /** + * Assume that the passed {@code MemorySegment} represents a memory address of a {@link \(decl.swiftNominal.name)}. + *

+ * Warnings: + *

+ */ + public static \(decl.swiftNominal.name) wrapMemoryAddressUnsafe(MemorySegment selfPointer, AllocatingSwiftArena swiftArena) { + return new \(decl.swiftNominal.name)(selfPointer, swiftArena); + } """ ) diff --git a/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift b/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift index 0920a89b..d3452cef 100644 --- a/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift +++ b/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift @@ -117,9 +117,22 @@ extension JNISwift2JavaGenerator { printer.print( """ - public \(decl.swiftNominal.name)(long selfPointer, SwiftArena swiftArena) { + private \(decl.swiftNominal.name)(long selfPointer, SwiftArena swiftArena) { super(selfPointer, swiftArena); } + + /** + * Assume that the passed {@code long} represents a memory address of a {@link \(decl.swiftNominal.name)}. + *

+ * Warnings: + *

+ */ + public static \(decl.swiftNominal.name) wrapMemoryAddressUnsafe(long selfPointer, SwiftArena swiftArena) { + return new \(decl.swiftNominal.name)(selfPointer, swiftArena); + } """ ) diff --git a/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaTranslation.swift b/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaTranslation.swift index 0db77ece..64ae23b7 100644 --- a/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaTranslation.swift +++ b/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaTranslation.swift @@ -383,7 +383,7 @@ extension JNISwift2JavaGenerator { javaType: javaType, annotations: resultAnnotations, outParameters: [], - conversion: .constructSwiftValue(.placeholder, javaType) + conversion: .wrapMemoryAddressUnsafe(.placeholder, javaType) ) case .tuple([]): @@ -465,7 +465,7 @@ extension JNISwift2JavaGenerator { discriminatorName: "result_discriminator$", optionalClass: "Optional", javaType: .long, - toValue: .constructSwiftValue(.placeholder, .class(package: nil, name: nominalTypeName)) + toValue: .wrapMemoryAddressUnsafe(.placeholder, .class(package: nil, name: nominalTypeName)) ) ) @@ -582,6 +582,9 @@ extension JNISwift2JavaGenerator { /// Call `new \(Type)(\(placeholder), swiftArena$)` indirect case constructSwiftValue(JavaNativeConversionStep, JavaType) + /// Call the `MyType.wrapMemoryAddressUnsafe` in order to wrap a memory address using the Java binding type + indirect case wrapMemoryAddressUnsafe(JavaNativeConversionStep, JavaType) + indirect case call(JavaNativeConversionStep, function: String) indirect case method(JavaNativeConversionStep, function: String, arguments: [JavaNativeConversionStep] = []) @@ -641,6 +644,10 @@ extension JNISwift2JavaGenerator { case .constructSwiftValue(let inner, let javaType): let inner = inner.render(&printer, placeholder) return "new \(javaType.className!)(\(inner), swiftArena$)" + + case .wrapMemoryAddressUnsafe(let inner, let javaType): + let inner = inner.render(&printer, placeholder) + return "\(javaType.className!).wrapMemoryAddressUnsafe(\(inner), swiftArena$)" case .call(let inner, let function): let inner = inner.render(&printer, placeholder) @@ -705,7 +712,7 @@ extension JNISwift2JavaGenerator { case .placeholder, .constant, .isOptionalPresent: return false - case .constructSwiftValue: + case .constructSwiftValue, .wrapMemoryAddressUnsafe: return true case .valueMemoryAddress(let inner): diff --git a/Sources/JavaKit/Helpers/_JNICache.swift b/Sources/JavaKit/Helpers/_JNICache.swift new file mode 100644 index 00000000..e69de29b diff --git a/Tests/JExtractSwiftTests/DataImportTests.swift b/Tests/JExtractSwiftTests/DataImportTests.swift index 7416779d..cf4ed387 100644 --- a/Tests/JExtractSwiftTests/DataImportTests.swift +++ b/Tests/JExtractSwiftTests/DataImportTests.swift @@ -86,7 +86,7 @@ final class DataImportTests { ] ) } - + @Test("Import Data: JavaBindings") func data_javaBindings() throws { @@ -167,7 +167,7 @@ final class DataImportTests { public static Data returnData(AllocatingSwiftArena swiftArena$) { MemorySegment _result = swiftArena$.allocate(Data.$LAYOUT); swiftjava_SwiftModule_returnData.call(_result); - return new Data(_result, swiftArena$); + return Data.wrapMemoryAddressUnsafe(_result, swiftArena$); } """, @@ -210,7 +210,7 @@ final class DataImportTests { public static Data init(java.lang.foreign.MemorySegment bytes, long count, AllocatingSwiftArena swiftArena$) { MemorySegment _result = swiftArena$.allocate(Data.$LAYOUT); swiftjava_SwiftModule_Data_init_bytes_count.call(bytes, count, _result); - return new Data(_result, swiftArena$); + return Data.wrapMemoryAddressUnsafe(_result, swiftArena$); } """, @@ -408,4 +408,5 @@ final class DataImportTests { ] ) } + } diff --git a/Tests/JExtractSwiftTests/JNI/JNIClassTests.swift b/Tests/JExtractSwiftTests/JNI/JNIClassTests.swift index 5b015f89..1e33efcb 100644 --- a/Tests/JExtractSwiftTests/JNI/JNIClassTests.swift +++ b/Tests/JExtractSwiftTests/JNI/JNIClassTests.swift @@ -66,11 +66,17 @@ struct JNIClassTests { System.loadLibrary(LIB_NAME); return true; } - - public MyClass(long selfPointer, SwiftArena swiftArena) { + """, + """ + private MyClass(long selfPointer, SwiftArena swiftArena) { super(selfPointer, swiftArena); } """, + """ + public static MyClass wrapMemoryAddressUnsafe(long selfPointer, SwiftArena swiftArena) { + return new MyClass(selfPointer, swiftArena); + } + """ ]) try assertOutput( input: source, @@ -164,7 +170,7 @@ struct JNIClassTests { * } */ public static MyClass init(long x, long y, SwiftArena swiftArena$) { - return new MyClass(MyClass.$init(x, y), swiftArena$); + return MyClass.wrapMemoryAddressUnsafe(MyClass.$init(x, y), swiftArena$); } """, """ @@ -175,7 +181,7 @@ struct JNIClassTests { * } */ public static MyClass init(SwiftArena swiftArena$) { - return new MyClass(MyClass.$init(), swiftArena$); + return MyClass.wrapMemoryAddressUnsafe(MyClass.$init(), swiftArena$); } """, """ @@ -309,7 +315,7 @@ struct JNIClassTests { * } */ public MyClass copy(SwiftArena swiftArena$) { - return new MyClass(MyClass.$copy(this.$memoryAddress()), swiftArena$); + return MyClass.wrapMemoryAddressUnsafe(MyClass.$copy(this.$memoryAddress()), swiftArena$); } """, """ diff --git a/Tests/JExtractSwiftTests/JNI/JNIOptionalTests.swift b/Tests/JExtractSwiftTests/JNI/JNIOptionalTests.swift index cd04660b..f9c70ba4 100644 --- a/Tests/JExtractSwiftTests/JNI/JNIOptionalTests.swift +++ b/Tests/JExtractSwiftTests/JNI/JNIOptionalTests.swift @@ -158,7 +158,7 @@ struct JNIOptionalTests { public static Optional optionalClass(Optional arg, SwiftArena swiftArena$) { byte[] result_discriminator$ = new byte[1]; long result$ = SwiftModule.$optionalClass(arg.map(MyClass::$memoryAddress).orElse(0L), result_discriminator$); - return (result_discriminator$[0] == 1) ? Optional.of(new MyClass(result$, swiftArena$)) : Optional.empty(); + return (result_discriminator$[0] == 1) ? Optional.of(MyClass.wrapMemoryAddressUnsafe(result$, swiftArena$)) : Optional.empty(); } """, """ diff --git a/Tests/JExtractSwiftTests/JNI/JNIStructTests.swift b/Tests/JExtractSwiftTests/JNI/JNIStructTests.swift index 01a2e3c0..59d7ee6b 100644 --- a/Tests/JExtractSwiftTests/JNI/JNIStructTests.swift +++ b/Tests/JExtractSwiftTests/JNI/JNIStructTests.swift @@ -56,10 +56,16 @@ struct JNIStructTests { System.loadLibrary(LIB_NAME); return true; } - - public MyStruct(long selfPointer, SwiftArena swiftArena) { + """, + """ + private MyStruct(long selfPointer, SwiftArena swiftArena) { super(selfPointer, swiftArena); } + """, + """ + public static MyStruct wrapMemoryAddressUnsafe(long selfPointer, SwiftArena swiftArena) { + return new MyStruct(selfPointer, swiftArena); + } """ ]) try assertOutput( @@ -110,7 +116,7 @@ struct JNIStructTests { * } */ public static MyStruct init(long x, long y, SwiftArena swiftArena$) { - return new MyStruct(MyStruct.$init(x, y), swiftArena$); + return MyStruct.wrapMemoryAddressUnsafe(MyStruct.$init(x, y), swiftArena$); } """, """ diff --git a/Tests/JExtractSwiftTests/MemoryManagementModeTests.swift b/Tests/JExtractSwiftTests/MemoryManagementModeTests.swift index 4edf59c2..e64d4397 100644 --- a/Tests/JExtractSwiftTests/MemoryManagementModeTests.swift +++ b/Tests/JExtractSwiftTests/MemoryManagementModeTests.swift @@ -43,7 +43,7 @@ struct MemoryManagementModeTests { * } */ public static MyClass f(SwiftArena swiftArena$) { - return new MyClass(SwiftModule.$f(), swiftArena$); + return MyClass.wrapMemoryAddressUnsafe(SwiftModule.$f(), swiftArena$); } """, ] @@ -68,7 +68,7 @@ struct MemoryManagementModeTests { """, """ public static MyClass f(SwiftArena swiftArena$) { - return new MyClass(SwiftModule.$f(), swiftArena$); + return MyClass.wrapMemoryAddressUnsafe(SwiftModule.$f(), swiftArena$); } """, ] diff --git a/Tests/JExtractSwiftTests/MethodImportTests.swift b/Tests/JExtractSwiftTests/MethodImportTests.swift index fd885f1b..3aa7d394 100644 --- a/Tests/JExtractSwiftTests/MethodImportTests.swift +++ b/Tests/JExtractSwiftTests/MethodImportTests.swift @@ -227,7 +227,7 @@ final class MethodImportTests { public static MySwiftClass globalReturnClass(AllocatingSwiftArena swiftArena$) { MemorySegment _result = swiftArena$.allocate(MySwiftClass.$LAYOUT); swiftjava___FakeModule_globalReturnClass.call(_result); - return new MySwiftClass(_result, swiftArena$); + return MySwiftClass.wrapMemoryAddressUnsafe(_result, swiftArena$); } """ ) @@ -404,7 +404,7 @@ final class MethodImportTests { public static MySwiftClass init(long len, long cap, AllocatingSwiftArena swiftArena$) { MemorySegment _result = swiftArena$.allocate(MySwiftClass.$LAYOUT); swiftjava___FakeModule_MySwiftClass_init_len_cap.call(len, cap, _result) - return new MySwiftClass(_result, swiftArena$); + return MySwiftClass.wrapMemoryAddressUnsafe(_result, swiftArena$); } """ ) @@ -449,7 +449,7 @@ final class MethodImportTests { public static MySwiftStruct init(long len, long cap, AllocatingSwiftArena swiftArena$) { MemorySegment _result = swiftArena$.allocate(MySwiftStruct.$LAYOUT); swiftjava___FakeModule_MySwiftStruct_init_len_cap.call(len, cap, _result) - return new MySwiftStruct(_result, swiftArena$); + return MySwiftStruct.wrapMemoryAddressUnsafe(_result, swiftArena$); } """ ) From bf40dbc495318468e3b231eeeaf3d6c4609b4764 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Thu, 14 Aug 2025 18:08:01 +0900 Subject: [PATCH 2/2] Delete Sources/JavaKit/Helpers/_JNICache.swift --- Sources/JavaKit/Helpers/_JNICache.swift | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 Sources/JavaKit/Helpers/_JNICache.swift diff --git a/Sources/JavaKit/Helpers/_JNICache.swift b/Sources/JavaKit/Helpers/_JNICache.swift deleted file mode 100644 index e69de29b..00000000