From efbee50600e63293608811d7ee200f12641f1958 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 16 Jul 2021 19:50:59 +0200 Subject: [PATCH] avoid manual Debug impls by adding extra Provenance bounds to types I wish the derive macro would support adding extra where clauses... --- .../rustc_middle/src/mir/interpret/pointer.rs | 8 +- .../rustc_mir/src/interpret/eval_context.rs | 28 ++---- compiler/rustc_mir/src/interpret/operand.rs | 75 ++++----------- compiler/rustc_mir/src/interpret/place.rs | 92 ++++--------------- 4 files changed, 51 insertions(+), 152 deletions(-) diff --git a/compiler/rustc_middle/src/mir/interpret/pointer.rs b/compiler/rustc_middle/src/mir/interpret/pointer.rs index c7daaec8d5d5..7e7a7119be6a 100644 --- a/compiler/rustc_middle/src/mir/interpret/pointer.rs +++ b/compiler/rustc_middle/src/mir/interpret/pointer.rs @@ -86,7 +86,9 @@ impl 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 fmt::Debug for Pointer { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - Tag::fmt(self, f) + Provenance::fmt(self, f) } } impl fmt::Debug for Pointer> { 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 --git a/compiler/rustc_mir/src/interpret/eval_context.rs b/compiler/rustc_mir/src/interpret/eval_context.rs index b130eb3ca0c3..516ef4f4e53c 100644 --- a/compiler/rustc_mir/src/interpret/eval_context.rs +++ b/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,7 +161,7 @@ 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, /// Don't modify if `Some`, this is only used to prevent computing the layout twice #[stable_hasher(ignore)] @@ -169,8 +169,8 @@ pub struct LocalState<'tcx, Tag = AllocId> { } /// Current value of a local variable -#[derive(Copy, Clone, PartialEq, Eq, HashStable)] -pub enum LocalValue { +#[derive(Copy, Clone, PartialEq, Eq, HashStable, Debug)] // Miri debug-prints these +pub enum LocalValue { /// 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 { Live(Operand), } -impl std::fmt::Debug for LocalValue { - // 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(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> +impl<'ctx, 'mir, 'tcx, Tag: Provenance, Extra> HashStable> for Frame<'mir, 'tcx, Tag, Extra> where Extra: HashStable>, diff --git a/compiler/rustc_mir/src/interpret/operand.rs b/compiler/rustc_mir/src/interpret/operand.rs index 04d4e3993e4f..aba7db781684 100644 --- a/compiler/rustc_mir/src/interpret/operand.rs +++ b/compiler/rustc_mir/src/interpret/operand.rs @@ -27,8 +27,8 @@ 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 { +#[derive(Copy, Clone, PartialEq, Eq, HashStable, Hash, Debug)] +pub enum Immediate { Scalar(ScalarMaybeUninit), ScalarPair(ScalarMaybeUninit, ScalarMaybeUninit), } @@ -36,31 +36,21 @@ pub enum Immediate { #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Immediate, 56); -impl std::fmt::Debug for Immediate { - 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 From> for Immediate { +impl From> for Immediate { #[inline(always)] fn from(val: ScalarMaybeUninit) -> Self { Immediate::Scalar(val) } } -impl From> for Immediate { +impl From> for Immediate { #[inline(always)] fn from(val: Scalar) -> Self { Immediate::Scalar(val.into()) } } -impl<'tcx, Tag> Immediate { +impl<'tcx, Tag: Provenance> Immediate { pub fn from_pointer(p: Pointer, cx: &impl HasDataLayout) -> Self { Immediate::Scalar(ScalarMaybeUninit::from_pointer(p, cx)) } @@ -93,8 +83,8 @@ impl<'tcx, Tag> Immediate { // 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, pub layout: TyAndLayout<'tcx>, } @@ -102,13 +92,6 @@ pub struct ImmTy<'tcx, Tag = AllocId> { #[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 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 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; #[inline(always)] fn deref(&self) -> &Immediate { @@ -167,39 +150,22 @@ 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 { +#[derive(Copy, Clone, PartialEq, Eq, HashStable, Hash, Debug)] +pub enum Operand { Immediate(Immediate), Indirect(MemPlace), } -impl std::fmt::Debug for Operand { - 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, // 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); +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() - } -} - -impl<'tcx, Tag> std::ops::Deref for OpTy<'tcx, Tag> { +impl<'tcx, Tag: Provenance> std::ops::Deref for OpTy<'tcx, Tag> { type Target = Operand; #[inline(always)] fn deref(&self) -> &Operand { @@ -207,28 +173,28 @@ impl<'tcx, Tag> std::ops::Deref for OpTy<'tcx, Tag> { } } -impl<'tcx, Tag: Copy> From> for OpTy<'tcx, Tag> { +impl<'tcx, Tag: Provenance> From> 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> for OpTy<'tcx, Tag> { +impl<'tcx, Tag: Provenance> From> 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, 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()) diff --git a/compiler/rustc_mir/src/interpret/place.rs b/compiler/rustc_mir/src/interpret/place.rs index 419c17595a1b..91fcc3495b1c 100644 --- a/compiler/rustc_mir/src/interpret/place.rs +++ b/compiler/rustc_mir/src/interpret/place.rs @@ -19,9 +19,9 @@ use super::{ Operand, Pointer, PointerArithmetic, Provenance, Scalar, ScalarMaybeUninit, }; -#[derive(Copy, Clone, Hash, PartialEq, Eq, HashStable)] +#[derive(Copy, Clone, Hash, PartialEq, Eq, HashStable, Debug)] /// Information required for the sound usage of a `MemPlace`. -pub enum MemPlaceMeta { +pub enum MemPlaceMeta { /// The unsized payload (e.g. length for slices or vtable pointer for trait objects). Meta(Scalar), /// `Sized` types or unsized `extern type` @@ -36,18 +36,7 @@ pub enum MemPlaceMeta { #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(MemPlaceMeta, 24); -impl std::fmt::Debug for MemPlaceMeta { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - use MemPlaceMeta::*; - match self { - Meta(s) => f.debug_tuple("Meta").field(s).finish(), - None => f.debug_tuple("None").finish(), - Poison => f.debug_tuple("Poison").finish(), - } - } -} - -impl MemPlaceMeta { +impl MemPlaceMeta { pub fn unwrap_meta(self) -> Scalar { match self { Self::Meta(s) => s, @@ -64,8 +53,8 @@ impl MemPlaceMeta { } } -#[derive(Copy, Clone, Hash, PartialEq, Eq, HashStable)] -pub struct MemPlace { +#[derive(Copy, Clone, Hash, PartialEq, Eq, HashStable, Debug)] +pub struct MemPlace { /// The pointer can be a pure integer, with the `None` tag. pub ptr: Pointer>, pub align: Align, @@ -78,19 +67,8 @@ pub struct MemPlace { #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(MemPlace, 48); -impl std::fmt::Debug for MemPlace { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let MemPlace { ptr, align, meta } = self; - f.debug_struct("MemPlace") - .field("ptr", ptr) - .field("align", align) - .field("meta", meta) - .finish() - } -} - -#[derive(Copy, Clone, Hash, PartialEq, Eq, HashStable)] -pub enum Place { +#[derive(Copy, Clone, Hash, PartialEq, Eq, HashStable, Debug)] +pub enum Place { /// A place referring to a value allocated in the `Memory` system. Ptr(MemPlace), @@ -102,20 +80,8 @@ pub enum Place { #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(Place, 56); -impl std::fmt::Debug for Place { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - use Place::*; - match self { - Ptr(p) => f.debug_tuple("Ptr").field(p).finish(), - Local { frame, local } => { - f.debug_struct("Local").field("frame", frame).field("local", local).finish() - } - } - } -} - -#[derive(Copy, Clone)] -pub struct PlaceTy<'tcx, Tag = AllocId> { +#[derive(Copy, Clone, Debug)] +pub struct PlaceTy<'tcx, Tag: Provenance = AllocId> { place: Place, // Keep this private; it helps enforce invariants. pub layout: TyAndLayout<'tcx>, } @@ -123,14 +89,7 @@ pub struct PlaceTy<'tcx, Tag = AllocId> { #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(PlaceTy<'_>, 72); -impl<'tcx, Tag: Provenance> std::fmt::Debug for PlaceTy<'tcx, Tag> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let PlaceTy { place, layout } = self; - f.debug_struct("PlaceTy").field("place", place).field("layout", layout).finish() - } -} - -impl<'tcx, Tag> std::ops::Deref for PlaceTy<'tcx, Tag> { +impl<'tcx, Tag: Provenance> std::ops::Deref for PlaceTy<'tcx, Tag> { type Target = Place; #[inline(always)] fn deref(&self) -> &Place { @@ -139,8 +98,8 @@ impl<'tcx, Tag> 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)] -pub struct MPlaceTy<'tcx, Tag = AllocId> { +#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] +pub struct MPlaceTy<'tcx, Tag: Provenance = AllocId> { mplace: MemPlace, pub layout: TyAndLayout<'tcx>, } @@ -148,14 +107,7 @@ pub struct MPlaceTy<'tcx, Tag = AllocId> { #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))] rustc_data_structures::static_assert_size!(MPlaceTy<'_>, 64); -impl<'tcx, Tag: Provenance> std::fmt::Debug for MPlaceTy<'tcx, Tag> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let MPlaceTy { mplace, layout } = self; - f.debug_struct("MPlaceTy").field("mplace", mplace).field("layout", layout).finish() - } -} - -impl<'tcx, Tag> std::ops::Deref for MPlaceTy<'tcx, Tag> { +impl<'tcx, Tag: Provenance> std::ops::Deref for MPlaceTy<'tcx, Tag> { type Target = MemPlace; #[inline(always)] fn deref(&self) -> &MemPlace { @@ -163,14 +115,14 @@ impl<'tcx, Tag> std::ops::Deref for MPlaceTy<'tcx, Tag> { } } -impl<'tcx, Tag> From> for PlaceTy<'tcx, Tag> { +impl<'tcx, Tag: Provenance> From> for PlaceTy<'tcx, Tag> { #[inline(always)] fn from(mplace: MPlaceTy<'tcx, Tag>) -> Self { PlaceTy { place: Place::Ptr(mplace.mplace), layout: mplace.layout } } } -impl MemPlace { +impl MemPlace { #[inline(always)] pub fn from_ptr(ptr: Pointer>, align: Align) -> Self { MemPlace { ptr, align, meta: MemPlaceMeta::None } @@ -212,7 +164,7 @@ impl MemPlace { } } -impl<'tcx, Tag: Copy> MPlaceTy<'tcx, Tag> { +impl<'tcx, Tag: Provenance> MPlaceTy<'tcx, Tag> { /// Produces a MemPlace that works for ZST but nothing else #[inline] pub fn dangling(layout: TyAndLayout<'tcx>) -> Self { @@ -239,10 +191,7 @@ impl<'tcx, Tag: Copy> MPlaceTy<'tcx, Tag> { } #[inline] - pub(super) fn len(&self, cx: &impl HasDataLayout) -> InterpResult<'tcx, u64> - where - Tag: Provenance, - { + pub(super) fn len(&self, cx: &impl HasDataLayout) -> InterpResult<'tcx, u64> { if self.layout.is_unsized() { // We need to consult `meta` metadata match self.layout.ty.kind() { @@ -269,7 +218,7 @@ impl<'tcx, Tag: Copy> MPlaceTy<'tcx, Tag> { } // These are defined here because they produce a place. -impl<'tcx, Tag: Copy> OpTy<'tcx, Tag> { +impl<'tcx, Tag: Provenance> OpTy<'tcx, Tag> { #[inline(always)] /// Note: do not call `as_ref` on the resulting place. This function should only be used to /// read from the resulting mplace, not to get its address back. @@ -284,10 +233,7 @@ impl<'tcx, Tag: Copy> OpTy<'tcx, Tag> { #[inline(always)] /// Note: do not call `as_ref` on the resulting place. This function should only be used to /// read from the resulting mplace, not to get its address back. - pub fn assert_mem_place(&self) -> MPlaceTy<'tcx, Tag> - where - Tag: Provenance, - { + pub fn assert_mem_place(&self) -> MPlaceTy<'tcx, Tag> { self.try_as_mplace().unwrap() } }