Skip to content

Commit

Permalink
Clarify MIR semantics of storage statements
Browse files Browse the repository at this point in the history
  • Loading branch information
JakobDegen committed Jul 8, 2022
1 parent 7665c35 commit 4939f6c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 14 deletions.
8 changes: 7 additions & 1 deletion compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,13 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}

if self.reachable_blocks.contains(location.block) && context.is_use() {
// Uses of locals must occur while the local's storage is allocated.
// We check that the local is live whenever it is used. Technically, violating this
// restriction is only UB and not actually indicative of not well-formed MIR. This means
// that an optimization which turns MIR that already has UB into MIR that fails this
// check is not necessarily wrong. However, we have no such optimizations at the moment,
// and so we include this check anyway to help us catch bugs. If you happen to write an
// optimization that might cause this to incorrectly fire, feel free to remove this
// check.
self.storage_liveness.seek_after_primary_effect(location);
let locals_with_storage = self.storage_liveness.get();
if !locals_with_storage.contains(local) {
Expand Down
26 changes: 13 additions & 13 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,19 +237,19 @@ pub enum StatementKind<'tcx> {

/// `StorageLive` and `StorageDead` statements mark the live range of a local.
///
/// Using a local before a `StorageLive` or after a `StorageDead` is not well-formed. These
/// statements are not required. If the entire MIR body contains no `StorageLive`/`StorageDead`
/// statements for a particular local, the local is always considered live.
///
/// More precisely, the MIR validator currently does a `MaybeStorageLiveLocals` analysis to
/// check validity of each use of a local. I believe this is equivalent to requiring for every
/// use of a local, there exist at least one path from the root to that use that contains a
/// `StorageLive` more recently than a `StorageDead`.
///
/// **Needs clarification**: Is it permitted to have two `StorageLive`s without an intervening
/// `StorageDead`? Two `StorageDead`s without an intervening `StorageLive`? LLVM says poison,
/// yes. If the answer to any of these is "no," is breaking that rule UB or is it an error to
/// have a path in the CFG that might do this?
/// At any point during the execution of a function, each local is either allocated or
/// unallocated. Except as noted below, all locals except function parameters are initially
/// unallocated. `StorageLive` statements cause memory to be allocated for the local while
/// `StorageDead` statements cause the memory to be freed. Using a local in any way (not only
/// reading/writing from it) while it is unallocated is UB.
///
/// Some locals have no `StorageLive` or `StorageDead` statements within the entire MIR body.
/// These locals are implicitly allocated for the full duration of the function. There is a
/// convenience method at `rustc_mir_dataflow::storage::always_storage_live_locals` for
/// computing these locals.
///
/// If the local is already allocated, calling `StorageLive` again is UB. However, for an
/// unallocated local an additional `StorageDead` all is simply a nop.
StorageLive(Local),

/// See `StorageLive` above.
Expand Down

0 comments on commit 4939f6c

Please sign in to comment.