search

tokio-rs / loom

Concurrency permutation testing tool for Rust.
MIT License
2.08k stars 110 forks source link

UnsafeCell::get_mut() is different from std #277

Open kvark opened 2 years ago

kvark commented 2 years ago

In std, it gets &mut self. In loom, it gets &self. I think it's not a good idea to diverge in this way. I understand the return type may be different, but &self should be borrowed in the same way as it is in std.

Darksonn commented 2 years ago

Ah, well, the get_mut function in loom was never intended to be the loom version of the standard library function of the same name. It is intended to be the loom version of UnsafeCell::get in the cases where you want to modify the value. (And our get is the version of UnsafeCell::get for situations where you only read the value.)

kvark commented 2 years ago

Ok, if the intent was so different, let's have a different name to avoid confusion. As far as I see, loom is generally used as a replacement for std, so having a difference in semantics like this is not desirable.

hawkw commented 2 years ago

it's worth noting that the type was previously named CausalCell, but was renamed to UnsafeCell in #111. here's Carl's rationale at the time: https://github.com/tokio-rs/loom/pull/111#discussion_r391882249

Darksonn commented 2 years ago

I don't think this api is that bad. Since std::cell::UnsafeCell::get can be used to reimplement std::cell::UnsafeCell::get_mut, you can use our get_mut both as a get and get_mut.

wyfo commented 1 year ago

Should it be to add a method equivalent to std::cell::UnsafeCell::get_mut in loom::cell::UnsafeCell? std get_mut is normally safe to call (because of exclusive reference), but calling loom equivalent requires unsafe code, while it could be avoided.

I've just written this post about implementing AsMut for cell types; could loom::cell::UnsafeCell also implement AsMut?