-
Notifications
You must be signed in to change notification settings - Fork 67
Make JNI handling of Int/UInt match FFM mode. #460
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
dc3411d
db9a600
9aed0c3
269e96a
291e547
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -103,11 +103,16 @@ extension JNISwift2JavaGenerator { | |
| throw JavaTranslationError.unsupportedSwiftType(type) | ||
| } | ||
|
|
||
| let indirectStepType = JNIJavaTypeTranslator.indirectConversionSetepSwiftType(for: knownType, from: knownTypes) | ||
| let indirectCheck = JNIJavaTypeTranslator.checkStep(for: knownType, from: knownTypes) | ||
|
|
||
| return NativeParameter( | ||
| parameters: [ | ||
| JavaParameter(name: parameterName, type: javaType) | ||
| ], | ||
| conversion: .initFromJNI(.placeholder, swiftType: type) | ||
| conversion: indirectStepType != nil ? .labelessAssignmentOfVariable(.placeholder, swiftType: type) : .initFromJNI(.placeholder, swiftType: type), | ||
| indirectConversion: indirectStepType.flatMap { .initFromJNI(.placeholder, swiftType: $0) }, | ||
| conversionCheck: indirectCheck | ||
| ) | ||
|
|
||
| } | ||
|
|
@@ -129,7 +134,9 @@ extension JNISwift2JavaGenerator { | |
| fatalErrorMessage: "\(parameterName) was null in call to \\(#function), but Swift requires non-optional!" | ||
| ), | ||
| wrapperName: nominalTypeName | ||
| ) | ||
| ), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -138,15 +145,19 @@ extension JNISwift2JavaGenerator { | |
| parameters: [ | ||
| JavaParameter(name: parameterName, type: .long) | ||
| ], | ||
| conversion: .pointee(.extractSwiftValue(.placeholder, swiftType: type)) | ||
| conversion: .pointee(.extractSwiftValue(.placeholder, swiftType: type)), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
|
|
||
| case .tuple([]): | ||
| return NativeParameter( | ||
| parameters: [ | ||
| JavaParameter(name: parameterName, type: .void) | ||
| ], | ||
| conversion: .placeholder | ||
| conversion: .placeholder, | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
|
|
||
| case .function(let fn): | ||
|
|
@@ -169,7 +180,9 @@ extension JNISwift2JavaGenerator { | |
| conversion: .closureLowering( | ||
| parameters: parameters, | ||
| result: result | ||
| ) | ||
| ), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
|
|
||
| case .optional(let wrapped): | ||
|
|
@@ -242,7 +255,9 @@ extension JNISwift2JavaGenerator { | |
| .placeholder, | ||
| typeMetadataVariableName: .combinedName(component: "typeMetadataAddress"), | ||
| protocolNames: protocolNames | ||
| ) | ||
| ), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -272,7 +287,9 @@ extension JNISwift2JavaGenerator { | |
| .initFromJNI(.placeholder, swiftType: swiftType), | ||
| discriminatorName: discriminatorName, | ||
| valueName: valueName | ||
| ) | ||
| ), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -285,7 +302,9 @@ extension JNISwift2JavaGenerator { | |
| parameters: [ | ||
| JavaParameter(name: parameterName, type: javaType) | ||
| ], | ||
| conversion: .optionalMap(.initializeJavaKitWrapper(.placeholder, wrapperName: nominalTypeName)) | ||
| conversion: .optionalMap(.initializeJavaKitWrapper(.placeholder, wrapperName: nominalTypeName)), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -300,7 +319,9 @@ extension JNISwift2JavaGenerator { | |
| allowNil: true | ||
| ) | ||
| ) | ||
| ) | ||
| ), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
|
|
||
| default: | ||
|
|
@@ -434,7 +455,9 @@ extension JNISwift2JavaGenerator { | |
| parameters: [ | ||
| JavaParameter(name: parameterName, type: javaType) | ||
| ], | ||
| conversion: .getJValue(.placeholder) | ||
| conversion: .getJValue(.placeholder), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -570,7 +593,9 @@ extension JNISwift2JavaGenerator { | |
| parameters: [ | ||
| JavaParameter(name: parameterName, type: .array(javaType)), | ||
| ], | ||
| conversion: .initFromJNI(.placeholder, swiftType: .array(elementType)) | ||
| conversion: .initFromJNI(.placeholder, swiftType: .array(elementType)), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -594,7 +619,9 @@ extension JNISwift2JavaGenerator { | |
| convertLongFromJNI: false | ||
| )))) | ||
| ] | ||
| ) | ||
| ), | ||
| indirectConversion: nil, | ||
| conversionCheck: nil | ||
| ) | ||
|
|
||
| default: | ||
|
|
@@ -616,6 +643,12 @@ extension JNISwift2JavaGenerator { | |
|
|
||
| /// Represents how to convert the JNI parameter to a Swift parameter | ||
| let conversion: NativeSwiftConversionStep | ||
|
|
||
| /// Represents swift type for indirect variable used in required checks, e.g Int64 for Int overflow check on 32-bit platforms | ||
| let indirectConversion: NativeSwiftConversionStep? | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe let's add that "This will introduce a new name$indirect variabler" so it is easier to mentally connect the two? The "indirect" name sounds good! |
||
|
|
||
| /// Represents check operations executed in if/guard conditional block for check during conversion | ||
| let conversionCheck: NativeSwiftConversionCheck? | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very nicely integrated here 👍 |
||
| } | ||
|
|
||
| struct NativeResult { | ||
|
|
@@ -695,6 +728,8 @@ extension JNISwift2JavaGenerator { | |
| /// `{ (args) -> return body }` | ||
| indirect case closure(args: [String] = [], body: NativeSwiftConversionStep) | ||
|
|
||
| indirect case labelessAssignmentOfVariable(NativeSwiftConversionStep, swiftType: SwiftType) | ||
|
|
||
| /// Returns the conversion string applied to the placeholder. | ||
| func render(_ printer: inout CodePrinter, _ placeholder: String) -> String { | ||
| // NOTE: 'printer' is used if the conversion wants to cause side-effects. | ||
|
|
@@ -1025,6 +1060,20 @@ extension JNISwift2JavaGenerator { | |
| } | ||
| } | ||
| return printer.finalize() | ||
| case .labelessAssignmentOfVariable(let name, let swiftType): | ||
| return "\(swiftType)(indirect_\(name.render(&printer, placeholder)))" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| enum NativeSwiftConversionCheck { | ||
| case check32BitIntOverflow(typeWithMinAndMax: SwiftType) | ||
|
|
||
| // Returns the check string | ||
| func render(_ printer: inout CodePrinter, _ placeholder: String) -> String { | ||
| switch self { | ||
| case .check32BitIntOverflow(let minMaxSource): | ||
| return "\(placeholder) >= \(minMaxSource).min && \(placeholder) <= \(minMaxSource).max" | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
likely
indirectConversionStepSwiftType?