Track the "static"-ness of slots in MIR and detect drops in constant initializers.

[eddyb]

Right now we distinguish between "constant initializer" (static, const, promoted fragments) and "runtime" MIR, in that all locals in the former are 'static and drops on them are ignored.
While this seems to work fine for the moment, it can't model, e.g. (mutable) local variables in "constant initializers" and may not be backwards-compatible with a model that can, if we stabilize any extensions.

Problematic example

If we accept rust-lang/rfcs#1817, the current implementation would allow both of these:

const FOO: i32 = (HasDrop {...}, 0).1;
const BAR: &'static i32 = &(HasDrop {...}, 0).1;

If we go by the rest of the language, FOO should drop the tuple after copying out its second field, whereas BAR should borrow the tuple forever, and happen to be pointing at the second tuple (resulting in no drop).
This is important even if we can't run destructors at compile-time, in order to error for FOO but not BAR.

So with the rules inferred above, we could desugar the two constants as follows:

const FOO: i32 = { let tmp = (HasDrop {...}, 0); tmp.1 /* drop(tmp) */ };
static TMP: (HasDrop, i32) = (HasDrop {...}, 0);
const BAR: &'static i32 = &TMP.1;

The distinction between the tmp local slot and the TMP static slot, and the lack of a drop for the latter, is what MIR is missing currently.

Proposed solution

@nikomatsakis and I have mulled this over, and the resulting plan is roughly as follows:

• use the existing rvalue scope rules that give the two borrowed temporaries in &f(&g()) different scopes (with the inner one living only for the duration of the outer call), with the outermost scope of a constant initializer being special: no destructors run, and its lifetime is 'static
  • this breaks some (unstable) const fn uses, e.g. id(&0) (recovered through rvalue promotion)
• for 'static scopes, MIR building would use a different kind of "slot" and not emit a drop
  • we might want to repurpose&rename Local to Slot
• MIR rvalue promotion would create such 'static "slots" for all borrows, e.g.:

// promoted MIR fragment for `&[&42]`
static0 = 42;
tmp0 = &static0;
static1 = [tmp0];
return = &static1;

• MIR borrowck doesn't require any special treatment, and it should be able to detect malformed MIR
  • e.g. a drop of a 'static slot that was borrowed would overlap with the 'static borrow
• in miri, only allow pointers into 'static slots to "escape" the constant initializer evaluation
  • eventually we could also allow pointers into allocations that were "leaked", e.g. because their owner was placed in a 'static slot: const S: &'static str = &X.to_string();

[eddyb]

I've done a crater run with the rule-enforcing side of this (i.e. non-'static temporaries from borrowck's perspective, but the MIR only differs in terms of Storage{Live,Dead}) with the rvalue promotion feature-gate lifted (not only f(&0) but even Some(&0) needed it), and the report shows a single regression, which can be traced back to the use of the unstable drop_types_in_const feature.

In fact, it's the sort of thing we want to break ASAP (and is blocking drop_types_in_const's stabilization): if some code didn't work without drop_types_in_const (i.e. it didn't fit our value-based approximation for when something needs Drop - which could be expanded to use dataflow, introspect discriminants etc. in the future), then it should never be allowed if there is any doubt that putting the code in a function won't trigger any drops to run.

[aidanhs]

As a data point, this did break some code of mine that isn't in crater, i.e. https://play.rust-lang.org/?gist=469a2c849813388f856bdacf55327d9e&version=nightly, but such is the risk of using unstable features.

[eddyb]

@aidanhs Breaking that kind of code was entirely intentional! You see, if you have:

let FOOS: &[Foo] = &[foo(&S)];

Even with no 'static restrictions, you'd get at least an error from the fact that &S doesn't live longer than the let intializer expression - it's a temporary, for the call to foo.
We now apply the same rules to static and const items, so that you don't get random &'static T where at runtime you'd have a short temporary and Drop evaluation.

For your code in particular, either replacing the OpClass::new calls with direct struct creation, or having issl! generate { const X: &[IString] = ...; X } would work under the new rules.
Also, you're using static where you actually want const, unless I'm missing something?

[aidanhs]

Yeah I get that it's consistent with behavior within functions now, I have indeed replaced it with direct struct creation. static is probably just historical from when I was using lazy_static.

[eddyb]

cc @rust-lang/wg-const-eval Do we want to just close this issue?

At this point it seems that "StorageLive but no StorageDead" is serving us relatively well, so moving to a more explicit system doesn't seem necessary.

[oli-obk]

Sgtm, I didn't know this issue still existed since we are handling everything nicely with the storage statements

[oli-obk]

cc @RalfJung

[RalfJung]

I do not understand the context of the original post here, but I agree that what we do currently seems nicer than having two kinds of "MIR slots".