Skip to content

Commit

Permalink
avoid manual Debug impls by adding extra Provenance bounds to types
Browse files Browse the repository at this point in the history
I wish the derive macro would support adding extra where clauses...
  • Loading branch information
RalfJung committed Jul 16, 2021
1 parent a5299fb commit efbee50
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 152 deletions.
8 changes: 5 additions & 3 deletions compiler/rustc_middle/src/mir/interpret/pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()),
}
}
Expand Down
28 changes: 8 additions & 20 deletions compiler/rustc_mir/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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>>,
Expand Down
75 changes: 19 additions & 56 deletions compiler/rustc_mir/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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> {
Expand All @@ -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 }
Expand Down Expand Up @@ -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())
Expand Down
Loading

0 comments on commit efbee50

Please sign in to comment.