-
Notifications
You must be signed in to change notification settings - Fork 70
jextract: remove unsigned integers mode, remove wrap-guava mode #485
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
Conversation
ktoso
left a comment
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.
lgtm, nice to see code removal, we can always bring it back if we'd need to
| case .wrapGuava: .wrapUnsignedGuava | ||
| } | ||
| } | ||
| } No newline at end of file |
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.
lgtm on record that yeah i think we can remove this mode; if we'd ever want it back it'll be easy to bring back from past commits
| public func getJNIValue(in environment: JNIEnvironment) -> JNIType { | ||
| // On 32-bit, standard JDK jlong is defined as long long = Int64 | ||
| // On 64-bit, standard JDK jlong is defined as long = Int | ||
| // On Android it's correctly marked as int64_t |
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.
The comment could be clearer about the "why" this is ok: is it because it is 64bit on all Android versions we're supporting?
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.
I updated the docs, is that better?
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.
Thanks that makes more sense
Per the forums thread, we decided to just use the underlying bit representation of any unsigned integral types. On top of that we then always use the
@Unsignedannotation in JExtract, and remove the unsigned integers mode, since noone is really using wrap-guava.