jmriesen / rust-RSM

Educational/portfolio project where I rewrite the Reference Standard M interpreter in rust.
0 stars 0 forks source link

Create into_c trait #19

Open jmriesen opened 1 week ago

jmriesen commented 1 week ago

A pattern has emerged in the FFI handling code that should be formalized.

The current pattern is whenever I need to convert to the C equivalent of a type I create some into_c... method on the Rust type that handles the conversion.

By formalizing this pattern into a trait we will 1) enforce a consistent naming scheem 2) centralize where the ffi crate needs to be imported. (all trait implementations can be found using the lsp's get all references function)

jmriesen commented 4 days ago

Pausing this issue, I started to try an implement this but quickly ran into issues.

Primarily My issues revolved around 1) My Type conversions were not always one to one. This mostly came up when I used optional types. There are a number of functions in the C code that can fail with only one error code. Many of these functions return an Option in my rewrite since that seems like the more natural abstraction. This also has the unfortunate consequences is that the None variant of Option may correspond to a different error code depending on where it was called.

I could resolve this by switching over and using the Result type, however I still feel like Option is the right abstraction.

2) I have been running into issues with the orphan rule. It seems like the into_c trait should live in the FFI crate, however if I do that then I also have to place all the Option and Result<T,E> implementations in the FFI crate due to the orphanage rule.

With my current understanding of rust, I could not find a clean way to write these generic trait impls. There may be a way forward with this, but I don't think now is the time for me to do a deep dive into generic trait implementation best practices.

3) Sometimes I have been using Rust types to compare equality. This issues is pretty simple to solve compared to the others, as I could just start implementing From for Rust Type. However doing so dose contribute to the overall messiness of this proposal.