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

Clippy: creating a shared reference to mutable static is discouraged #340

Open
MikkelPaulson opened this issue Jun 5, 2024 · 3 comments

Comments

@MikkelPaulson
Copy link
Collaborator

Clippy sez:

error: creating a shared reference to mutable static is discouraged
  --> web/src/lib.rs:91:40
   |
91 |     if let Some(element_id) = unsafe { &ROOT_ELEMENT_ID } {
   |                                        ^^^^^^^^^^^^^^^^ shared 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 shared reference has lifetime `'static`, but if the static ever gets mutated, or a mutable reference is created, then any further use of this shared reference is Undefined Behavior
   = note: `-D static-mut-refs` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(static_mut_refs)]`
help: use `addr_of!` instead to create a raw pointer
   |
91 |     if let Some(element_id) = unsafe { addr_of!(ROOT_ELEMENT_ID) } {
   |                                        ~~~~~~~~~~~~~~~~~~~~~~~~~

This needs a bit of testing to verify that it has no side effects, but "hard error in 2024" sounds bad.

MikkelPaulson added a commit that referenced this issue Jun 6, 2024
Issue #340 created for further investigation.
@ThinkerDreamer
Copy link

What kind of testing did you have in mind? I changed that to

fn get_root_element() -> Option<Element> {
    // #[allow(static_mut_refs)]
    if let Some(element_id) = unsafe { (*addr_of!(ROOT_ELEMENT_ID)).clone() } {
        window()
            .and_then(|w| w.document())
            .and_then(|d| d.get_element_by_id(&element_id))
    } else {
        None
    }
}

and I ran test.sh and all the existing tests passed.

@MikkelPaulson
Copy link
Collaborator Author

@ThinkerDreamer There are no end-to-end tests (#61), so this case probably wouldn't be covered. It will need to be tested manually. If the website loads at all, the change is valid.

@ThinkerDreamer
Copy link

Okay, I'll try it and see 👍🏻

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

2 participants