Skip to content

Conversation

@pelekon
Copy link
Contributor

@pelekon pelekon commented Nov 22, 2025

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
Copy link
Contributor

madsodgaard commented Nov 23, 2025

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
Copy link
Contributor Author

pelekon commented Nov 23, 2025

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
Copy link
Collaborator

ktoso commented Nov 23, 2025

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
Copy link
Contributor

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
Copy link
Collaborator

ktoso commented Nov 23, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants