Auto merge of rust-lang#99309 - RalfJung:no-large-copies, r=oli-obk

interpret: make some large types not Copy

Also remove some unused trait impls (mostly HashStable).

This didn't find any unnecessary copies that I managed to avoid, but it might still be better to require explicit clone for these types? Not sure.

r? `@oli-obk`

Author: bors

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -2,10 +2,8 @@ use std::cell::Cell;
 use std::fmt;
 use std::mem;
 
-use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_hir::{self as hir, def_id::DefId, definitions::DefPathData};
 use rustc_index::vec::IndexVec;
-use rustc_macros::HashStable;
 use rustc_middle::mir;
 use rustc_middle::mir::interpret::{InterpError, InvalidProgramInfo};
 use rustc_middle::ty::layout::{
@@ -16,7 +14,6 @@ use rustc_middle::ty::{
     self, query::TyCtxtAt, subst::SubstsRef, ParamEnv, Ty, TyCtxt, TypeFoldable,
 };
 use rustc_mir_dataflow::storage::always_storage_live_locals;
-use rustc_query_system::ich::StableHashingContext;
 use rustc_session::Limit;
 use rustc_span::{Pos, Span};
 use rustc_target::abi::{call::FnAbi, Align, HasDataLayout, Size, TargetDataLayout};
@@ -142,7 +139,7 @@ pub struct FrameInfo<'tcx> {
 }
 
 /// Unwind information.
-#[derive(Clone, Copy, Eq, PartialEq, Debug, HashStable)]
+#[derive(Clone, Copy, Eq, PartialEq, Debug)]
 pub enum StackPopUnwind {
     /// The cleanup block.
     Cleanup(mir::BasicBlock),
@@ -152,7 +149,7 @@ pub enum StackPopUnwind {
     NotAllowed,
 }
 
-#[derive(Clone, Copy, Eq, PartialEq, Debug, HashStable)] // Miri debug-prints these
+#[derive(Clone, Copy, Eq, PartialEq, Debug)] // Miri debug-prints these
 pub enum StackPopCleanup {
     /// Jump to the next block in the caller, or cause UB if None (that's a function
     /// that may never return). Also store layout of return place so
@@ -168,16 +165,15 @@ pub enum StackPopCleanup {
 }
 
 /// State of a local variable including a memoized layout
-#[derive(Clone, Debug, PartialEq, Eq, HashStable)]
+#[derive(Clone, Debug)]
 pub struct LocalState<'tcx, Tag: Provenance = AllocId> {
     pub value: LocalValue<Tag>,
     /// Don't modify if `Some`, this is only used to prevent computing the layout twice
-    #[stable_hasher(ignore)]
     pub layout: Cell<Option<TyAndLayout<'tcx>>>,
 }
 
 /// Current value of a local variable
-#[derive(Copy, Clone, PartialEq, Eq, HashStable, Debug)] // Miri debug-prints these
+#[derive(Copy, Clone, Debug)] // Miri debug-prints these
 pub enum LocalValue<Tag: Provenance = AllocId> {
     /// This local is not currently alive, and cannot be used at all.
     Dead,
@@ -678,7 +674,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             body,
             loc: Err(body.span), // Span used for errors caused during preamble.
             return_to_block,
-            return_place: *return_place,
+            return_place: return_place.clone(),
             // empty local array, we fill it in below, after we are inside the stack frame and
             // all methods actually know about the frame
             locals: IndexVec::new(),
@@ -799,7 +795,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             let op = self
                 .local_to_op(self.frame(), mir::RETURN_PLACE, None)
                 .expect("return place should always be live");
-            let dest = self.frame().return_place;
+            let dest = self.frame().return_place.clone();
             let err = self.copy_op(&op, &dest, /*allow_transmute*/ true);
             trace!("return value: {:?}", self.dump_place(*dest));
             // We delay actually short-circuiting on this error until *after* the stack frame is
@@ -1021,31 +1017,3 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> std::fmt::Debug
         }
     }
 }
-
-impl<'ctx, 'mir, 'tcx, Tag: Provenance, Extra> HashStable<StableHashingContext<'ctx>>
-    for Frame<'mir, 'tcx, Tag, Extra>
-where
-    Extra: HashStable<StableHashingContext<'ctx>>,
-    Tag: HashStable<StableHashingContext<'ctx>>,
-{
-    fn hash_stable(&self, hcx: &mut StableHashingContext<'ctx>, hasher: &mut StableHasher) {
-        // Exhaustive match on fields to make sure we forget no field.
-        let Frame {
-            body,
-            instance,
-            return_to_block,
-            return_place,
-            locals,
-            loc,
-            extra,
-            tracing_span: _,
-        } = self;
-        body.hash_stable(hcx, hasher);
-        instance.hash_stable(hcx, hasher);
-        return_to_block.hash_stable(hcx, hasher);
-        return_place.hash_stable(hcx, hasher);
-        locals.hash_stable(hcx, hasher);
-        loc.hash_stable(hcx, hasher);
-        extra.hash_stable(hcx, hasher);
-    }
-}

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -455,8 +455,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
                 for i in 0..dest_len {
                     let place = self.mplace_index(&dest, i)?;
-                    let value =
-                        if i == index { *elem } else { self.mplace_index(&input, i)?.into() };
+                    let value = if i == index {
+                        elem.clone()
+                    } else {
+                        self.mplace_index(&input, i)?.into()
+                    };
                     self.copy_op(&value, &place.into(), /*allow_transmute*/ false)?;
                 }
             }

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -4,7 +4,6 @@
 use std::fmt::Write;
 
 use rustc_hir::def::Namespace;
-use rustc_macros::HashStable;
 use rustc_middle::ty::layout::{LayoutOf, PrimitiveExt, TyAndLayout};
 use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter, Printer};
 use rustc_middle::ty::{ConstInt, DelaySpanBugEmitted, Ty};
@@ -25,7 +24,7 @@ use super::{
 /// operations and wide pointers. This idea was taken from rustc's codegen.
 /// In particular, thanks to `ScalarPair`, arithmetic operations and casts can be entirely
 /// defined on `Immediate`, and do not have to work with a `Place`.
-#[derive(Copy, Clone, PartialEq, Eq, HashStable, Hash, Debug)]
+#[derive(Copy, Clone, Debug)]
 pub enum Immediate<Tag: Provenance = AllocId> {
     /// A single scalar value (must have *initialized* `Scalar` ABI).
     /// FIXME: we also currently often use this for ZST.
@@ -112,7 +111,7 @@ impl<'tcx, Tag: Provenance> Immediate<Tag> {
 
 // ScalarPair needs a type to interpret, so we often have an immediate and a type together
 // as input for binary and cast operations.
-#[derive(Copy, Clone, Debug)]
+#[derive(Clone, Debug)]
 pub struct ImmTy<'tcx, Tag: Provenance = AllocId> {
     imm: Immediate<Tag>,
     pub layout: TyAndLayout<'tcx>,
@@ -182,13 +181,16 @@ impl<'tcx, Tag: Provenance> std::ops::Deref for ImmTy<'tcx, Tag> {
 /// An `Operand` is the result of computing a `mir::Operand`. It can be immediate,
 /// or still in memory. The latter is an optimization, to delay reading that chunk of
 /// memory and to avoid having to store arbitrary-sized data here.
-#[derive(Copy, Clone, PartialEq, Eq, HashStable, Hash, Debug)]
+#[derive(Copy, Clone, Debug)]
 pub enum Operand<Tag: Provenance = AllocId> {
     Immediate(Immediate<Tag>),
     Indirect(MemPlace<Tag>),
 }
 
-#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
+#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
+rustc_data_structures::static_assert_size!(Operand, 64);
+
+#[derive(Clone, Debug)]
 pub struct OpTy<'tcx, Tag: Provenance = AllocId> {
     op: Operand<Tag>, // Keep this private; it helps enforce invariants.
     pub layout: TyAndLayout<'tcx>,

compiler/rustc_const_eval/src/interpret/place.rs

@@ -5,7 +5,6 @@
 use std::hash::Hash;
 
 use rustc_ast::Mutability;
-use rustc_macros::HashStable;
 use rustc_middle::mir;
 use rustc_middle::ty;
 use rustc_middle::ty::layout::{LayoutOf, PrimitiveExt, TyAndLayout};
@@ -17,7 +16,7 @@ use super::{
     Pointer, Provenance, Scalar, ScalarMaybeUninit,
 };
 
-#[derive(Copy, Clone, Hash, PartialEq, Eq, HashStable, Debug)]
+#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
 /// Information required for the sound usage of a `MemPlace`.
 pub enum MemPlaceMeta<Tag: Provenance = AllocId> {
     /// The unsized payload (e.g. length for slices or vtable pointer for trait objects).
@@ -47,7 +46,7 @@ impl<Tag: Provenance> MemPlaceMeta<Tag> {
     }
 }
 
-#[derive(Copy, Clone, Hash, PartialEq, Eq, HashStable, Debug)]
+#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
 pub struct MemPlace<Tag: Provenance = AllocId> {
     /// The pointer can be a pure integer, with the `None` tag.
     pub ptr: Pointer<Option<Tag>>,
@@ -60,7 +59,22 @@ pub struct MemPlace<Tag: Provenance = AllocId> {
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
 rustc_data_structures::static_assert_size!(MemPlace, 40);
 
-#[derive(Copy, Clone, Hash, PartialEq, Eq, HashStable, Debug)]
+/// A MemPlace with its layout. Constructing it is only possible in this module.
+#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
+pub struct MPlaceTy<'tcx, Tag: Provenance = AllocId> {
+    mplace: MemPlace<Tag>,
+    pub layout: TyAndLayout<'tcx>,
+    /// rustc does not have a proper way to represent the type of a field of a `repr(packed)` struct:
+    /// it needs to have a different alignment than the field type would usually have.
+    /// So we represent this here with a separate field that "overwrites" `layout.align`.
+    /// This means `layout.align` should never be used for a `MPlaceTy`!
+    pub align: Align,
+}
+
+#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
+rustc_data_structures::static_assert_size!(MPlaceTy<'_>, 64);
+
+#[derive(Copy, Clone, Debug)]
 pub enum Place<Tag: Provenance = AllocId> {
     /// A place referring to a value allocated in the `Memory` system.
     Ptr(MemPlace<Tag>),
@@ -73,7 +87,7 @@ pub enum Place<Tag: Provenance = AllocId> {
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
 rustc_data_structures::static_assert_size!(Place, 48);
 
-#[derive(Copy, Clone, Debug)]
+#[derive(Clone, Debug)]
 pub struct PlaceTy<'tcx, Tag: Provenance = AllocId> {
     place: Place<Tag>, // Keep this private; it helps enforce invariants.
     pub layout: TyAndLayout<'tcx>,
@@ -95,21 +109,6 @@ impl<'tcx, Tag: Provenance> std::ops::Deref for PlaceTy<'tcx, Tag> {
     }
 }
 
-/// A MemPlace with its layout. Constructing it is only possible in this module.
-#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
-pub struct MPlaceTy<'tcx, Tag: Provenance = AllocId> {
-    mplace: MemPlace<Tag>,
-    pub layout: TyAndLayout<'tcx>,
-    /// rustc does not have a proper way to represent the type of a field of a `repr(packed)` struct:
-    /// it needs to have a different alignment than the field type would usually have.
-    /// So we represent this here with a separate field that "overwrites" `layout.align`.
-    /// This means `layout.align` should never be used for a `MPlaceTy`!
-    pub align: Align,
-}
-
-#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
-rustc_data_structures::static_assert_size!(MPlaceTy<'_>, 64);
-
 impl<'tcx, Tag: Provenance> std::ops::Deref for MPlaceTy<'tcx, Tag> {
     type Target = MemPlace<Tag>;
     #[inline(always)]

compiler/rustc_const_eval/src/interpret/projection.rs

@@ -157,7 +157,7 @@ where
         variant: VariantIdx,
     ) -> InterpResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
         // Downcast just changes the layout
-        let mut base = *base;
+        let mut base = base.clone();
         base.layout = base.layout.for_variant(self, variant);
         Ok(base)
     }
@@ -168,7 +168,7 @@ where
         variant: VariantIdx,
     ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
         // Downcast just changes the layout
-        let mut base = *base;
+        let mut base = base.clone();
         base.layout = base.layout.for_variant(self, variant);
         Ok(base)
     }
@@ -350,7 +350,7 @@ where
         use rustc_middle::mir::ProjectionElem::*;
         Ok(match proj_elem {
             OpaqueCast(ty) => {
-                let mut place = *base;
+                let mut place = base.clone();
                 place.layout = self.layout_of(ty)?;
                 place
             }
@@ -379,7 +379,7 @@ where
         use rustc_middle::mir::ProjectionElem::*;
         Ok(match proj_elem {
             OpaqueCast(ty) => {
-                let mut op = *base;
+                let mut op = base.clone();
                 op.layout = self.layout_of(ty)?;
                 op
             }

compiler/rustc_const_eval/src/interpret/terminator.rs

@@ -444,7 +444,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                             trace!("eval_fn_call: Will pass last argument by untupling");
                             Cow::from(
                                 args.iter()
-                                    .map(|&a| Ok(a))
+                                    .map(|a| Ok(a.clone()))
                                     .chain(
                                         (0..untuple_arg.layout.fields.count())
                                             .map(|i| self.operand_field(untuple_arg, i)),
@@ -525,7 +525,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // We have to implement all "object safe receivers". So we have to go search for a
                 // pointer or `dyn Trait` type, but it could be wrapped in newtypes. So recursively
                 // unwrap those newtypes until we are there.
-                let mut receiver = args[0];
+                let mut receiver = args[0].clone();
                 let receiver_place = loop {
                     match receiver.layout.ty.kind() {
                         ty::Ref(..) | ty::RawPtr(..) => break self.deref_operand(&receiver)?,

compiler/rustc_const_eval/src/interpret/visitor.rs

@@ -13,7 +13,7 @@ use super::{InterpCx, MPlaceTy, Machine, OpTy, PlaceTy};
 /// A thing that we can project into, and that has a layout.
 /// This wouldn't have to depend on `Machine` but with the current type inference,
 /// that's just more convenient to work with (avoids repeating all the `Machine` bounds).
-pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Copy {
+pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized {
     /// Gets this value's layout.
     fn layout(&self) -> TyAndLayout<'tcx>;
 
@@ -54,7 +54,7 @@ pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Copy {
 /// A thing that we can project into given *mutable* access to `ecx`, and that has a layout.
 /// This wouldn't have to depend on `Machine` but with the current type inference,
 /// that's just more convenient to work with (avoids repeating all the `Machine` bounds).
-pub trait ValueMut<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Copy {
+pub trait ValueMut<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized {
     /// Gets this value's layout.
     fn layout(&self) -> TyAndLayout<'tcx>;
 
@@ -106,12 +106,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> Value<'mir, 'tcx, M> for OpTy<'tc
         &self,
         _ecx: &InterpCx<'mir, 'tcx, M>,
     ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
-        Ok(*self)
+        Ok(self.clone())
     }
 
     #[inline(always)]
     fn from_op(op: &OpTy<'tcx, M::PointerTag>) -> Self {
-        *op
+        op.clone()
     }
 
     #[inline(always)]
@@ -146,20 +146,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueMut<'mir, 'tcx, M>
         &self,
         _ecx: &InterpCx<'mir, 'tcx, M>,
     ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
-        Ok(*self)
+        Ok(self.clone())
     }
 
     #[inline(always)]
     fn to_op_for_proj(
         &self,
         _ecx: &mut InterpCx<'mir, 'tcx, M>,
     ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
-        Ok(*self)
+        Ok(self.clone())
     }
 
     #[inline(always)]
     fn from_op(op: &OpTy<'tcx, M::PointerTag>) -> Self {
-        *op
+        op.clone()
     }
 
     #[inline(always)]

compiler/rustc_mir_transform/src/const_prop.rs

@@ -516,7 +516,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         let l = self.use_ecx(|this| this.ecx.read_immediate(&this.ecx.eval_operand(left, None)?));
         // Check for exceeding shifts *even if* we cannot evaluate the LHS.
         if op == BinOp::Shr || op == BinOp::Shl {
-            let r = r?;
+            let r = r.clone()?;
             // We need the type of the LHS. We cannot use `place_layout` as that is the type
             // of the result, which for checked binops is not the same!
             let left_ty = left.ty(self.local_decls, self.tcx);

compiler/rustc_mir_transform/src/const_prop_lint.rs

@@ -584,7 +584,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         });
         // Check for exceeding shifts *even if* we cannot evaluate the LHS.
         if op == BinOp::Shr || op == BinOp::Shl {
-            let r = r?;
+            let r = r.clone()?;
             // We need the type of the LHS. We cannot use `place_layout` as that is the type
             // of the result, which for checked binops is not the same!
             let left_ty = left.ty(self.local_decls, self.tcx);
@@ -616,10 +616,10 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
             }
         }
 
-        if let (Some(l), Some(r)) = (&l, &r) {
+        if let (Some(l), Some(r)) = (l, r) {
             // The remaining operators are handled through `overflowing_binary_op`.
             if self.use_ecx(source_info, |this| {
-                let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
+                let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, &l, &r)?;
                 Ok(overflow)
             })? {
                 self.report_assert_as_lint(