UnsafeCell::into_inner should not be unsafe

[strega-nil]

It's been unsafe since 2014 when it was called unwrap. Nobody thought about it, I guess, but it shouldn't be unsafe.

[retep998]

For reference the PR which added it is #12686

[durka]

Someone thought about it enough to document the rationale:

This function is unsafe because this thread or another thread may currently be inspecting the inner value.

Has something changed to make this consideration unnecessary or do you have an explanation of why the reasoning is faulty?

[retep998]

@durka Because the function takes self so by definition it must be the owner of the value so there can be no other viewers, either from this thread or another thread.

[strega-nil]

Although this isn't backwards-compatible

let x: unsafe fn(_) -> _ = UnsafeCell::<u32>::into_inner;

there is some prior art of going unsafe -> safe: std::mem::forget (although, that was pre-1.0)

[durka]

That's weird, it seems like you ought to be able to assign a safe fn to an
unsafe fn pointer.

On Wed, Jul 27, 2016 at 3:38 PM, Nicole Mazzuca notifications@github.com
wrote:

Although this isn't backwards-compatible

let x: unsafe fn(_) -> _ = UnsafeCell::::into_inner;

there is some prior art of going unsafe -> safe: std::mem::forget
(although, that was pre-1.0)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#35067 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAC3n27xH8O46Airgb7KocadqJX4BX33ks5qZ7OrgaJpZM4JV6mY
.

[nikomatsakis]

We removed the subtyping relation at some point; it does seem like a coercion would be (probably) ok, but that would still not be (strictly) backwards compatible.

[nikomatsakis]

(I tend to agree that into_inner could be safe, though, and I highly doubt any existing crates would actually break due to this change.)

[retep998]

We could always do a crater run just to be absolutely sure nobody is turning UnsafeCell::into_inner into a function pointer.

[cyplo]

HI ! Any news on this one ?

[nikomatsakis]

No change that I know of.

[cramertj]

FWIW the relevant coercion was added in #37389.

[asomers]

This change is breaking my crate. If I add the unsafe then it will work for older compilers. If I remove the unsafe then it will work for newer compilers. Is there a conditional compilation attribute for compiler version? I can't find one.

[cramertj]

@asomers Use #[allow(unused_unsafe)] unsafe { ... }

[asomers]

Yeah, that's what I'm doing for now. It seems kind of lame, though. But it does work.