anza-xyz / move

Move compiler targeting llvm supported backends
https://discord.gg/wFgfjG9J
Apache License 2.0
107 stars 32 forks source link

Fix ABI of move_rt_str_cmp_eq and move_rt_deserialize #393

Closed brson closed 8 months ago

brson commented 8 months ago

I was prototyping some type signatures for storage-related runtime calls and saw that these declarations had issues.

The ABI here technically worked as written, but is not defined to work according to Rust.

Any function called through FFI needs to be declared extern "C" to have a well-defined ABI and these weren't. Adding those declarations and the compiler printed warnings for some additional problems with the types used in these signatures.

move_rt_str_cmp_eq

This function accepted two string slices as arguments, but Rust slices do not have a defined layout. These args are now passed as ptr, len, ptr, len; which is actually how they are declared in LLVM. I thought it was pretty interesting that the LLVM lowering actually agreed with the Rust slice argument lowering.

move_rt_deserialize

This function returned a tuple, but Rust tuples (surprisingly) do not have defined layout, though they in practice have struct layout. I changed the return type to be a repr(C) struct. This aggregate return type also contained a slice, which I changed to a repr(C) struct of ptr, len so that it agrees with the LLVM declaration.

I also removed the pub declarations from deserialize and its constants, as they are not needed.