floooh / sokol-rust

Rust bindings for the sokol headers (https://github.com/floooh/sokol)
zlib License
62 stars 7 forks source link

New warning: creating a mutable reference to mutable static is discouraged #26

Closed waywardmonkeys closed 5 months ago

waywardmonkeys commented 7 months ago
warning: creating a mutable reference to mutable static is discouraged
   --> examples/cube/cube.rs:114:26
    |
114 |     let state = unsafe { &mut STATE };
    |                          ^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see issue #114447 <https://github.com/rust-lang/rust/issues/114447>
    = note: this will be a hard error in the 2024 edition
    = note: this mutable reference has lifetime `'static`, but if the static gets accessed (read or written) by any other means, or any other reference is created, then any further use of this mutable reference is Undefined Behavior
help: use `addr_of_mut!` instead to create a raw pointer
    |
114 |     let state = unsafe { addr_of_mut!(STATE) };
    |                          ~~~~~~~~~~~~~~~~~~~
floooh commented 7 months ago

Yeah I noticed that too when testing with Rust 1.77, but just pushed the investigation to the side for now until I see the end of the tunnel in my current sokol-shdc work :)

waywardmonkeys commented 7 months ago

Just filing it for future reference ...

floooh commented 7 months ago

Ooof, I tinkered around a bit following Rust's error hints and the result looks even worse (Rust really doesn't like global mutable state it seems).

PS: twitter feedback:

You can do let state: &mut State = unsafe { &mut (*addr_of_mut!(STATE)) }; to keep the previous behavior.

...maybe it makes sense to add a couple of helper macros to the bindings... the "exotic" part that doesn't fit into Rust's expectations seems to be that there is no common function at the root of the call graph (like what's typically the main function).

floooh commented 7 months ago

More twitter feedback:

In the code you linked, you can wrap the state into a static Arc. Would be std-only without extra dependencies and looking at the API it seems plausible to have a lock in this situation. Cleanest way would allow the user to pass in their own state - but hard to do safely.

Looks like the C-with-records OOP approach in old Id code, and the callbacks make it hard to tunnel any object across the callback barrier - type and lifetime-wise. Just use the global static Arc, it’ll be ok

...for reference: https://twitter.com/FlohOfWoe/status/1774148497731670038

floooh commented 7 months ago

Wild idea: sokol-app has alternative userdata callbacks. It might be possible to come up with a Rust wrapper which passes a state reference from the sapp::run() function into the callbacks similar to what https://github.com/rust-windowing/winit does:

    let mut app = ControlFlowDemo::default();
    event_loop.run_app(&mut app)

(...and basically wrap the "raw" extern "C" fn callback functions into a Rust trait)

gw3583 commented 6 months ago

Added one possible workaround for this in https://github.com/floooh/sokol-rust/pull/30 - makes use of the user_data support to pass a heap allocated state struct.

floooh commented 5 months ago

https://github.com/floooh/sokol-rust/pull/30 merged.