-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: Add bindings for exception handling (try/catch/throw/rethrow) #197
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
We generally try to keep the C bindings to match the C API as closely as possible—would you mind updating this to just have a caml_binaryen_try
and then implement the other APIs in OCaml, just calling that one function? That way we get the same interface but less binding code (and guarantees the same behavior across JS and OCaml).
@ospencer Good call. Fixed 👍 |
@@ -1884,6 +1884,339 @@ caml_binaryen_ref_eq(value _module, value _left, value _right) { | |||
CAMLreturn(alloc_BinaryenExpressionRef(exp)); | |||
} | |||
|
|||
// Exception handling operations | |||
CAMLprim value | |||
caml_binaryen_try_native(value _module, value _name, value _body, value _catchTags, value _catchBodies, value _delegateTarget) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we name this just caml_binaryen_try
?
//Provides: caml_binaryen_try_get_name | ||
//Requires: Binaryen, caml_string_of_jsstring | ||
function caml_binaryen_try_get_name(expr) { | ||
return caml_string_of_jsstring(Binaryen['_BinaryenTryGetName'](expr)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were you able to verify that this works? I don't know if this actually returns a JS string or if it's just a pointer or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think any of the raw _Binaryen
functions actually return strings, as those would be operating in Asm.js memory space
Closes #189 .
One note about this API: the Binaryen constructor for
try
accepts either a list of catch expressions or a delegate tag. Keeping in line with the other bindings, we expose this API throughBinaryen.Try.make
, but we additionally defineBinaryen.Try_Catch.make
andBinaryen.Try_Delegate.make
expressions which are more safe to use (since users don't need to worry about passingNone
toTry.make
)