-
Notifications
You must be signed in to change notification settings - Fork 226
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
Exposing enif_schedule_nif safely #107
Comments
Would it really be a bad thing to change the return type to add a |
I think using a trait is better. The idea is that the user decides what the return type is, and Rustler figures out what to do with it. So a user may keep using |
I don't know how readable this is, but here's the approach I took for a little Scheme implementation I've been messing around with: jorendorff/cell-gc@149ce24 Before: fn char_upcase<'h>(_hs: &mut GcHeapSession<'h>, args: Vec<Value<'h>>) -> Result<Trampoline<'h>> {
if args.len() != 1 {
return Err("char-upcase: 1 argument required".into());
}
let c = args[0].clone().as_char("char-upcase: character required")?;
// I think the only character that doesn't upcase to a single character is
// U+00DF LATIN SMALL LETTER SHARP S ('ß'). Guile returns it unchanged.
// Fine with me.
let mut up = c.to_uppercase();
let result = match (up.next(), up.next()) {
(Some(d), None) => d,
_ => c,
};
Ok(Trampoline::Value(Value::Char(result)))
} After: builtins! {
fn char_upcase "char-upcase" <'h>(_hs, c: char) -> char {
// I think the only character that doesn't upcase to a single character is
// U+00DF LATIN SMALL LETTER SHARP S ('ß'). Guile returns it unchanged.
// Fine with me.
let mut up = c.to_uppercase();
match (up.next(), up.next()) {
(Some(d), None) => d,
_ => c,
}
}
} This probably looks like it's solving a completely different problem. But if we can cope with return values like |
Of course I don't mean to propose any particular detail of that for Rustler. In particular, that I just wanted to show that something like this is possible. And the traits I used aren't much different from the |
Requesting comments on an updated PR for this enhancement here: #232 |
enif_schedule_nif
comes with a contract, much likeenif_raise_exception
. It's hard to support safely.But we already support
enif_raise_exception
(see https://github.com/hansihe/rustler/blob/master/src/codegen_runtime.rs#L44 ). We could supportenif_schedule_nif
in the same way. Don't let the user call it directly, but let them return a special Rust value that means "callenif_schedule_nif
safely for me".There is one problem that we would need to solve, though. Currently, all NIFs have the return type
NifResult<NifTerm<'a>>
, allowing just two possibilities:Ok(term)
: "return this term"Err(exc)
: "raise this exception"Now we want to add a third option, "schedule this nif". But we can't just change the return type, because that would break all existing code. I guess the way to do this is with a trait. Come to think of it, we should allow a wide variety of return types anyway, not just
NifResult<NifTerm<'a>>
; for example, anything that implementsNifEncode
should be just as good as aNifTerm
, right?The text was updated successfully, but these errors were encountered: