Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce special treatment for zsts #67501

Merged
merged 13 commits into from
Jan 10, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions src/librustc_mir/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,28 +115,31 @@ pub(super) fn op_to_const<'tcx>(
// by-val is if we are in const_field, i.e., if this is (a field of) something that we
// "tried to make immediate" before. We wouldn't do that for non-slice scalar pairs or
// structs containing such.
op.try_as_mplace()
op.try_as_mplace(ecx)
};
let val = match immediate {
Ok(mplace) => {
let ptr = mplace.ptr.assert_ptr();

let to_const_value = |mplace: MPlaceTy<'_>| match mplace.ptr {
Scalar::Ptr(ptr) => {
let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id);
ConstValue::ByRef { alloc, offset: ptr.offset }
}
Scalar::Raw { data, .. } => {
assert_eq!(
data,
mplace.layout.align.abi.bytes().into(),
"this MPlaceTy must come from `try_as_mplace` being used on a zst, so we know what
value this integer address must have",
);
assert!(mplace.layout.is_zst());
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
ConstValue::Scalar(Scalar::zst())
}
};
let val = match immediate {
Ok(mplace) => to_const_value(mplace),
// see comment on `let try_as_immediate` above
Err(ImmTy { imm: Immediate::Scalar(x), .. }) => match x {
ScalarMaybeUndef::Scalar(s) => ConstValue::Scalar(s),
ScalarMaybeUndef::Undef => {
// When coming out of "normal CTFE", we'll always have an `Indirect` operand as
// argument and we will not need this. The only way we can already have an
// `Immediate` is when we are called from `const_field`, and that `Immediate`
// comes from a constant so it can happen have `Undef`, because the indirect
// memory that was read had undefined bytes.
let mplace = op.assert_mem_place();
let ptr = mplace.ptr.assert_ptr();
let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id);
ConstValue::ByRef { alloc, offset: ptr.offset }
}
ScalarMaybeUndef::Undef => to_const_value(op.assert_mem_place(ecx)),
},
Err(ImmTy { imm: Immediate::ScalarPair(a, b), .. }) => {
let (data, start) = match a.not_undef().unwrap() {
Expand Down
13 changes: 6 additions & 7 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_macros::HashStable;
use rustc_span::source_map::{self, Span, DUMMY_SP};

use super::{
Immediate, MPlaceTy, Machine, MemPlace, Memory, OpTy, Operand, Place, PlaceTy,
Immediate, MPlaceTy, Machine, MemPlace, MemPlaceMeta, Memory, OpTy, Operand, Place, PlaceTy,
ScalarMaybeUndef, StackPopInfo,
};

Expand Down Expand Up @@ -393,7 +393,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// This can fail to provide an answer for extern types.
pub(super) fn size_and_align_of(
&self,
metadata: Option<Scalar<M::PointerTag>>,
metadata: MemPlaceMeta<M::PointerTag>,
layout: TyLayout<'tcx>,
) -> InterpResult<'tcx, Option<(Size, Align)>> {
if !layout.is_unsized() {
Expand Down Expand Up @@ -465,14 +465,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Ok(Some((size, align)))
}
ty::Dynamic(..) => {
let vtable = metadata.expect("dyn trait fat ptr must have vtable");
let vtable = metadata.unwrap_unsized();
// Read size and align from vtable (already checks size).
Ok(Some(self.read_size_and_align_from_vtable(vtable)?))
}

ty::Slice(_) | ty::Str => {
let len =
metadata.expect("slice fat ptr must have length").to_machine_usize(self)?;
let len = metadata.unwrap_unsized().to_machine_usize(self)?;
let elem = layout.field(self, 0)?;

// Make sure the slice is not too big.
Expand Down Expand Up @@ -818,8 +817,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
" by align({}){} ref:",
mplace.align.bytes(),
match mplace.meta {
Some(meta) => format!(" meta({:?})", meta),
None => String::new(),
MemPlaceMeta::Unsized(meta) => format!(" meta({:?})", meta),
MemPlaceMeta::Poison | MemPlaceMeta::None => String::new(),
}
)
.unwrap();
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_mir/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx
{
// Validation has already errored on an invalid vtable pointer so we can safely not
// do anything if this is not a real pointer.
if let Scalar::Ptr(vtable) = mplace.meta.unwrap() {
if let Scalar::Ptr(vtable) = mplace.meta.unwrap_unsized() {
// Explicitly choose `Immutable` here, since vtables are immutable, even
// if the reference of the fat pointer is mutable.
self.intern_shallow(vtable.alloc_id, Mutability::Not, None)?;
Expand Down Expand Up @@ -226,7 +226,8 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx
| (InternMode::Const, hir::Mutability::Mut) => match referenced_ty.kind {
ty::Array(_, n)
if n.eval_usize(self.ecx.tcx.tcx, self.ecx.param_env) == 0 => {}
ty::Slice(_) if mplace.meta.unwrap().to_machine_usize(self.ecx)? == 0 => {}
ty::Slice(_)
if mplace.meta.unwrap_unsized().to_machine_usize(self.ecx)? == 0 => {}
_ => bug!("const qualif failed to prevent mutable references"),
},
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub use rustc::mir::interpret::*; // have all the `interpret` symbols in one pla

pub use self::eval_context::{Frame, InterpCx, LocalState, LocalValue, StackPopCleanup};

pub use self::place::{MPlaceTy, MemPlace, Place, PlaceTy};
pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy};

pub use self::memory::{AllocCheck, FnVal, Memory, MemoryKind};

Expand Down
54 changes: 5 additions & 49 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,30 +153,6 @@ pub enum Operand<Tag = (), Id = AllocId> {
Indirect(MemPlace<Tag, Id>),
}

impl<Tag> Operand<Tag> {
#[inline]
pub fn assert_mem_place(self) -> MemPlace<Tag>
where
Tag: ::std::fmt::Debug,
{
match self {
Operand::Indirect(mplace) => mplace,
_ => bug!("assert_mem_place: expected Operand::Indirect, got {:?}", self),
}
}

#[inline]
pub fn assert_immediate(self) -> Immediate<Tag>
where
Tag: ::std::fmt::Debug,
{
match self {
Operand::Immediate(imm) => imm,
_ => bug!("assert_immediate: expected Operand::Immediate, got {:?}", self),
}
}
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub struct OpTy<'tcx, Tag = ()> {
op: Operand<Tag>, // Keep this private; it helps enforce invariants.
Expand Down Expand Up @@ -267,7 +243,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
&self,
op: OpTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
match op.try_as_mplace() {
match op.try_as_mplace(self) {
Ok(mplace) => Ok(self.force_mplace_ptr(mplace)?.into()),
Err(imm) => Ok(imm.into()), // Nothing to cast/force
}
Expand Down Expand Up @@ -335,7 +311,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
&self,
src: OpTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, Result<ImmTy<'tcx, M::PointerTag>, MPlaceTy<'tcx, M::PointerTag>>> {
Ok(match src.try_as_mplace() {
Ok(match src.try_as_mplace(self) {
Ok(mplace) => {
if let Some(val) = self.try_read_immediate_from_mplace(mplace)? {
Ok(val)
Expand Down Expand Up @@ -383,7 +359,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
op: OpTy<'tcx, M::PointerTag>,
field: u64,
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
let base = match op.try_as_mplace() {
let base = match op.try_as_mplace(self) {
Ok(mplace) => {
// The easy case
let field = self.mplace_field(mplace, field)?;
Expand Down Expand Up @@ -420,7 +396,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
variant: VariantIdx,
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
// Downcasts only change the layout
Ok(match op.try_as_mplace() {
Ok(match op.try_as_mplace(self) {
Ok(mplace) => self.mplace_downcast(mplace, variant)?.into(),
Err(..) => {
let layout = op.layout.for_variant(self, variant);
Expand All @@ -439,30 +415,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Field(field, _) => self.operand_field(base, field.index() as u64)?,
Downcast(_, variant) => self.operand_downcast(base, variant)?,
Deref => self.deref_operand(base)?.into(),
ConstantIndex { .. } | Index(_) if base.layout.is_zst() => {
OpTy {
op: Operand::Immediate(Scalar::zst().into()),
// the actual index doesn't matter, so we just pick a convenient one like 0
layout: base.layout.field(self, 0)?,
}
}
Subslice { from, to, from_end } if base.layout.is_zst() => {
let elem_ty = if let ty::Array(elem_ty, _) = base.layout.ty.kind {
elem_ty
} else {
bug!("slices shouldn't be zero-sized");
};
assert!(!from_end, "arrays shouldn't be subsliced from the end");

OpTy {
op: Operand::Immediate(Scalar::zst().into()),
layout: self.layout_of(self.tcx.mk_array(elem_ty, (to - from) as u64))?,
}
}
Subslice { .. } | ConstantIndex { .. } | Index(_) => {
// The rest should only occur as mplace, we do not use Immediates for types
// allowing such operations. This matches place_projection forcing an allocation.
let mplace = base.assert_mem_place();
let mplace = base.assert_mem_place(self);
self.mplace_projection(mplace, proj_elem)?.into()
}
})
Expand Down
Loading