Skip to content

Commit

Permalink
Rollup merge of rust-lang#67501 - oli-obk:test-slice-patterns, r=Ralf…
Browse files Browse the repository at this point in the history
…Jung

Reduce special treatment for zsts

addresses rust-lang#67467 (comment)

cc @RalfJung
  • Loading branch information
Centril authored Jan 10, 2020
2 parents fd9991c + c5c4fa8 commit 79bb166
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 152 deletions.
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!(mplace.layout.is_zst());
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",
);
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_meta();
// 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_meta().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::Meta(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_meta() {
// 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_meta().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

0 comments on commit 79bb166

Please sign in to comment.