Skip to content

Commit

Permalink
Fix kotlin unsigned type conversion (#747)
Browse files Browse the repository at this point in the history
* fix unsigned type conversion on ffi boundary

* cleaner fix

* tests

* fmt

* typo

* comments

* remove commented out code

---------

Co-authored-by: Ellen Arteca <emarteca@google.com>
  • Loading branch information
emarteca and Ellen Arteca authored Dec 10, 2024
1 parent f5384d4 commit 893cf50
Show file tree
Hide file tree
Showing 13 changed files with 109 additions and 22 deletions.
2 changes: 2 additions & 0 deletions feature_tests/c/include/OpaqueMutexedString.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion feature_tests/c/include/TesterTrait.d.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions feature_tests/cpp/include/OpaqueMutexedString.d.hpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions feature_tests/cpp/include/OpaqueMutexedString.hpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions feature_tests/dart/lib/src/OpaqueMutexedString.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions feature_tests/js/api/OpaqueMutexedString.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions feature_tests/js/api/OpaqueMutexedString.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions feature_tests/src/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ pub mod ffi {
.collect();
Box::new(Utf16Wrap(chars))
}

pub fn to_unsigned_from_unsigned(&self, input: u16) -> u16 {
input
}
}

impl Utf16Wrap {
Expand Down
4 changes: 2 additions & 2 deletions feature_tests/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ mod ffi {
y: i32,
}
pub trait TesterTrait {
fn test_trait_fn(&self, x: i32) -> i32;
fn test_trait_fn(&self, x: u32) -> u32;
fn test_void_trait_fn(&self);
fn test_struct_trait_fn(&self, s: TraitTestingStruct) -> i32;
}
Expand All @@ -18,7 +18,7 @@ mod ffi {
impl TraitWrapper {
pub fn test_with_trait(t: impl TesterTrait, x: i32) -> i32 {
t.test_void_trait_fn();
t.test_trait_fn(x)
t.test_trait_fn(x.try_into().unwrap()).try_into().unwrap()
}

pub fn test_trait_with_struct(t: impl TesterTrait) -> i32 {
Expand Down
51 changes: 42 additions & 9 deletions tool/src/kotlin/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ impl<'tcx> KotlinFormatter<'tcx> {

pub fn fmt_primitive_to_native_conversion(&self, name: &str, prim: PrimitiveType) -> String {
match prim {
PrimitiveType::Bool => format!("{name}.toByte()"),
PrimitiveType::Int(IntType::U8) => format!("{name}.toByte()"),
PrimitiveType::Int(IntType::U16) => format!("{name}.toShort()"),
PrimitiveType::Int(IntType::U32) => format!("{name}.toInt()"),
Expand Down Expand Up @@ -70,21 +69,55 @@ impl<'tcx> KotlinFormatter<'tcx> {
"Array<String>"
}

pub fn fmt_primitive_as_ffi(&self, prim: PrimitiveType) -> &'static str {
pub fn fmt_primitive_as_ffi(
&self,
prim: PrimitiveType,
support_unsigned: bool,
) -> &'static str {
match prim {
PrimitiveType::Bool => "Byte",
PrimitiveType::Bool => "Boolean",
PrimitiveType::Char => "Int",
PrimitiveType::Int(IntType::I8) => "Byte",
PrimitiveType::Int(IntType::I16) => "Short",
PrimitiveType::Int(IntType::I32) => "Int",
PrimitiveType::Int(IntType::I64) => "Long",
PrimitiveType::Int(IntType::U8) => "UByte",
PrimitiveType::Int(IntType::U16) => "UShort",
PrimitiveType::Int(IntType::U32) => "UInt",
PrimitiveType::Int(IntType::U64) => "ULong",
PrimitiveType::Int(IntType::U8) => {
if support_unsigned {
"UByte"
} else {
"Byte"
}
}
PrimitiveType::Int(IntType::U16) => {
if support_unsigned {
"UShort"
} else {
"Short"
}
}
PrimitiveType::Int(IntType::U32) => {
if support_unsigned {
"UInt"
} else {
"Int"
}
}
PrimitiveType::Int(IntType::U64) => {
if support_unsigned {
"ULong"
} else {
"Long"
}
}
PrimitiveType::Byte => "Byte",
PrimitiveType::IntSize(IntSizeType::Isize) => "Long",
PrimitiveType::IntSize(IntSizeType::Usize) => "Long",
PrimitiveType::IntSize(IntSizeType::Usize) => {
if support_unsigned {
"ULong"
} else {
"Long"
}
}
PrimitiveType::Float(FloatType::F32) => "Float",
PrimitiveType::Float(FloatType::F64) => "Double",
PrimitiveType::Int128(_) => panic!("i128 not supported in Kotlin"),
Expand Down Expand Up @@ -342,7 +375,7 @@ impl<'tcx> KotlinFormatter<'tcx> {
PrimitiveType::Int(IntType::U32) => "Int",
PrimitiveType::Int(IntType::U64) => "Long",
PrimitiveType::IntSize(_) => "Long",
prim => self.fmt_primitive_as_ffi(prim),
prim => self.fmt_primitive_as_ffi(prim, false),
}
}

Expand Down
21 changes: 15 additions & 6 deletions tool/src/kotlin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,11 +597,11 @@ return string{return_type_modifier}"#
_ => todo!(),
},
Slice::Primitive(Some(_), prim_ty) => {
let prim_ty = self.formatter.fmt_primitive_as_ffi(*prim_ty);
let prim_ty = self.formatter.fmt_primitive_as_kt(*prim_ty);
format!(" return PrimitiveArrayTools.get{prim_ty}Array({val_name}){return_type_modifier}")
}
Slice::Primitive(None, prim_ty) => {
let prim_ty = self.formatter.fmt_primitive_as_ffi(*prim_ty);
let prim_ty = self.formatter.fmt_primitive_as_kt(*prim_ty);
let prim_ty_array = format!("{prim_ty}Array");
Self::boxed_slice_return(prim_ty_array.as_str(), val_name, return_type_modifier)
}
Expand Down Expand Up @@ -1106,7 +1106,7 @@ returnVal.option() ?: return null
.unzip();
let (native_output_type, return_modification) = match **output {
Some(ref ty) => (
self.gen_native_type_name(ty, None).into(),
self.gen_native_type_name(ty, None, false).into(),
match ty {
Type::Enum(..) => ".toNative()",
Type::Struct(..) => ".nativeStruct",
Expand Down Expand Up @@ -1286,7 +1286,7 @@ returnVal.option() ?: return null

param_decls.push(format!(
"{param_name}: {}",
self.gen_native_type_name(&param.ty, additional_name.clone()),
self.gen_native_type_name(&param.ty, additional_name.clone(), false),
));
}
if let ReturnType::Infallible(SuccessType::Write)
Expand Down Expand Up @@ -1603,7 +1603,7 @@ returnVal.option() ?: return null
});
let (native_output_type, return_modification) = match *method.output {
Some(ref ty) => (
self.gen_native_type_name(ty, None).into(),
self.gen_native_type_name(ty, None, true).into(),
match ty {
Type::Enum(..) => ".toNative()",
Type::Struct(..) => ".nativeStruct",
Expand Down Expand Up @@ -1827,9 +1827,18 @@ returnVal.option() ?: return null
&self,
ty: &Type<P>,
additional_name: Option<String>,
// flag to represent whether the API this type is a part of has support for unsigned types.
// Non-trait methods do not, because they are called through the JNA library built in Java
// which doesn't support unsigned types.
// The true fix is to use JNA `IntegerType` to represent unsigned ints:
// TODO: https://github.com/rust-diplomat/diplomat/issues/748
support_unsigned: bool,
) -> Cow<'cx, str> {
match *ty {
Type::Primitive(prim) => self.formatter.fmt_primitive_as_ffi(prim).into(),
Type::Primitive(prim) => self
.formatter
.fmt_primitive_as_ffi(prim, support_unsigned)
.into(),
Type::Opaque(ref op) => {
let optional = if op.is_optional() { "?" } else { "" };
format!("Pointer{optional}").into()
Expand Down

0 comments on commit 893cf50

Please sign in to comment.