Make JNI handling of Int/UInt match FFM mode. #460

pelekon · 2025-11-22T19:51:37Z

I believe that both modes should behave similarly when handling Int/UInt types. This PR makes JNI handle these types in the same way as FFM.

madsodgaard · 2025-11-23T09:07:44Z

Thanks for this! Definitely an important step that we handle Int/UInt soon, because that is the most common used integer type in Swift.

I do however, think we need to give this some more thought. Especially since the JNI mode will probably be used a lot by Android folks, which is commonly 32-bit. If you have the following Swift function:

func f(i: Int)

with this PR it gets translated to

void f(long i)

however, the downcall to Swift would be Int64(fromJNI: i, ...), which is not compatible with Int in Swift. so that would not work. I am not sure if its ok to just drop the top 32-bits, if we are running on a 32-bit machine. Maybe we need some kind of annotation mode like the unsigned stuff? Or throwing an error, if we are not in Int32.min...Int32.max on 32-bit? What do you think?

Also we should add some codegen tests and runtime tests while we are it, as well as add the UInt to the @Unsigned annotation handling

pelekon · 2025-11-23T10:51:49Z

I am aware of Int64 -> Int32 conversion problem but I didn't think about it as problem needed to be addressed in the same PR. To be honest I thought 32-bit is rare for Android, so I have no problem to work on solution for it in this PR.
I don't know much about annotations in Java, so for potential solutions I thought about SwiftInt java object which will throw IntegerOverflowException in those cases.
Of course I will add tests for it too :)

ktoso · 2025-11-23T10:57:52Z

Thanks for the PR, this would be good to address.

Overall the two "modes" should behave the same wherever possible and it is a problem that they're a bit independent currently so this would be good to align the two.

32bit is very rare and AFAICS there is a number of prominent apps that choose to not support 32bit Android devices.

The Java annotation mentioned is the https://github.com/swiftlang/swift-java/blob/main/SwiftKitCore/src/main/java/org/swift/swiftkit/core/annotations/Unsigned.java so please have a look how we're using it in the ffm mode to mark something was an "U"Int. The imported parameters or return types basically get an @Unsigned long etc, this is valuable because users can then handle it and notice in the API description, without having to guess

madsodgaard · 2025-11-23T11:45:43Z

I know 32-bit is rare these days. But since it's still a supported platform, for both Swift and the Swift Android SDK, I think we should still handle it. For example for us, most of our devices are 32-bit, and that is in the millions:)

OK by me to merge this and we can tackle it in another PR, just wanted to flag this, as we need to handle it somehow.
An idea could be sourcegen something similar to this:

@_cdecl()
func $f(i: jlong) {
  let swiftInt = Int64(fromJNI: i, ...)
  #if _pointerBitWidth(_32)
  guard swiftInt <= Int32.max, swiftInt >= Int32.min else {
    // throw overflow exception
  }
  #endif
}

maybe this could even just be in the extension Int: JavaValue methods.

ktoso · 2025-11-23T12:02:38Z

Well we can't merge anything this week, so yes, let's please polish this up before branches get unlocked next week.

…, implemented besic check for overflow of Int/UInt.

pelekon · 2025-11-24T19:00:58Z

I committed small change with a proposed solution for check and basic tests. Let me know what you think, and if the response is positive, I'll cover rest of JNI cases(enum cases etc) and apply it to FFM.

ktoso

Overall this looks quite neat, thank you.

Can you add some runtime tests please?

ktoso · 2025-11-26T13:23:07Z

Sources/JExtractSwiftLib/JNI/JNIJavaTypeTranslator.swift

    }
  }
+
+  static func indirectConversionSetepSwiftType(for knownKind: SwiftKnownTypeDeclKind, from knownTypes: SwiftKnownTypes) -> SwiftType? {


likely indirectConversionStepSwiftType?

ktoso · 2025-11-26T13:24:58Z

Sources/JavaTypes/JavaException.swift

+//===----------------------------------------------------------------------===//
+
+/// Describes a Java exception class (e.g. `SwiftIntegerOverflowException`)
+public struct JavaException: Equatable, Hashable {


Probably need to call this JavaExceptionType so we don't confuse ourselfes with a JavaExpcetion that would be a @javaclass

SwiftKitCore/src/main/java/org/swift/swiftkit/core/SwiftIntegerOverflowException.java

Tests/JExtractSwiftTests/JNI/JNIConversionChecksTests.swift

Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+SwiftThunkPrinting.swift

ktoso · 2025-11-28T05:04:23Z

Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+NativeTranslation.swift

    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?


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!

Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+NativeTranslation.swift

Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+SwiftThunkPrinting.swift

madsodgaard · 2025-11-28T05:35:19Z

This looks great!

I only have one comment regarding the implementation for maintainability. Up until now all conversions have just been done in the ConversionStep types. This includes any checks as well (if, guards), for example for nullable checks. As more people become interested in collaborating on the project, perhaps we should make sure we unify the way we do conversions. Just a bit worried that we deviate and do things differently in the project. Makes it harder for new contributors!

Was there a specific reason for introducing the "indirect" and "check" types? And not just doing it inside a ConversionStep?

pelekon · 2025-11-28T09:47:57Z

Firstly, thank you @madsodgaard for sharing your concern :)

Was there a specific reason for introducing the "indirect" and "check" types? And not just doing it inside a ConversionStep?

Of course, but I realise that for others this may not be a sufficient reason. As someone who just started to contribute I felt NativeSwiftConversionStep already does a lot and I didn't want to add too much to it. From my perspective, it should handle conversion between Java and Swift, while other things, such as checking and validation, should be handled by something else to separate their responsibilities. That is why I wanted to create a separate mechanism for handling this; I believe that this approach increases maintainability. Of course, I won't insist on this if you and @ktoso decide that the current approach is better, I have no objection to adapting to it :)

…ith ChatGPT)

ktoso · 2025-12-01T11:04:08Z

Sources/JExtractSwiftLib/SwiftTypes/SwiftType.swift

    }
  }

+  var isArchDependingInteger: Bool {


Typo

Suggested change

var isArchDependingInteger: Bool {

var isArchDependentInteger: Bool {

ktoso · 2025-12-01T11:07:56Z

Tests/JExtractSwiftTests/JNI/JNIIntConversionChecksTests.swift

+    case secondCase(UInt)
+  }
+  """
+


Can you please add a test to UnsignedNumberTests.swift that the .java output in .jni mode also looks correct? The unsigned integers should appear as void unsignedInt(@Unsigned int arg) { to Java.

ktoso · 2025-12-01T11:08:46Z

SwiftKitCore/src/main/java/org/swift/swiftkit/core/SwiftIntegerOverflowException.java

+ *   <li>{@link java.lang.RuntimeException}</li>
+ *   <li>SwiftIntegerOverflowException</li>
+ * </ul>
+ * </p>


Thank you for the docs, ok to drop the inheritance though -- javadoc does this automatically very well

ktoso · 2025-12-01T11:11:35Z

Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+NativeTranslation.swift

              ],
-              conversion: .initFromJNI(.placeholder, swiftType: type)
+              conversion: indirectStepType != nil ? .labelessAssignmentOfVariable(.placeholder, swiftType: type) : .initFromJNI(.placeholder, swiftType: type),
+              indirectConversion: indirectStepType.flatMap { .initFromJNI(.placeholder, swiftType: $0) },


Hmm, would we be able to make this indirect conversion step the parameter the same as conversionStep? However, I do like the conversionCheck, so we'd keep that perhaps, WDYT? Makes sense?

I don't love the fact we have two conversion steps and if one is nil we want the other one etc. The check is nice because it is always in addition to a conversion

Sure, I will move indirectConversion into conversion. I am not sure how soon I can do it cuz my PC died, but I will try to do it asap.

Oh that sucks to hear :\ Hope you can get a functional computer soon enough. thanks for the info!

ktoso · 2025-12-01T11:11:56Z

A few comments, this is shaping up quite well

Make JNI handling of Int/UInt match FFM mode.

dc3411d

Add support for checks during conversion of values from java to swift…

db9a600

…, implemented besic check for overflow of Int/UInt.

pelekon marked this pull request as draft November 25, 2025 18:22

ktoso reviewed Nov 28, 2025

View reviewed changes

pelekon added 3 commits November 29, 2025 17:00

Add documentation for SwiftIntegerOverflowException (ngl, generated w…

9aed0c3

…ith ChatGPT)

Review changes.

269e96a

More tests.

291e547

ktoso reviewed Dec 1, 2025

View reviewed changes

	var isArchDependingInteger: Bool {
	var isArchDependentInteger: Bool {

Make JNI handling of Int/UInt match FFM mode. #460

Are you sure you want to change the base?

Make JNI handling of Int/UInt match FFM mode. #460

Uh oh!

Conversation

pelekon commented Nov 22, 2025

Uh oh!

madsodgaard commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pelekon commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktoso commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

madsodgaard commented Nov 23, 2025

Uh oh!

ktoso commented Nov 23, 2025

Uh oh!

pelekon commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktoso left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

madsodgaard commented Nov 28, 2025

Uh oh!

pelekon commented Nov 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pelekon Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktoso commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

madsodgaard commented Nov 23, 2025 •

edited

Loading

pelekon commented Nov 23, 2025 •

edited

Loading

ktoso commented Nov 23, 2025 •

edited

Loading

pelekon commented Nov 24, 2025 •

edited

Loading

pelekon Dec 2, 2025 •

edited

Loading