NIF Codegen rework

[scrogson]

I've got the new macros all working! 🎉

I'm pretty excited about this, but I'm pretty sure there are some opportunities for some clean up.

One thing I've noticed is that if you specify any term which is has decode called on it, that function needs to return a Result<T, E>. It would be nice to avoid this if possible.

Let me know what you think.

[evnu]

One thing I've noticed is that if you specify any term which is has decode called on it, that function needs to return a Result<T, E>.

I am not sure if I understand this correctly, but I think my comment here might cover this.

[evnu]

@scrogson I really like this PR, great work! :)

[scrogson]

I really like this PR, great work! :)

@evnu, thank you. This has been on my mind for quite a long time. It feels great to finally get it working. Thanks for your review!

[scrogson]

When looking into the failures with the rustler_mix_test I realized that the init! macro doesn't work without specifying the on_load function as either None or Some(fun).

error: expected expression, found `,`
 --> src/lib.rs:6:1
  |
6 | rustler::init!("Elixir.RustlerMixTest", [add]);
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected expression

Running cargo expand on the code reveals this output for the nif_load body:

extern "C" fn nif_load(
    env: rustler::codegen_runtime::NIF_ENV,
    _priv_data: *mut *mut rustler::codegen_runtime::c_void,
    load_info: rustler::codegen_runtime::NIF_TERM,
) -> rustler::codegen_runtime::c_int {
    unsafe {
        (/*ERROR*/)
    }
}

@evnu do you have any ideas how to fix that?

[evnu]

@evnu do you have any ideas how to fix that?

On mobile, so only a first question: originally, a NIf entry was of the form(erlang Name, rust function, optional flags), and right now we are missing the option to pass in the flags, right?

[scrogson]

@evnu the way I wrote the parser, I thought this would be covered (but apparently it isn't):

https://github.com/rusterlium/rustler/pull/264/files#diff-8daf9e0986d966ebc2c4878e8a772b5fR20-R25

[filmor]

I'll have a look at the code tomorrow, but one more suggestion: Now that this is an actual proc_macro, I think we should support a syntax like this instead:

rustler::init!("mod", entries, on_load = something)

This opens up the possibility of adding more parameters in the future (e.g. I'd really like to be able to allow a reload.

[scrogson]

This opens up the possibility of adding more parameters in the future (e.g. I'd really like to be able to allow a reload.

Yeah, I like that. I'll work on that tonight.

[evnu]

Running cargo expand on the code reveals this output for the nif_load body:

See https://github.com/rusterlium/rustler/pull/264/files#r341824704

We need to encode None as a TokenStream. Otherwise, it seems that expanding it results in the whole call to handle_nif_init_call() being dropped. I don't know why this happens (some property of quote!{} maybe?), but quoting None manually works.

[evnu]

@scrogson see evnu@1794b33 for an idea on how to fix the problem with on_load. See evnu@5e22350 for a proposal to fix the template.

[filmor]

@evnu Can you have a last look? In my opinion this is good to go.

Tremendous work @scrogson, this makes the interface so much simpler :)