Skip to content

Commit af341a0

Browse files
ktosoktoso
committed
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
1 parent 5dd3f36 commit af341a0

13 files changed

+84
-26
lines changed

‎Sources/JExtractSwiftLib/FFM/ConversionStep.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ enum ConversionStep: Equatable {
123123
// of splatting out text.
124124
let renderedArgumentList = renderedArguments.joined(separator: ", ")
125125
return "\(raw: type.description)(\(raw: renderedArgumentList))"
126-
126+
127127
case .tuplify(let elements):
128128
let renderedElements: [String] = elements.enumerated().map { (index, element) in
129129
element.asExprSyntax(placeholder: "\(placeholder)_\(index)", bodyItems: &bodyItems)!.description

‎Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaBindingsPrinting.swift

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@ extension FFMSwift2JavaGenerator {
407407
"arena$"
408408
}
409409

410+
// FIXME: use trailing$ convention
410411
let varName = outParameter.name.isEmpty ? "_result" : "_result_" + outParameter.name
411412

412413
printer.print(
@@ -463,7 +464,7 @@ extension FFMSwift2JavaGenerator.JavaConversionStep {
463464
switch self {
464465
case .placeholder, .explodedName, .constant, .readMemorySegment:
465466
return false
466-
case .constructSwiftValue:
467+
case .constructSwiftValue, .wrapMemoryAddressUnsafe:
467468
return true
468469

469470
case .call(let inner, _, _), .cast(let inner, _), .construct(let inner, _),
@@ -482,7 +483,11 @@ extension FFMSwift2JavaGenerator.JavaConversionStep {
482483
return false
483484
case .readMemorySegment:
484485
return true
485-
case .cast(let inner, _), .construct(let inner, _), .constructSwiftValue(let inner, _), .swiftValueSelfSegment(let inner):
486+
case .cast(let inner, _),
487+
.construct(let inner, _),
488+
.constructSwiftValue(let inner, _),
489+
.swiftValueSelfSegment(let inner),
490+
.wrapMemoryAddressUnsafe(let inner, _):
486491
return inner.requiresSwiftArena
487492
case .call(let inner, _, let withArena):
488493
return withArena || inner.requiresTemporaryArena
@@ -522,6 +527,10 @@ extension FFMSwift2JavaGenerator.JavaConversionStep {
522527
let inner = inner.render(&printer, placeholder)
523528
return "new \(javaType.className!)(\(inner), swiftArena$)"
524529

530+
case .wrapMemoryAddressUnsafe(let inner, let javaType):
531+
let inner = inner.render(&printer, placeholder)
532+
return "\(javaType.className!).wrapMemoryAddressUnsafe(\(inner), swiftArena$)"
533+
525534
case .construct(let inner, let javaType):
526535
let inner = inner.render(&printer, placeholder)
527536
return "new \(javaType)(\(inner))"

‎Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator+JavaTranslation.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -684,7 +684,7 @@ extension FFMSwift2JavaGenerator {
684684
outParameters: [
685685
JavaParameter(name: "", type: javaType)
686686
],
687-
conversion: .constructSwiftValue(.placeholder, javaType)
687+
conversion: .wrapMemoryAddressUnsafe(.placeholder, javaType)
688688
)
689689

690690
case .tuple:
@@ -732,6 +732,9 @@ extension FFMSwift2JavaGenerator {
732732
// Call 'new \(Type)(\(placeholder), swiftArena$)'.
733733
indirect case constructSwiftValue(JavaConversionStep, JavaType)
734734

735+
/// Call the `MyType.wrapMemoryAddressUnsafe` in order to wrap a memory address using the Java binding type
736+
indirect case wrapMemoryAddressUnsafe(JavaConversionStep, JavaType)
737+
735738
// Construct the type using the placeholder as arguments.
736739
indirect case construct(JavaConversionStep, JavaType)
737740

‎Sources/JExtractSwiftLib/FFM/FFMSwift2JavaGenerator.swift

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,9 +216,22 @@ extension FFMSwift2JavaGenerator {
216216

217217
printer.print(
218218
"""
219-
public \(decl.swiftNominal.name)(MemorySegment segment, AllocatingSwiftArena arena) {
219+
private \(decl.swiftNominal.name)(MemorySegment segment, AllocatingSwiftArena arena) {
220220
super(segment, arena);
221221
}
222+
223+
/**
224+
* Assume that the passed {@code MemorySegment} represents a memory address of a {@link \(decl.swiftNominal.name)}.
225+
* <p/>
226+
* Warnings:
227+
* <ul>
228+
* <li>No checks are performed about the compatibility of the pointed at memory and the actual \(decl.swiftNominal.name) types.</li>
229+
* <li>This operation does not copy, or retain, the pointed at pointer, so its lifetime must be ensured manually to be valid when wrapping.</li>
230+
* </ul>
231+
*/
232+
public static \(decl.swiftNominal.name) wrapMemoryAddressUnsafe(MemorySegment selfPointer, AllocatingSwiftArena swiftArena) {
233+
return new \(decl.swiftNominal.name)(selfPointer, swiftArena);
234+
}
222235
"""
223236
)
224237

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,22 @@ extension JNISwift2JavaGenerator {
117117

118118
printer.print(
119119
"""
120-
public \(decl.swiftNominal.name)(long selfPointer, SwiftArena swiftArena) {
120+
private \(decl.swiftNominal.name)(long selfPointer, SwiftArena swiftArena) {
121121
super(selfPointer, swiftArena);
122122
}
123+
124+
/**
125+
* Assume that the passed {@code long} represents a memory address of a {@link \(decl.swiftNominal.name)}.
126+
* <p/>
127+
* Warnings:
128+
* <ul>
129+
* <li>No checks are performed about the compatibility of the pointed at memory and the actual \(decl.swiftNominal.name) types.</li>
130+
* <li>This operation does not copy, or retain, the pointed at pointer, so its lifetime must be ensured manually to be valid when wrapping.</li>
131+
* </ul>
132+
*/
133+
public static \(decl.swiftNominal.name) wrapMemoryAddressUnsafe(long selfPointer, SwiftArena swiftArena) {
134+
return new \(decl.swiftNominal.name)(selfPointer, swiftArena);
135+
}
123136
"""
124137
)
125138

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ extension JNISwift2JavaGenerator {
383383
javaType: javaType,
384384
annotations: resultAnnotations,
385385
outParameters: [],
386-
conversion: .constructSwiftValue(.placeholder, javaType)
386+
conversion: .wrapMemoryAddressUnsafe(.placeholder, javaType)
387387
)
388388

389389
case .tuple([]):
@@ -465,7 +465,7 @@ extension JNISwift2JavaGenerator {
465465
discriminatorName: "result_discriminator$",
466466
optionalClass: "Optional",
467467
javaType: .long,
468-
toValue: .constructSwiftValue(.placeholder, .class(package: nil, name: nominalTypeName))
468+
toValue: .wrapMemoryAddressUnsafe(.placeholder, .class(package: nil, name: nominalTypeName))
469469
)
470470
)
471471

@@ -582,6 +582,9 @@ extension JNISwift2JavaGenerator {
582582
/// Call `new \(Type)(\(placeholder), swiftArena$)`
583583
indirect case constructSwiftValue(JavaNativeConversionStep, JavaType)
584584

585+
/// Call the `MyType.wrapMemoryAddressUnsafe` in order to wrap a memory address using the Java binding type
586+
indirect case wrapMemoryAddressUnsafe(JavaNativeConversionStep, JavaType)
587+
585588
indirect case call(JavaNativeConversionStep, function: String)
586589

587590
indirect case method(JavaNativeConversionStep, function: String, arguments: [JavaNativeConversionStep] = [])
@@ -641,6 +644,10 @@ extension JNISwift2JavaGenerator {
641644
case .constructSwiftValue(let inner, let javaType):
642645
let inner = inner.render(&printer, placeholder)
643646
return "new \(javaType.className!)(\(inner), swiftArena$)"
647+
648+
case .wrapMemoryAddressUnsafe(let inner, let javaType):
649+
let inner = inner.render(&printer, placeholder)
650+
return "\(javaType.className!).wrapMemoryAddressUnsafe(\(inner), swiftArena$)"
644651

645652
case .call(let inner, let function):
646653
let inner = inner.render(&printer, placeholder)
@@ -705,7 +712,7 @@ extension JNISwift2JavaGenerator {
705712
case .placeholder, .constant, .isOptionalPresent:
706713
return false
707714

708-
case .constructSwiftValue:
715+
case .constructSwiftValue, .wrapMemoryAddressUnsafe:
709716
return true
710717

711718
case .valueMemoryAddress(let inner):

‎Sources/JavaKit/Helpers/_JNICache.swift

Whitespace-only changes.

‎Tests/JExtractSwiftTests/DataImportTests.swift

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ final class DataImportTests {
8686
]
8787
)
8888
}
89-
89+
9090
@Test("Import Data: JavaBindings")
9191
func data_javaBindings() throws {
9292

@@ -167,7 +167,7 @@ final class DataImportTests {
167167
public static Data returnData(AllocatingSwiftArena swiftArena$) {
168168
MemorySegment _result = swiftArena$.allocate(Data.$LAYOUT);
169169
swiftjava_SwiftModule_returnData.call(_result);
170-
return new Data(_result, swiftArena$);
170+
return Data.wrapMemoryAddressUnsafe(_result, swiftArena$);
171171
}
172172
""",
173173

@@ -210,7 +210,7 @@ final class DataImportTests {
210210
public static Data init(java.lang.foreign.MemorySegment bytes, long count, AllocatingSwiftArena swiftArena$) {
211211
MemorySegment _result = swiftArena$.allocate(Data.$LAYOUT);
212212
swiftjava_SwiftModule_Data_init_bytes_count.call(bytes, count, _result);
213-
return new Data(_result, swiftArena$);
213+
return Data.wrapMemoryAddressUnsafe(_result, swiftArena$);
214214
}
215215
""",
216216

@@ -408,4 +408,5 @@ final class DataImportTests {
408408
]
409409
)
410410
}
411+
411412
}

‎Tests/JExtractSwiftTests/JNI/JNIClassTests.swift

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,17 @@ struct JNIClassTests {
6666
System.loadLibrary(LIB_NAME);
6767
return true;
6868
}
69-
70-
public MyClass(long selfPointer, SwiftArena swiftArena) {
69+
""",
70+
"""
71+
private MyClass(long selfPointer, SwiftArena swiftArena) {
7172
super(selfPointer, swiftArena);
7273
}
7374
""",
75+
"""
76+
public static MyClass wrapMemoryAddressUnsafe(long selfPointer, SwiftArena swiftArena) {
77+
return new MyClass(selfPointer, swiftArena);
78+
}
79+
"""
7480
])
7581
try assertOutput(
7682
input: source,
@@ -164,7 +170,7 @@ struct JNIClassTests {
164170
* }
165171
*/
166172
public static MyClass init(long x, long y, SwiftArena swiftArena$) {
167-
return new MyClass(MyClass.$init(x, y), swiftArena$);
173+
return MyClass.wrapMemoryAddressUnsafe(MyClass.$init(x, y), swiftArena$);
168174
}
169175
""",
170176
"""
@@ -175,7 +181,7 @@ struct JNIClassTests {
175181
* }
176182
*/
177183
public static MyClass init(SwiftArena swiftArena$) {
178-
return new MyClass(MyClass.$init(), swiftArena$);
184+
return MyClass.wrapMemoryAddressUnsafe(MyClass.$init(), swiftArena$);
179185
}
180186
""",
181187
"""
@@ -309,7 +315,7 @@ struct JNIClassTests {
309315
* }
310316
*/
311317
public MyClass copy(SwiftArena swiftArena$) {
312-
return new MyClass(MyClass.$copy(this.$memoryAddress()), swiftArena$);
318+
return MyClass.wrapMemoryAddressUnsafe(MyClass.$copy(this.$memoryAddress()), swiftArena$);
313319
}
314320
""",
315321
"""

‎Tests/JExtractSwiftTests/JNI/JNIOptionalTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ struct JNIOptionalTests {
158158
public static Optional<MyClass> optionalClass(Optional<MyClass> arg, SwiftArena swiftArena$) {
159159
byte[] result_discriminator$ = new byte[1];
160160
long result$ = SwiftModule.$optionalClass(arg.map(MyClass::$memoryAddress).orElse(0L), result_discriminator$);
161-
return (result_discriminator$[0] == 1) ? Optional.of(new MyClass(result$, swiftArena$)) : Optional.empty();
161+
return (result_discriminator$[0] == 1) ? Optional.of(MyClass.wrapMemoryAddressUnsafe(result$, swiftArena$)) : Optional.empty();
162162
}
163163
""",
164164
"""

0 commit comments

Comments
 (0)