search

matklad / once_cell

Rust library for single assignment cells and lazy statics without macros
Apache License 2.0
1.87k stars 109 forks source link

Keep provenance intact by avoiding ptr-int-ptr #185

Closed saethlin closed 2 years ago

saethlin commented 2 years ago

once_cell is an extremely widely-used crate, so it would be very nice if it conformed to the stricted/simplest/checkable model we have for aliasing in Rust. To do this, we need to avoid creating a pointer from an integer by cast or transmute. Pointers created this way can be valid, but are a nightmare for a checker like Miri. The situation is generally explained by these docs: https://doc.rust-lang.org/nightly/std/ptr/fn.from_exposed_addr.html

This implementation is mostly gross because all the APIs that would make this ergonomic are behind #![feature(strict_provenance)]. (in particular, it would be great to have map_addr here)

saethlin commented 2 years ago

~When running the tests locally with MIRIFLAGS=-Zmiri-disable-isolation cargo miri test --all-features Miri ends up in a deadlock. It looks like this doesn't reproduce in CI.~ This is related to the parking_lot feature which has other problems I am reporting separately.

matklad commented 2 years ago

. (in particular, it would be great to have map_addr here)

Could you add a map_addr as a free-standing function ("polyfill")?

LoganDark commented 2 years ago

Is there a reason to be pasting in sptr rather than depending on it?

Noratrieb commented 2 years ago

I assume it's to avoid having an extra dependency, as sptr contains more code than is required here, which would have a negative impact on build times.

LoganDark commented 2 years ago

I assume it's to avoid having an extra dependency, as sptr contains more code than is required here, which would have a negative impact on build times.

sptr is a relatively small crate compared to what you'd usually blame for inflating compile times; a proper benchmark is required to reach a proper conclusion. I'd assume the bulk of the overhead would be the dependency itself (i.e. cargo having to download and compile it separately), not the code inside, but cargo is already very good at compiling large dependency trees.

matklad commented 2 years ago

bors r+

I don't think it's worth it to go from 0 deps to >0 deps just for this

LoganDark commented 2 years ago

Well, at least this is getting merged.

bors[bot] commented 2 years ago

Build succeeded:

matklad commented 2 years ago

Released as https://crates.io/crates/once_cell/1.13.1