diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 3892d1920cef7..2e47cf8921073 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -15,7 +15,7 @@ use rustc_middle::ty::layout::{ use rustc_middle::ty::{ self, query::TyCtxtAt, subst::SubstsRef, ParamEnv, Ty, TyCtxt, TypeFoldable, }; -use rustc_mir_dataflow::storage::always_live_locals; +use rustc_mir_dataflow::storage::always_storage_live_locals; use rustc_query_system::ich::StableHashingContext; use rustc_session::Limit; use rustc_span::{Pos, Span}; @@ -707,7 +707,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let mut locals = IndexVec::from_elem(dummy, &body.local_decls); // Now mark those locals as live that have no `Storage*` annotations. - let always_live = always_live_locals(self.body()); + let always_live = always_storage_live_locals(self.body()); for local in locals.indices() { if always_live.contains(local) { locals[local].value = LocalValue::Live(Operand::Immediate(Immediate::Uninit)); diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index 56ecc5535285b..d3bf6b49f1205 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -15,7 +15,7 @@ use rustc_middle::ty::fold::BottomUpFolder; use rustc_middle::ty::subst::Subst; use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeFoldable, TypeVisitable}; use rustc_mir_dataflow::impls::MaybeStorageLive; -use rustc_mir_dataflow::storage::always_live_locals; +use rustc_mir_dataflow::storage::always_storage_live_locals; use rustc_mir_dataflow::{Analysis, ResultsCursor}; use rustc_target::abi::{Size, VariantIdx}; @@ -49,7 +49,7 @@ impl<'tcx> MirPass<'tcx> for Validator { let param_env = tcx.param_env(def_id); let mir_phase = self.mir_phase; - let always_live_locals = always_live_locals(body); + let always_live_locals = always_storage_live_locals(body); let storage_liveness = MaybeStorageLive::new(always_live_locals) .into_engine(tcx, body) .iterate_to_fixpoint() @@ -206,7 +206,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) { diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index dcc37c565c9b6..45fc5f24a607a 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -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. diff --git a/compiler/rustc_mir_dataflow/src/storage.rs b/compiler/rustc_mir_dataflow/src/storage.rs index 4a354c4c65b08..566c9d2d5054e 100644 --- a/compiler/rustc_mir_dataflow/src/storage.rs +++ b/compiler/rustc_mir_dataflow/src/storage.rs @@ -7,7 +7,7 @@ use rustc_middle::mir::{self, Local}; // // FIXME: Currently, we need to traverse the entire MIR to compute this. We should instead store it // as a field in the `LocalDecl` for each `Local`. -pub fn always_live_locals(body: &mir::Body<'_>) -> BitSet { +pub fn always_storage_live_locals(body: &mir::Body<'_>) -> BitSet { let mut always_live_locals = BitSet::new_filled(body.local_decls.len()); for block in body.basic_blocks() { diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index 9078b8a1cb7cd..9fdea8359671f 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -67,7 +67,7 @@ use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt}; use rustc_mir_dataflow::impls::{ MaybeBorrowedLocals, MaybeLiveLocals, MaybeRequiresStorage, MaybeStorageLive, }; -use rustc_mir_dataflow::storage; +use rustc_mir_dataflow::storage::always_storage_live_locals; use rustc_mir_dataflow::{self, Analysis}; use rustc_target::abi::VariantIdx; use rustc_target::spec::PanicStrategy; @@ -1379,7 +1379,7 @@ impl<'tcx> MirPass<'tcx> for StateTransform { }, ); - let always_live_locals = storage::always_live_locals(&body); + let always_live_locals = always_storage_live_locals(&body); let liveness_info = locals_live_across_suspend_points(tcx, body, &always_live_locals, movable); diff --git a/library/alloc/src/borrow.rs b/library/alloc/src/borrow.rs index 7a79fb77dea55..904a53bb4acc7 100644 --- a/library/alloc/src/borrow.rs +++ b/library/alloc/src/borrow.rs @@ -60,7 +60,7 @@ pub trait ToOwned { /// Uses borrowed data to replace owned data, usually by cloning. /// - /// This is borrow-generalized version of `Clone::clone_from`. + /// This is borrow-generalized version of [`Clone::clone_from`]. /// /// # Examples ///