Auto merge of rust-lang#99082 - matthiaskrgr:rollup-nouwsh7, r=matthi…

…askrgr

Rollup of 3 pull requests

Successful merges:

 - rust-lang#99022 (MIR dataflow: Rename function to `always_storage_live_locals`)
 - rust-lang#99050 (Clarify MIR semantics of storage statements)
 - rust-lang#99067 (Intra-doc-link-ify reference to Clone::clone_from)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

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));

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) {

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.

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<Local> {
+pub fn always_storage_live_locals(body: &mir::Body<'_>) -> BitSet<Local> {
     let mut always_live_locals = BitSet::new_filled(body.local_decls.len());
 
     for block in body.basic_blocks() {

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);

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
     ///