Skip to content
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

Rust bindings #39

Closed
vshymanskyy opened this issue Jan 7, 2020 · 13 comments
Closed

Rust bindings #39

vshymanskyy opened this issue Jan 7, 2020 · 13 comments

Comments

@vshymanskyy
Copy link
Member

vshymanskyy commented Jan 7, 2020

Implement Rust bindings, so wasm3 can be easily used from Rust.

@vshymanskyy vshymanskyy added the help wanted Relying on community support label Jan 7, 2020
@Veykril
Copy link

Veykril commented Jan 21, 2020

I'll take a stab at that over the next few days. 😄
https://github.com/Veykril/wasm3-rs

@vshymanskyy
Copy link
Member Author

@Veykril FYI the project now uses CMake library structure.

@Veykril
Copy link

Veykril commented Jan 24, 2020

Ah great, I'll see if I can make use of it.

@Veykril
Copy link

Veykril commented Jan 26, 2020

I got a small question regarding the M3Result type. Currently its just a char pointer to static strings if I'm not mistaken in case of errors. What I would like to know is whether it would be possible to maybe replace this with constants/an enum and offer a function to map from that to the static error message string for example. The reason for why I am asking about this is because having the MResult the way it currently is makes it kinda annoying to map to in rust, while if they were just integer constants I could properly match on them and map them to rust error enums for example.

Would be great if you could let me know whether this would be possible. I wouldn't mind being the one to do the change either in that case 😄

@vshymanskyy
Copy link
Member Author

@soundandform any objections?

@soundandform
Copy link
Member

Seems okay. The interpreter does use these as well: the trap conditions. The interpreter return type needs to remain as-is. But it's probably fine to cast the enum to a fake pointer in those cases.

@Veykril
Copy link

Veykril commented Jan 27, 2020

Why exactly do the traps have to stay pointers? Aren't all traps checked by pointer equality anyways? I don't quite see where the problem would arise from moving those to the enum as well.

@soundandform
Copy link
Member

Yes. I'm saying make them enums as well. But when you do that work, you will find that the exec portion of the code is what returns the traps. Without going into deep discussion of how the interpreter works at this moment, this return type must remain a pointer. So, you'll have to do some enum to pointer casting and back.

@vshymanskyy
Copy link
Member Author

@Veykril let me know if you need any help

@Veykril
Copy link

Veykril commented Jan 28, 2020

Tried making the changes in #76. Also I think understood what Steven meant, I assume it was about the m3ret_t return type staying a pointer which makes sense.

@soundandform
Copy link
Member

Reflecting on this more, I don't see value in enumerating every error message for this library. These error messages are largely intended for humans, not the machine. Largely, there are just two cases: success (Wasm code is cool) and fail (Wasm code is bogus). This should be easy to distinguish in the foreign language: null-pointer or not null-pointer, which can be casted to a bool in C before the interface, if necessary.

The only fined-grain compare and switch I see in our own code thus far is (result == m3Err_trapExit). If this is difficult in the foreign language, it can be achieved by using a C helper function, e.g. bool m3_IsTrapExit (M3Result result) { return result == m3Err_trapExit; }

I use C-strings for errors because I've spent a lifetime looking at useless error dialogs on stupid-crashy software. Internal Error -10298 "Yeah, thanks!" :) Software developers (aka people) are lazy and if you give them a number to work with, they'll just print the number. Giving them a string forces their hand to display something useful.

@Veykril
Copy link

Veykril commented Jan 29, 2020

Mmh, ye thats actually a good point, given its only failure or success it's really not necessary. It would probably do more harm than good in the end I agree. I'll close the PR then. 😄

@vshymanskyy vshymanskyy removed the help wanted Relying on community support label Jan 31, 2020
@vshymanskyy
Copy link
Member Author

Thanks @Veykril. The wasm3-rs is featured on the main README file.
I'm closing this, but let's continue discussion here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants