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 the mut_from_ref lint apply to unsafe functions? #4281

Open
MaikKlein opened this issue Jul 17, 2019 · 2 comments
Open

Should the mut_from_ref lint apply to unsafe functions? #4281

MaikKlein opened this issue Jul 17, 2019 · 2 comments

Comments

@MaikKlein
Copy link

pub struct MutUnsafeSlice<'s>(&'s mut [u32]);
struct Foo {}
impl Foo {
    pub unsafe fn foo(&self) -> &mut [u32] {
        panic!()
    }
    pub unsafe fn foo1(&self) -> MutUnsafeSlice {
        panic!()
    }
}
fn main() {}

playground

Should mut_from_ref apply to unsafe functions as well?

It is trivial to work around that error by wrapping the mutable reference in a new struct.

Here is the refcell implementation that does something very similar.

I personally don't have any strong opinions in either direction. I just needed to implement something similar to RefCell (runtime borrowing) and I ran into this lint.

@flip1995
Copy link
Member

IMO it should also trigger on unsafe functions. Even though the function is unsafe, the erroneous behavior this lint detects is the same: You're able to get multiple mutable references to the same data. Adding an allow to the function is even beneficial IMO, since it signals "Yes, I really want to do this".

@anko
Copy link

anko commented Dec 11, 2024

I think whether or not the function is unsafe is not really relevant. Any function (safe or unsafe) can contain an unsafe block that does some silly undefined behaviour that turns a & to &mut. The only ever correct way to turn & into &mut is UnsafeCell.

Is it possible to filter this lint based on whether the returned reference is derived from UnsafeCell? The original issue proposing the lint mentioned carving out an exception for UnsafeCell (presumably because this is literally what it is for), but it seems that didn't make it into the implementation.

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