owned this note
owned this note
Published
Linked with GitHub
# Proposal: eliminate static from MIR places
## Proposal
* Remove `PlaceBase::Static` entirely, so that all places begin with a local variable
* Why? Simplifies the logic and shrinks places to 2 words
* Introduce two new Rvalue to get a pointer to a static `S` (where `S` has type `T`):
* `RefStatic(S)`, which produces a `&T` when executed
* `RawStatic(S)`, which produces a `*mut T` when executed
* When the user writes a shared borrow expression like `x = &S.foo`, this gets compiled to MIR like:
* `TMP = RefStatic(S)` where `TMP: &T`
* `x = &(*TMP).foo`
* When the user copies from a static like `S.foo`, this gets compiled to MIR like:
* `TMP = RefStatic(S)`
* `x = *TMP`
* When the user writes a mutable borrow expression like `x = &mut S.foo`, this gets compiled to:
* `TMP = RawStatic(S)` where `TMP: *mut T`
* `x = &(*TMP).foo`
* When the user assigns to a static like `S.foo = 22`, this gets compiled to:
* `TMP = RawStatic(S)` where `TMP: *mut T`
* `(*TMP).foo = 22`
## Interaction with the borrow checker
I believe that these transformations will not introduce new errors into borrow checking. The change is that instead of borrowing a specific projection from a static (`S.foo`) we are alwyas borrowing the entire static (`S`). However, in safe code these borrows are always shared borrows, so there is no conflict. In unsafe code, we don't check the borrows anyway.
(Also there is a "first principles" argument: knowing exactly which field our function accesses only matters if we can check all the other access to see that they are for disjoint fields, but since those accesses could occur in other functions we can never do that.)
## Interaction with stacked borrows
The changes described in the borrow checker section are relevant here, but this change may also impact some other (as yet "in progress") parts of stacked borrows.
The following was posted as a [Zulip comment by RalfJung][rj]:
[rj]: https://rust-lang.zulipchat.com/#narrow/stream/189540-t-compiler.2Fwg-mir-opt/topic/removing.20static.20from.20place/near/178538920
This moves us towards a model where the static itself does *not* have a "tag" at all as it isn't a place. There's no "unique root identity" at the bottom of the borrow stack of a static. That's in strong contrast to locals where there's a unique tag at the bottom. This unique tag is what makes `local = 5` kill all other pointers to that local. I thought the same should happen for statics and actually already did some implementation work along those lines in Miri (making sure we pick a consistent tag for the `static` place), but this proposal indicates to me that we maybe don't want a base identity for statics, and that `STATIC = 5` should *not* kill all other pointers (including raw pointers)!
So, this is illegal:
```
let mut x: u32 = 0;
let ptr = &raw mut x;
x = 2;
let _val = *ptr; // UB
```
But this is allowed:
```
static mut X: u32 = 0;
let ptr = &raw mut x;
X = 2;
let _val = *ptr; // all is good
```
I'd be happy to remove the code I wrote to make statics more like locals, it's somewhat annoyingly complicated (and other people ran into that code like a brick wall when trying to implement shims for an extern static in Miri).
So, summary: The change seems fine to me. It *does* have some effects on Stacked Borrows, maybe more than you expected, but I think these are good effects. Basically it we could take this to mean that there is an inconsistency between `static` and locals *both* in Stacked Borrows and the rustc internal data structure, and it's nice to at least be consistent in these inconsistencies :D
## Interaction with the unsafety checker
One implication of the above desugaring is that `&mut S.foo` (or assigning to a static, like `S.foo = 22`) is only legal in an unsafe block. This is because the generated MIR includes a dereference of a raw pointer (`*TMP`, in the examples above), and the unsafety checker will report an error if such a deref occurs outside of an unsafe block.
This is not a problem because, indeed, `&mut S` is only legal for a `static mut` variable and those can only be accessed inside unsafe blocks.
However, it may generate duplicate or unclear diagnostics, so we may want to add some kind of tweaks to improve those.
## Implementation steps
* First don't change the definition of place at all. Instead, just change place *lowering* so that for an expression like `x = &S.foo` we produce the MIR:
* `TMP = &S` where `TMP: &T`
* `x = &(*TMP).goo`
* Next, or as part of the same step, do the same transformation, but for `&mut S.foo` and `S.foo = ...`.
* Fix the diagnostics for unsafety checker
* Introduce `RefStatic` and `RawStatic` and start to use
* Change the definition of Place to exclude static
### old notes from when spastorino and I were talking
```rust
fn foo() {
let x: &&(u32, u32) = ...;
let y = &(**x).0;
}
```
```
Y = & (**x).1;
```
```
P0: &&(_ _) = &x
P1: &(_, _) = *P0
Y: &_ = &(*P1).1;
```
```
P0: *mut &(_, _) = &raw x;
P1 = *P0
Y =
```
## today
```rust
struct Place {
base: PlaceBase
proj: PlaceProjections
}
enum PlaceBase {
Local, Static
}
```
## tomorrow?
```rust
struct Place {
base: PlaceBase
proj: PlaceProjections
}
enum PlaceBase {
Local
}
static X: (u32, u32) = (22, 44);
let p: &u32 = &X.1;
```
```
enum Rvalue {
Ref(Place),
// Gives a `&M T` for a static of type T
RawStaticRef(Mutability, Static),
}
```
```
// old
p = &X.1;
// new
TMP: &(u32, u32) = RawStaticRef(shared, X);
p = &(*TMP).1;
```
```rust
static mut X: u32 = 22;
fn main() {
unsafe {
X += 1;
}
}
```
```
TMP: &mut (u32, u32) = RawStaticRef(mut, X);
*TMP += 1;
```
* Two rvalues:
* RefStatic(Static) -- gives a `&` to a static
* RawRefStatic(Mutability, Static) -- gives a `*const` or `*mut` to a static