rusterlium / rustler

Safe Rust bridge for creating Erlang NIF functions
https://docs.rs/crate/rustler
Apache License 2.0
4.24k stars 222 forks source link

rustler::resource! produces compiler warning #606

Closed skwerlman closed 5 days ago

skwerlman commented 2 months ago

rust: 1.79.0-nightly (7f2fc33da 2024-04-22) rustler: 0.32.1 elixir: 1.16.2 erlang: 26.2.4

this code:

struct SimplexResource {
    pub gen: Simplex,
}

pub fn load(env: Env, _: Term) -> bool {
    rustler::resource!(SimplexResource, env);
    true
}

produces this warning:

warning: non-local `impl` definition, they should be avoided as they go against expectation
 --> src/lib.rs:8:5
  |
8 |     rustler::resource!(SimplexResource, env);
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: move this `impl` block outside the of the current function `load`
  = note: an `impl` definition is non-local if it is nested inside an item and may impact type checking outside of that item. This can be the case if neither the trait or the self type are at the same nesting level as the `impl`
  = note: one exception to the rule are anon-const (`const _: () = { ... }`) at top-level module and anon-const at the same nesting as the trait or type
  = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
  = note: the macro `rustler::resource` may come from an old version of the `rustler` crate, try updating your dependency with `cargo update -p rustler`
  = note: `#[warn(non_local_definitions)]` on by default
  = note: this warning originates in the macro `rustler::resource` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `noise_nif` (lib) generated 1 warning

and with the suggested -Z flag:

warning: non-local `impl` definition, they should be avoided as they go against expectation
   --> /home/sk/.local/share/cargo/registry/src/index.crates.io-6f17d22bba15001f/rustler-0.32.1/src/resource.rs:292:13
    |
273 |   macro_rules! resource {
    |   --------------------- in this expansion of `rustler::resource!`
...
292 | /             impl $crate::resource::ResourceTypeProvider for $struct_name {
293 | |                 fn get_type() -> &'static $crate::resource::ResourceType<Self> {
294 | |                     unsafe { &STRUCT_TYPE }.as_ref()
295 | |                         .expect("The resource type hasn't been initialized. Did you remember to call the function where you used the `resource!` macro?")
296 | |                 }
297 | |             }
    | |_____________^
    |
   ::: src/lib.rs:8:5
    |
8   |       rustler::resource!(SimplexResource, env);
    |       ---------------------------------------- in this macro invocation
    |
    = help: move this `impl` block outside the of the current function `load`
    = note: an `impl` definition is non-local if it is nested inside an item and may impact type checking outside of that item. This can be the case if neither the trait or the self type are at the same nesting level as the `impl`
    = note: one exception to the rule are anon-const (`const _: () = { ... }`) at top-level module and anon-const at the same nesting as the trait or type
    = note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <https://github.com/rust-lang/rust/issues/120363>
    = note: the macro `rustler::resource` may come from an old version of the `rustler` crate, try updating your dependency with `cargo update -p rustler`
    = note: `#[warn(non_local_definitions)]` on by default

warning: `noise_nif` (lib) generated 1 warning
filmor commented 2 months ago

Does this also happen on beta? Warnings/lints are always a bit in flux.

skwerlman commented 2 months ago

it doesn't happen on beta, just nightly

filmor commented 2 months ago

Hmm, looking at https://github.com/rust-lang/rfcs/blob/master/text/3373-avoid-nonlocal-definitions-in-fns.md, this might require us to adjust how resource types are registered. I'd much rather have this automatic anyhow (i.e. a derive macro and/or including the types in rustler::init!).

bm-w commented 4 weeks ago

FWIW, this is now happening on beta as well, as of 1.80.0-beta.2. Normally this means this will start happening on stable in 6 weeks from now, when that reaches 1.80.

filmor commented 3 weeks ago

Good to know, thanks. It's still just a warning and we'll have to handle it properly once we switch to edition 2024 which will only be available in a stable version in October (https://doc.rust-lang.org/edition-guide/rust-2024/index.html). I have a working alternative implementation in #617, but I'd like to explore a few more possible changes or adjustments if we have to break the interface anyhow.