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

Exposing enif_schedule_nif safely #107

Open
jorendorff opened this issue Jul 12, 2017 · 5 comments
Open

Exposing enif_schedule_nif safely #107

jorendorff opened this issue Jul 12, 2017 · 5 comments

Comments

@jorendorff
Copy link
Collaborator

enif_schedule_nif comes with a contract, much like enif_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 support enif_schedule_nif in the same way. Don't let the user call it directly, but let them return a special Rust value that means "call enif_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 implements NifEncode should be just as good as a NifTerm, right?

@treyzania
Copy link

Would it really be a bad thing to change the return type to add a Sched(...) option? It obviously would break existing code trying to upgrade, but Rustler isn't at the magical "1.0" yet so there is a little more flexibility.

@jorendorff
Copy link
Collaborator Author

jorendorff commented Jul 20, 2017

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 NifResult<NifTerm> for their existing NIFs... and write a new NIF that returns NifResult<NifSched> ... and write another new NIF that returns a Rust String, there's no reason Rustler couldn't handle all of them.

@jorendorff
Copy link
Collaborator Author

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 char, we can also cope with NifResult<NifTerm> vs. NifResult<NifSched> or whatever.

@jorendorff
Copy link
Collaborator Author

Of course I don't mean to propose any particular detail of that for Rustler. In particular, that builtins! macro syntax is silly. 🤡

I just wanted to show that something like this is possible. And the traits I used aren't much different from the NifEncode and NifDecode traits Rustler already has.

@sunny-g
Copy link
Contributor

sunny-g commented May 26, 2021

Requesting comments on an updated PR for this enhancement here: #232

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

No branches or pull requests

3 participants