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

Should NifTerm be an enum? #34

Closed
scrogson opened this issue Nov 28, 2016 · 8 comments
Closed

Should NifTerm be an enum? #34

scrogson opened this issue Nov 28, 2016 · 8 comments

Comments

@scrogson
Copy link
Member

scrogson commented Nov 28, 2016

It would be really nice to be able to use match on any NifTerm.

pub enum NifTerm<'a> {
    Atom { term: NIF_TERM, env: &'a NifEnv },
    Binary { term: NIF_TERM, env: &'a NifEnv },
    EmptyList { term: NIF_TERM, env: &'a NifEnv },
    Exception { term: NIF_TERM, env: &'a NifEnv },
    Fun { term: NIF_TERM, env: &'a NifEnv },
    List { term: NIF_TERM, env: &'a NifEnv },
    Map { term: NIF_TERM, env: &'a NifEnv },
    Pid { term: NIF_TERM, env: &'a NifEnv },
    Port { term: NIF_TERM, env: &'a NifEnv },
    Ref { term: NIF_TERM, env: &'a NifEnv },
    Tuple { term: NIF_TERM, env: &'a NifEnv },
}

There are enif_is_* functions available in the erlang_nif_sys to determine each of the variants above.

@hansihe thoughts?

@hansihe
Copy link
Member

hansihe commented Nov 28, 2016

This id certainly an interesting idea. It would be incredibly neat if you where able to use pattern matching on terms.

The main problem I can think of is that this would require us to recursively check the type of every term that enters nif-land. Because there is no function that returns the absolute type of a term, we would need to use the enif_is_* binary functions to determine the type of all terms. I don't know how expensive this would be, but it would at least require some thought.

@hansihe
Copy link
Member

hansihe commented Nov 28, 2016

I may have misunderstood a bit?

@OvermindDL1
Copy link

You could always submit a PR to the OTP Erlang project to add such a callback to get the type of a term as an enumeration. But yes, right now you'd need to use enif_is_* as each branch is tested for as far as I recall.

@scrogson
Copy link
Member Author

we would need to use the enif_is_* binary functions to determine the type of all terms. I don't know how expensive this would be, but it would at least require some thought.

@hansihe yes this is exactly what I had in mind. I also share the cost concern associated with recursively testing each term.

The main thing is that I find that you will need to do this sort of thing in user code anyways. Here is an incomplete example for turning erlang terms to JSON: https://github.com/scrogson/fast_json/blob/master/native/fast_json/src/lib.rs.in#L88-L102

Obviously not the most ergonomic way to deal with this sort of thing.

Perhaps it could be apart of the NifEncoder and NifDecoder traits so that the conversion happens in the userland code?

match term.decode(env) {
    Atom { ... } => ...,
    ...  
}

You could always submit a PR to the OTP Erlang project to add such a callback to get the type of a term as an enumeration.

@OvermindDL1 can you expand on this?

@hansihe
Copy link
Member

hansihe commented Nov 28, 2016

@scrogson your use case is something I hadn't considered. I have mainly been using it for cases where you already knew the structure of the type you wanted to ship in and out of erlang-land.

An alternative to making all types dynamic would be to have a DynamicTerm enum that a NifTerm could decode into. The decoder would then do all of the work, and it would make things a lot less awkward for your code. At the same time there would be no overhead for normal usage, only for when you need the functionality. What do you think? I can have a go at implementing it if you are willing to try it out?

Edit: Missed your code example. That is pretty much exactly how what I wrote would look like.

@scrogson
Copy link
Member Author

@hansihe sounds good! Let me know when it's ready and I'll give it a shot.

@scrogson
Copy link
Member Author

Closed by #39

@OvermindDL1
Copy link

@OvermindDL1 can you expand on this?

The erlang compiler is open-source, you could submit a PR to add a function like nif_get_type or so to the interface. :-)

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