Auto merge of #71467 - Dylan-DPC:rollup-d1os8ug, r=Dylan-DPC

Rollup of 6 pull requests

Successful merges:

 - #71005 (Reading from the return place is fine)
 - #71198 (Const check/promotion cleanup and sanity assertion)
 - #71396 (Improve E0308 error message wording again)
 - #71452 (Remove outdated reference to interpreter snapshotting)
 - #71454 (Inline some function docs in `core::ptr`)
 - #71461 (Improve E0567 explanation)

Failed merges:

r? @ghost

src/libcore/ptr/mod.rs

@@ -76,12 +76,15 @@ use crate::intrinsics::{self, is_aligned_and_not_null, is_nonoverlapping};
 use crate::mem::{self, MaybeUninit};
 
 #[stable(feature = "rust1", since = "1.0.0")]
+#[doc(inline)]
 pub use crate::intrinsics::copy_nonoverlapping;
 
 #[stable(feature = "rust1", since = "1.0.0")]
+#[doc(inline)]
 pub use crate::intrinsics::copy;
 
 #[stable(feature = "rust1", since = "1.0.0")]
+#[doc(inline)]
 pub use crate::intrinsics::write_bytes;
 
 mod non_null;

src/librustc_error_codes/error_codes/E0308.md

@@ -12,8 +12,7 @@ let x: i32 = "I am not a number!";
 //    type `i32` assigned to variable `x`
 ```
 
-This error occurs when the compiler was unable to infer the concrete type of a
-variable. It can happen in several cases, the most common being a mismatch
-between the type that the compiler inferred for a variable based on its
-initializing expression, on the one hand, and the type the author explicitly
-assigned to the variable, on the other hand.
+This error occurs when the compiler is unable to infer the concrete type of a
+variable. It can occur in several cases, the most common being a mismatch
+between two types: the type the author explicitly assigned, and the type the
+compiler inferred.

src/librustc_error_codes/error_codes/E0567.md

@@ -6,8 +6,7 @@ Erroneous code example:
 #![feature(optin_builtin_traits)]
 
 auto trait Generic<T> {} // error!
-
-fn main() {}
+# fn main() {}
 ```
 
 Since an auto trait is implemented on all existing types, the
@@ -20,6 +19,5 @@ To fix this issue, just remove the generics:
 #![feature(optin_builtin_traits)]
 
 auto trait Generic {} // ok!
-
-fn main() {}
+# fn main() {}
 ```

src/librustc_middle/mir/interpret/allocation.rs

@@ -13,8 +13,6 @@ use super::{
     read_target_uint, write_target_uint, AllocId, InterpResult, Pointer, Scalar, ScalarMaybeUndef,
 };
 
-// NOTE: When adding new fields, make sure to adjust the `Snapshot` impl in
-// `src/librustc_mir/interpret/snapshot.rs`.
 #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
 #[derive(HashStable)]
 pub struct Allocation<Tag = (), Extra = ()> {

src/librustc_middle/mir/interpret/error.rs

@@ -361,8 +361,6 @@ pub enum UndefinedBehaviorInfo {
     InvalidUndefBytes(Option<Pointer>),
     /// Working with a local that is not currently live.
     DeadLocal,
-    /// Trying to read from the return place of a function.
-    ReadFromReturnPlace,
 }
 
 impl fmt::Debug for UndefinedBehaviorInfo {
@@ -424,7 +422,6 @@ impl fmt::Debug for UndefinedBehaviorInfo {
                 "using uninitialized data, but this operation requires initialized memory"
             ),
             DeadLocal => write!(f, "accessing a dead local variable"),
-            ReadFromReturnPlace => write!(f, "reading from return place"),
         }
     }
 }

src/librustc_middle/mir/interpret/value.rs

@@ -188,11 +188,6 @@ impl<'tcx, Tag> Scalar<Tag> {
         }
     }
 
-    #[inline]
-    pub fn null_ptr(cx: &impl HasDataLayout) -> Self {
-        Scalar::Raw { data: 0, size: cx.data_layout().pointer_size.bytes() as u8 }
-    }
-
     #[inline]
     pub fn zst() -> Self {
         Scalar::Raw { data: 0, size: 0 }

src/librustc_mir/borrow_check/type_check/mod.rs

@@ -42,7 +42,10 @@ use rustc_trait_selection::traits::{self, ObligationCause, PredicateObligations}
 use crate::dataflow::move_paths::MoveData;
 use crate::dataflow::MaybeInitializedPlaces;
 use crate::dataflow::ResultsCursor;
-use crate::transform::promote_consts::should_suggest_const_in_array_repeat_expressions_attribute;
+use crate::transform::{
+    check_consts::ConstCx,
+    promote_consts::should_suggest_const_in_array_repeat_expressions_attribute,
+};
 
 use crate::borrow_check::{
     borrow_set::BorrowSet,
@@ -1984,14 +1987,17 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                         let span = body.source_info(location).span;
                         let ty = operand.ty(body, tcx);
                         if !self.infcx.type_is_copy_modulo_regions(self.param_env, ty, span) {
+                            let ccx = ConstCx::new_with_param_env(
+                                tcx,
+                                self.mir_def_id,
+                                body,
+                                self.param_env,
+                            );
                             // To determine if `const_in_array_repeat_expressions` feature gate should
                             // be mentioned, need to check if the rvalue is promotable.
                             let should_suggest =
                                 should_suggest_const_in_array_repeat_expressions_attribute(
-                                    tcx,
-                                    self.mir_def_id,
-                                    body,
-                                    operand,
+                                    &ccx, operand,
                                 );
                             debug!("check_rvalue: should_suggest={:?}", should_suggest);
 

src/librustc_mir/interpret/eval_context.rs

@@ -628,35 +628,30 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let frame = M::init_frame_extra(self, pre_frame)?;
         self.stack_mut().push(frame);
 
-        // don't allocate at all for trivial constants
-        if body.local_decls.len() > 1 {
-            // Locals are initially uninitialized.
-            let dummy = LocalState { value: LocalValue::Uninitialized, layout: Cell::new(None) };
-            let mut locals = IndexVec::from_elem(dummy, &body.local_decls);
-            // Return place is handled specially by the `eval_place` functions, and the
-            // entry in `locals` should never be used. Make it dead, to be sure.
-            locals[mir::RETURN_PLACE].value = LocalValue::Dead;
-            // Now mark those locals as dead that we do not want to initialize
-            match self.tcx.def_kind(instance.def_id()) {
-                // statics and constants don't have `Storage*` statements, no need to look for them
-                //
-                // FIXME: The above is likely untrue. See
-                // <https://github.com/rust-lang/rust/pull/70004#issuecomment-602022110>. Is it
-                // okay to ignore `StorageDead`/`StorageLive` annotations during CTFE?
-                Some(DefKind::Static | DefKind::Const | DefKind::AssocConst) => {}
-                _ => {
-                    // Mark locals that use `Storage*` annotations as dead on function entry.
-                    let always_live = AlwaysLiveLocals::new(self.body());
-                    for local in locals.indices() {
-                        if !always_live.contains(local) {
-                            locals[local].value = LocalValue::Dead;
-                        }
+        // Locals are initially uninitialized.
+        let dummy = LocalState { value: LocalValue::Uninitialized, layout: Cell::new(None) };
+        let mut locals = IndexVec::from_elem(dummy, &body.local_decls);
+
+        // Now mark those locals as dead that we do not want to initialize
+        match self.tcx.def_kind(instance.def_id()) {
+            // statics and constants don't have `Storage*` statements, no need to look for them
+            //
+            // FIXME: The above is likely untrue. See
+            // <https://github.com/rust-lang/rust/pull/70004#issuecomment-602022110>. Is it
+            // okay to ignore `StorageDead`/`StorageLive` annotations during CTFE?
+            Some(DefKind::Static | DefKind::Const | DefKind::AssocConst) => {}
+            _ => {
+                // Mark locals that use `Storage*` annotations as dead on function entry.
+                let always_live = AlwaysLiveLocals::new(self.body());
+                for local in locals.indices() {
+                    if !always_live.contains(local) {
+                        locals[local].value = LocalValue::Dead;
                     }
                 }
             }
-            // done
-            self.frame_mut().locals = locals;
         }
+        // done
+        self.frame_mut().locals = locals;
 
         M::after_stack_push(self)?;
         info!("ENTERING({}) {}", self.frame_idx(), self.frame().instance);
@@ -734,6 +729,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let frame =
             self.stack_mut().pop().expect("tried to pop a stack frame, but there were none");
 
+        if !unwinding {
+            // Copy the return value to the caller's stack frame.
+            if let Some(return_place) = frame.return_place {
+                let op = self.access_local(&frame, mir::RETURN_PLACE, None)?;
+                self.copy_op_transmute(op, return_place)?;
+                self.dump_place(*return_place);
+            } else {
+                throw_ub!(Unreachable);
+            }
+        }
+
         // Now where do we jump next?
 
         // Usually we want to clean up (deallocate locals), but in a few rare cases we don't.
@@ -759,7 +765,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             self.deallocate_local(local.value)?;
         }
 
-        let return_place = frame.return_place;
         if M::after_stack_pop(self, frame, unwinding)? == StackPopJump::NoJump {
             // The hook already did everything.
             // We want to skip the `info!` below, hence early return.
@@ -772,25 +777,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             self.unwind_to_block(unwind);
         } else {
             // Follow the normal return edge.
-            // Validate the return value. Do this after deallocating so that we catch dangling
-            // references.
-            if let Some(return_place) = return_place {
-                if M::enforce_validity(self) {
-                    // Data got changed, better make sure it matches the type!
-                    // It is still possible that the return place held invalid data while
-                    // the function is running, but that's okay because nobody could have
-                    // accessed that same data from the "outside" to observe any broken
-                    // invariant -- that is, unless a function somehow has a ptr to
-                    // its return place... but the way MIR is currently generated, the
-                    // return place is always a local and then this cannot happen.
-                    self.validate_operand(self.place_to_op(return_place)?)?;
-                }
-            } else {
-                // Uh, that shouldn't happen... the function did not intend to return
-                throw_ub!(Unreachable);
-            }
-
-            // Jump to new block -- *after* validation so that the spans make more sense.
             if let Some(ret) = next_block {
                 self.return_to_block(ret)?;
             }

src/librustc_mir/interpret/operand.rs

@@ -419,7 +419,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         local: mir::Local,
         layout: Option<TyAndLayout<'tcx>>,
     ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
-        assert_ne!(local, mir::RETURN_PLACE);
         let layout = self.layout_of_local(frame, local, layout)?;
         let op = if layout.is_zst() {
             // Do not read from ZST, they might not be initialized
@@ -454,16 +453,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         place: mir::Place<'tcx>,
         layout: Option<TyAndLayout<'tcx>>,
     ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
-        let base_op = match place.local {
-            mir::RETURN_PLACE => throw_ub!(ReadFromReturnPlace),
-            local => {
-                // Do not use the layout passed in as argument if the base we are looking at
-                // here is not the entire place.
-                let layout = if place.projection.is_empty() { layout } else { None };
-
-                self.access_local(self.frame(), local, layout)?
-            }
-        };
+        // Do not use the layout passed in as argument if the base we are looking at
+        // here is not the entire place.
+        let layout = if place.projection.is_empty() { layout } else { None };
+
+        let base_op = self.access_local(self.frame(), place.local, layout)?;
 
         let op = place
             .projection

src/librustc_mir/interpret/place.rs

@@ -135,12 +135,6 @@ impl<Tag> MemPlace<Tag> {
         MemPlace { ptr, align, meta: MemPlaceMeta::None }
     }
 
-    /// Produces a Place that will error if attempted to be read from or written to
-    #[inline(always)]
-    fn null(cx: &impl HasDataLayout) -> Self {
-        Self::from_scalar_ptr(Scalar::null_ptr(cx), Align::from_bytes(1).unwrap())
-    }
-
     #[inline(always)]
     pub fn from_ptr(ptr: Pointer<Tag>, align: Align) -> Self {
         Self::from_scalar_ptr(ptr.into(), align)
@@ -260,12 +254,6 @@ impl<'tcx, Tag: ::std::fmt::Debug + Copy> OpTy<'tcx, Tag> {
 }
 
 impl<Tag: ::std::fmt::Debug> Place<Tag> {
-    /// Produces a Place that will error if attempted to be read from or written to
-    #[inline(always)]
-    fn null(cx: &impl HasDataLayout) -> Self {
-        Place::Ptr(MemPlace::null(cx))
-    }
-
     #[inline]
     pub fn assert_mem_place(self) -> MemPlace<Tag> {
         match self {
@@ -641,35 +629,10 @@ where
         &mut self,
         place: mir::Place<'tcx>,
     ) -> InterpResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
-        let mut place_ty = match place.local {
-            mir::RETURN_PLACE => {
-                // `return_place` has the *caller* layout, but we want to use our
-                // `layout to verify our assumption. The caller will validate
-                // their layout on return.
-                PlaceTy {
-                    place: match self.frame().return_place {
-                        Some(p) => *p,
-                        // Even if we don't have a return place, we sometimes need to
-                        // create this place, but any attempt to read from / write to it
-                        // (even a ZST read/write) needs to error, so let us make this
-                        // a NULL place.
-                        //
-                        // FIXME: Ideally we'd make sure that the place projections also
-                        // bail out.
-                        None => Place::null(&*self),
-                    },
-                    layout: self.layout_of(
-                        self.subst_from_current_frame_and_normalize_erasing_regions(
-                            self.frame().body.return_ty(),
-                        ),
-                    )?,
-                }
-            }
-            local => PlaceTy {
-                // This works even for dead/uninitialized locals; we check further when writing
-                place: Place::Local { frame: self.frame_idx(), local },
-                layout: self.layout_of_local(self.frame(), local, None)?,
-            },
+        let mut place_ty = PlaceTy {
+            // This works even for dead/uninitialized locals; we check further when writing
+            place: Place::Local { frame: self.frame_idx(), local: place.local },
+            layout: self.layout_of_local(self.frame(), place.local, None)?,
         };
 
         for elem in place.projection.iter() {

src/librustc_mir/interpret/terminator.rs

@@ -19,7 +19,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         use rustc_middle::mir::TerminatorKind::*;
         match terminator.kind {
             Return => {
-                self.frame().return_place.map(|r| self.dump_place(*r));
                 self.pop_stack_frame(/* unwinding */ false)?
             }
 

src/librustc_mir/transform/check_consts/mod.rs

@@ -20,20 +20,29 @@ pub mod validation;
 
 /// Information about the item currently being const-checked, as well as a reference to the global
 /// context.
-pub struct Item<'mir, 'tcx> {
+pub struct ConstCx<'mir, 'tcx> {
     pub body: &'mir mir::Body<'tcx>,
     pub tcx: TyCtxt<'tcx>,
     pub def_id: DefId,
     pub param_env: ty::ParamEnv<'tcx>,
     pub const_kind: Option<ConstKind>,
 }
 
-impl Item<'mir, 'tcx> {
+impl ConstCx<'mir, 'tcx> {
     pub fn new(tcx: TyCtxt<'tcx>, def_id: DefId, body: &'mir mir::Body<'tcx>) -> Self {
         let param_env = tcx.param_env(def_id);
+        Self::new_with_param_env(tcx, def_id, body, param_env)
+    }
+
+    pub fn new_with_param_env(
+        tcx: TyCtxt<'tcx>,
+        def_id: DefId,
+        body: &'mir mir::Body<'tcx>,
+        param_env: ty::ParamEnv<'tcx>,
+    ) -> Self {
         let const_kind = ConstKind::for_item(tcx, def_id);
 
-        Item { body, tcx, def_id, param_env, const_kind }
+        ConstCx { body, tcx, def_id, param_env, const_kind }
     }
 
     /// Returns the kind of const context this `Item` represents (`const`, `static`, etc.).