avoid manual Debug impls by adding extra Provenance bounds to types

I wish the derive macro would support adding extra where clauses...

Author: RalfJung

Diff for: compiler/rustc_middle/src/mir/interpret/pointer.rs

@@ -86,7 +86,9 @@ impl<T: HasDataLayout> PointerArithmetic for T {}
 /// This trait abstracts over the kind of provenance that is associated with a `Pointer`. It is
 /// mostly opaque; the `Machine` trait extends it with some more operations that also have access to
 /// some global state.
-pub trait Provenance: Copy {
+/// We don't actually care about this `Debug` bound (we use `Provenance::fmt` to format the entire
+/// pointer), but `derive` adds some unecessary bounds.
+pub trait Provenance: Copy + fmt::Debug {
     /// Says whether the `offset` field of `Pointer`s with this provenance is the actual physical address.
     /// If `true, ptr-to-int casts work by simply discarding the provenance.
     /// If `false`, ptr-to-int casts are not supported. The offset *must* be relative in that case.
@@ -142,14 +144,14 @@ static_assert_size!(Pointer, 16);
 // all the Miri types.
 impl<Tag: Provenance> fmt::Debug for Pointer<Tag> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        Tag::fmt(self, f)
+        Provenance::fmt(self, f)
     }
 }
 
 impl<Tag: Provenance> fmt::Debug for Pointer<Option<Tag>> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self.provenance {
-            Some(tag) => Tag::fmt(&Pointer::new(tag, self.offset), f),
+            Some(tag) => Provenance::fmt(&Pointer::new(tag, self.offset), f),
             None => write!(f, "0x{:x}", self.offset.bytes()),
         }
     }

Diff for: compiler/rustc_mir/src/interpret/eval_context.rs

@@ -80,7 +80,7 @@ impl Drop for SpanGuard {
 }
 
 /// A stack frame.
-pub struct Frame<'mir, 'tcx, Tag = AllocId, Extra = ()> {
+pub struct Frame<'mir, 'tcx, Tag: Provenance = AllocId, Extra = ()> {
     ////////////////////////////////////////////////////////////////////////////////
     // Function and callsite information
     ////////////////////////////////////////////////////////////////////////////////
@@ -161,16 +161,16 @@ pub enum StackPopCleanup {
 
 /// State of a local variable including a memoized layout
 #[derive(Clone, PartialEq, Eq, HashStable)]
-pub struct LocalState<'tcx, Tag = AllocId> {
+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)]
-pub enum LocalValue<Tag = AllocId> {
+#[derive(Copy, Clone, PartialEq, Eq, HashStable, Debug)] // Miri debug-prints these
+pub enum LocalValue<Tag: Provenance = AllocId> {
     /// This local is not currently alive, and cannot be used at all.
     Dead,
     /// This local is alive but not yet initialized. It can be written to
@@ -186,19 +186,7 @@ pub enum LocalValue<Tag = AllocId> {
     Live(Operand<Tag>),
 }
 
-impl<Tag: Provenance> std::fmt::Debug for LocalValue<Tag> {
-    // Miri debug-prints these
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        use LocalValue::*;
-        match self {
-            Dead => f.debug_tuple("Dead").finish(),
-            Uninitialized => f.debug_tuple("Uninitialized").finish(),
-            Live(o) => f.debug_tuple("Live").field(o).finish(),
-        }
-    }
-}
-
-impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> {
+impl<'tcx, Tag: Provenance + 'static> LocalState<'tcx, Tag> {
     /// Read the local's value or error if the local is not yet live or not live anymore.
     ///
     /// Note: This may only be invoked from the `Machine::access_local` hook and not from
@@ -232,7 +220,7 @@ impl<'tcx, Tag: Copy + 'static> LocalState<'tcx, Tag> {
     }
 }
 
-impl<'mir, 'tcx, Tag> Frame<'mir, 'tcx, Tag> {
+impl<'mir, 'tcx, Tag: Provenance> Frame<'mir, 'tcx, Tag> {
     pub fn with_extra<Extra>(self, extra: Extra) -> Frame<'mir, 'tcx, Tag, Extra> {
         Frame {
             body: self.body,
@@ -247,7 +235,7 @@ impl<'mir, 'tcx, Tag> Frame<'mir, 'tcx, Tag> {
     }
 }
 
-impl<'mir, 'tcx, Tag, Extra> Frame<'mir, 'tcx, Tag, Extra> {
+impl<'mir, 'tcx, Tag: Provenance, Extra> Frame<'mir, 'tcx, Tag, Extra> {
     /// Get the current location within the Frame.
     ///
     /// If this is `Err`, we are not currently executing any particular statement in
@@ -1024,7 +1012,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> std::fmt::Debug
     }
 }
 
-impl<'ctx, 'mir, 'tcx, Tag, Extra> HashStable<StableHashingContext<'ctx>>
+impl<'ctx, 'mir, 'tcx, Tag: Provenance, Extra> HashStable<StableHashingContext<'ctx>>
     for Frame<'mir, 'tcx, Tag, Extra>
 where
     Extra: HashStable<StableHashingContext<'ctx>>,

Diff for: compiler/rustc_mir/src/interpret/operand.rs

@@ -27,40 +27,30 @@ 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)]
-pub enum Immediate<Tag = AllocId> {
+#[derive(Copy, Clone, PartialEq, Eq, HashStable, Hash, Debug)]
+pub enum Immediate<Tag: Provenance = AllocId> {
     Scalar(ScalarMaybeUninit<Tag>),
     ScalarPair(ScalarMaybeUninit<Tag>, ScalarMaybeUninit<Tag>),
 }
 
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
 rustc_data_structures::static_assert_size!(Immediate, 56);
 
-impl<Tag: Provenance> std::fmt::Debug for Immediate<Tag> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        use Immediate::*;
-        match self {
-            Scalar(s) => f.debug_tuple("Scalar").field(s).finish(),
-            ScalarPair(s1, s2) => f.debug_tuple("ScalarPair").field(s1).field(s2).finish(),
-        }
-    }
-}
-
-impl<Tag> From<ScalarMaybeUninit<Tag>> for Immediate<Tag> {
+impl<Tag: Provenance> From<ScalarMaybeUninit<Tag>> for Immediate<Tag> {
     #[inline(always)]
     fn from(val: ScalarMaybeUninit<Tag>) -> Self {
         Immediate::Scalar(val)
     }
 }
 
-impl<Tag> From<Scalar<Tag>> for Immediate<Tag> {
+impl<Tag: Provenance> From<Scalar<Tag>> for Immediate<Tag> {
     #[inline(always)]
     fn from(val: Scalar<Tag>) -> Self {
         Immediate::Scalar(val.into())
     }
 }
 
-impl<'tcx, Tag> Immediate<Tag> {
+impl<'tcx, Tag: Provenance> Immediate<Tag> {
     pub fn from_pointer(p: Pointer<Tag>, cx: &impl HasDataLayout) -> Self {
         Immediate::Scalar(ScalarMaybeUninit::from_pointer(p, cx))
     }
@@ -93,22 +83,15 @@ impl<'tcx, Tag> 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)]
-pub struct ImmTy<'tcx, Tag = AllocId> {
+#[derive(Copy, Clone, Debug)]
+pub struct ImmTy<'tcx, Tag: Provenance = AllocId> {
     imm: Immediate<Tag>,
     pub layout: TyAndLayout<'tcx>,
 }
 
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
 rustc_data_structures::static_assert_size!(ImmTy<'_>, 72);
 
-impl<'tcx, Tag: Provenance> std::fmt::Debug for ImmTy<'tcx, Tag> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let ImmTy { imm, layout } = self;
-        f.debug_struct("ImmTy").field("imm", imm).field("layout", layout).finish()
-    }
-}
-
 impl<Tag: Provenance> std::fmt::Display for ImmTy<'tcx, Tag> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         /// Helper function for printing a scalar to a FmtPrinter
@@ -156,7 +139,7 @@ impl<Tag: Provenance> std::fmt::Display for ImmTy<'tcx, Tag> {
     }
 }
 
-impl<'tcx, Tag> std::ops::Deref for ImmTy<'tcx, Tag> {
+impl<'tcx, Tag: Provenance> std::ops::Deref for ImmTy<'tcx, Tag> {
     type Target = Immediate<Tag>;
     #[inline(always)]
     fn deref(&self) -> &Immediate<Tag> {
@@ -167,68 +150,51 @@ impl<'tcx, Tag> 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)]
-pub enum Operand<Tag = AllocId> {
+#[derive(Copy, Clone, PartialEq, Eq, HashStable, Hash, Debug)]
+pub enum Operand<Tag: Provenance = AllocId> {
     Immediate(Immediate<Tag>),
     Indirect(MemPlace<Tag>),
 }
 
-impl<Tag: Provenance> std::fmt::Debug for Operand<Tag> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        use Operand::*;
-        match self {
-            Immediate(i) => f.debug_tuple("Immediate").field(i).finish(),
-            Indirect(p) => f.debug_tuple("Indirect").field(p).finish(),
-        }
-    }
-}
-
-#[derive(Copy, Clone, PartialEq, Eq, Hash)]
-pub struct OpTy<'tcx, Tag = AllocId> {
+#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
+pub struct OpTy<'tcx, Tag: Provenance = AllocId> {
     op: Operand<Tag>, // Keep this private; it helps enforce invariants.
     pub layout: TyAndLayout<'tcx>,
 }
 
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
-rustc_data_structures::static_assert_size!(OpTy<'_, ()>, 80);
-
-impl<'tcx, Tag: Provenance> std::fmt::Debug for OpTy<'tcx, Tag> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        let OpTy { op, layout } = self;
-        f.debug_struct("OpTy").field("op", op).field("layout", layout).finish()
-    }
-}
+rustc_data_structures::static_assert_size!(OpTy<'_>, 80);
 
-impl<'tcx, Tag> std::ops::Deref for OpTy<'tcx, Tag> {
+impl<'tcx, Tag: Provenance> std::ops::Deref for OpTy<'tcx, Tag> {
     type Target = Operand<Tag>;
     #[inline(always)]
     fn deref(&self) -> &Operand<Tag> {
         &self.op
     }
 }
 
-impl<'tcx, Tag: Copy> From<MPlaceTy<'tcx, Tag>> for OpTy<'tcx, Tag> {
+impl<'tcx, Tag: Provenance> From<MPlaceTy<'tcx, Tag>> for OpTy<'tcx, Tag> {
     #[inline(always)]
     fn from(mplace: MPlaceTy<'tcx, Tag>) -> Self {
         OpTy { op: Operand::Indirect(*mplace), layout: mplace.layout }
     }
 }
 
-impl<'tcx, Tag: Copy> From<&'_ MPlaceTy<'tcx, Tag>> for OpTy<'tcx, Tag> {
+impl<'tcx, Tag: Provenance> From<&'_ MPlaceTy<'tcx, Tag>> for OpTy<'tcx, Tag> {
     #[inline(always)]
     fn from(mplace: &MPlaceTy<'tcx, Tag>) -> Self {
         OpTy { op: Operand::Indirect(**mplace), layout: mplace.layout }
     }
 }
 
-impl<'tcx, Tag> From<ImmTy<'tcx, Tag>> for OpTy<'tcx, Tag> {
+impl<'tcx, Tag: Provenance> From<ImmTy<'tcx, Tag>> for OpTy<'tcx, Tag> {
     #[inline(always)]
     fn from(val: ImmTy<'tcx, Tag>) -> Self {
         OpTy { op: Operand::Immediate(val.imm), layout: val.layout }
     }
 }
 
-impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag> {
+impl<'tcx, Tag: Provenance> ImmTy<'tcx, Tag> {
     #[inline]
     pub fn from_scalar(val: Scalar<Tag>, layout: TyAndLayout<'tcx>) -> Self {
         ImmTy { imm: val.into(), layout }
@@ -259,10 +225,7 @@ impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag> {
     }
 
     #[inline]
-    pub fn to_const_int(self) -> ConstInt
-    where
-        Tag: Provenance,
-    {
+    pub fn to_const_int(self) -> ConstInt {
         assert!(self.layout.ty.is_integral());
         let int = self.to_scalar().expect("to_const_int doesn't work on scalar pairs").assert_int();
         ConstInt::new(int, self.layout.ty.is_signed(), self.layout.ty.is_ptr_sized_integral())