rust-diplomat / diplomat

Experimental Rust tool for generating FFI definitions allowing many other languages to call Rust code
https://rust-diplomat.github.io/book/
Other
532 stars 51 forks source link

Kotlin result #728

Closed jcrist1 closed 1 week ago

jcrist1 commented 1 week ago

Uses kotlin result. An attempt to address #697. I have to admit I'm not a big fan. On the plus side it is more ergonomic, but a big minus is we completely erase the type of the error. Unfortunately as per the discussion in the issue we can't make a generic wrapping class. Instead we would have to change the error type to extend Throwable. Let me know what y'all think of this.

jcrist1 commented 1 week ago

(a) can we get rid of Res?

Yes and done.

(b) Can we replace the .ok/.err methods that seem to be globally added (?) with local free functions?

I'm not sure I understand. We can move them to always be local to the class, but I don't understand the benefit as it would just duplicate the code accross classes. Could you elaborate on the reasoning? In any case it should be a small change. I've made them internal now, so they shouldn't escape the library.

As for free functions, I'm not familiary with the term. A quick googling tells me it's like a function which doesn't take a "self" param in Rust's terminology. However, a lot of the code flow assumes that the .ok() and .err() will be applied as postfix operations, as such it would require more changes to get them to be free functions, if I'm understanding it correctly.

Manishearth commented 1 week ago

We can move them to always be local to the class,

No I just meant package local. internal is fine.

As for free functions, I'm not familiary with the term. A quick googling tells me it's like a function which doesn't take a "self" param in Rust's terminology.

I was thinking of a free function as in the equivalent of a standalone fn in rust, not on an object. If Kotlin doesn't have that, then perhaps a static method on some shared utility class.

But this is fine too!!

Manishearth commented 1 week ago

cc @emarteca