From 1e40681f50bf820844480b375ab0379aef4ec429 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 14 Dec 2019 00:01:12 +0100 Subject: [PATCH 01/22] Don't ICE on the use of integer addresses for ZST constants in pattern matching --- src/librustc/mir/interpret/allocation.rs | 4 ++ src/librustc_mir/hair/pattern/_match.rs | 52 ++++++++++++++++++------ src/librustc_mir/hair/pattern/mod.rs | 6 +++ src/test/ui/consts/consts-in-patterns.rs | 13 ++++++ 4 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 67f1c8072d680..957a028a59ebb 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -127,6 +127,10 @@ impl Allocation { extra: (), } } + + pub fn zst(align: Align) -> Self { + Self::undef(Size::ZERO, align) + } } impl Allocation<(), ()> { diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index f267be812c3d2..9851f24689744 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -237,11 +237,11 @@ use super::{FieldPat, Pat, PatKind, PatRange}; use rustc::hir::def_id::DefId; use rustc::hir::{HirId, RangeEnd}; -use rustc::ty::layout::{Integer, IntegerExt, Size, VariantIdx}; +use rustc::ty::layout::{Align, Integer, IntegerExt, Size, VariantIdx}; use rustc::ty::{self, Const, Ty, TyCtxt, TypeFoldable, VariantDef}; use rustc::lint; -use rustc::mir::interpret::{truncate, AllocId, ConstValue, Pointer, Scalar}; +use rustc::mir::interpret::{truncate, AllocId, Allocation, ConstValue, Pointer, Scalar}; use rustc::mir::Field; use rustc::util::captures::Captures; use rustc::util::common::ErrorReported; @@ -252,6 +252,7 @@ use syntax_pos::{Span, DUMMY_SP}; use arena::TypedArena; use smallvec::{smallvec, SmallVec}; +use std::borrow::Cow; use std::cmp::{self, max, min, Ordering}; use std::convert::TryInto; use std::fmt; @@ -260,11 +261,12 @@ use std::ops::RangeInclusive; use std::u128; pub fn expand_pattern<'a, 'tcx>(cx: &MatchCheckCtxt<'a, 'tcx>, pat: Pat<'tcx>) -> Pat<'tcx> { - LiteralExpander { tcx: cx.tcx }.fold_pattern(&pat) + LiteralExpander { tcx: cx.tcx, param_env: cx.param_env }.fold_pattern(&pat) } struct LiteralExpander<'tcx> { tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, } impl LiteralExpander<'tcx> { @@ -284,9 +286,23 @@ impl LiteralExpander<'tcx> { debug!("fold_const_value_deref {:?} {:?} {:?}", val, rty, crty); match (val, &crty.kind, &rty.kind) { // the easy case, deref a reference - (ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => { - let alloc = self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id); - ConstValue::ByRef { alloc, offset: p.offset } + (ConstValue::Scalar(p), x, y) if x == y => { + match p { + Scalar::Ptr(p) => { + let alloc = self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id); + ConstValue::ByRef { alloc, offset: p.offset } + } + Scalar::Raw { .. } => { + let layout = self.tcx.layout_of(self.param_env.and(rty)).unwrap(); + if layout.is_zst() { + // Deref of a reference to a ZST is a nop. + ConstValue::Scalar(Scalar::zst()) + } else { + // FIXME(oli-obk): this is reachable for `const FOO: &&&u32 = &&&42;` + bug!("cannot deref {:#?}, {} -> {}", val, crty, rty); + } + } + } } // unsize array to slice if pattern is array but match value or other patterns are slice (ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => { @@ -2348,16 +2364,28 @@ fn specialize_one_pattern<'p, 'tcx>( // just integers. The only time they should be pointing to memory // is when they are subslices of nonzero slices. let (alloc, offset, n, ty) = match value.ty.kind { - ty::Array(t, n) => match value.val { - ty::ConstKind::Value(ConstValue::ByRef { offset, alloc, .. }) => { - (alloc, offset, n.eval_usize(cx.tcx, cx.param_env), t) + ty::Array(t, n) => { + let n = n.eval_usize(cx.tcx, cx.param_env); + match value.val { + ty::ConstKind::Value(ConstValue::ByRef { offset, alloc, .. }) => { + (Cow::Borrowed(alloc), offset, n, t) + } + ty::ConstKind::Value(ConstValue::Scalar(Scalar::Raw { data, .. })) + if n == 0 => + { + let align = Align::from_bytes(data as u64).unwrap(); + // empty array + (Cow::Owned(Allocation::zst(align)), Size::ZERO, 0, t) + } + _ => span_bug!(pat.span, "array pattern is {:?}", value,), } - _ => span_bug!(pat.span, "array pattern is {:?}", value,), - }, + } ty::Slice(t) => { match value.val { ty::ConstKind::Value(ConstValue::Slice { data, start, end }) => { - (data, Size::from_bytes(start as u64), (end - start) as u64, t) + let offset = Size::from_bytes(start as u64); + let n = (end - start) as u64; + (Cow::Borrowed(data), offset, n, t) } ty::ConstKind::Value(ConstValue::ByRef { .. }) => { // FIXME(oli-obk): implement `deref` for `ConstValue` diff --git a/src/librustc_mir/hair/pattern/mod.rs b/src/librustc_mir/hair/pattern/mod.rs index 869aeeba418da..a68ee3308bc23 100644 --- a/src/librustc_mir/hair/pattern/mod.rs +++ b/src/librustc_mir/hair/pattern/mod.rs @@ -993,6 +993,12 @@ pub fn compare_const_vals<'tcx>( return fallback(); } + // Early return for equal constants (so e.g. references to ZSTs can be compared, even if they + // are just integer addresses). + if a.val == b.val { + return from_bool(true); + } + let a_bits = a.try_eval_bits(tcx, param_env, ty); let b_bits = b.try_eval_bits(tcx, param_env, ty); diff --git a/src/test/ui/consts/consts-in-patterns.rs b/src/test/ui/consts/consts-in-patterns.rs index ac6c5a5150688..ee1e3cc22f77d 100644 --- a/src/test/ui/consts/consts-in-patterns.rs +++ b/src/test/ui/consts/consts-in-patterns.rs @@ -1,7 +1,10 @@ // run-pass +#![feature(const_transmute)] const FOO: isize = 10; const BAR: isize = 3; +const ZST: &() = unsafe { std::mem::transmute(1usize) }; +const ZST_ARR: &[u8; 0] = unsafe { std::mem::transmute(1usize) }; const fn foo() -> isize { 4 } const BOO: isize = foo(); @@ -15,4 +18,14 @@ pub fn main() { _ => 3 }; assert_eq!(y, 2); + let z = match &() { + ZST => 9, + // FIXME: this should not be required + _ => 42, + }; + assert_eq!(z, 9); + let z = match b"" { + ZST_ARR => 10, + }; + assert_eq!(z, 10); } From b5b5258d7486ee99fa52f21cbd514dce80c03466 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 14 Dec 2019 00:04:27 +0100 Subject: [PATCH 02/22] Retire `to_ptr` which should already have no users but still kept getting new ones --- src/librustc/mir/interpret/value.rs | 9 ++------- src/librustc/ty/relate.rs | 4 ++-- src/librustc_mir/const_eval/eval_queries.rs | 6 +++--- src/librustc_mir/interpret/eval_context.rs | 4 +++- src/librustc_mir/interpret/intern.rs | 15 +++++++++------ 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 49b542af0a034..93f167cdb9e54 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -367,8 +367,9 @@ impl<'tcx, Tag> Scalar { } /// Do not call this method! Use either `assert_ptr` or `force_ptr`. + /// This method is intentionally private, do not make it public. #[inline] - pub fn to_ptr(self) -> InterpResult<'tcx, Pointer> { + fn to_ptr(self) -> InterpResult<'tcx, Pointer> { match self { Scalar::Raw { data: 0, .. } => throw_unsup!(InvalidNullPointerUsage), Scalar::Raw { .. } => throw_unsup!(ReadBytesAsPointer), @@ -544,12 +545,6 @@ impl<'tcx, Tag> ScalarMaybeUndef { } } - /// Do not call this method! Use either `assert_ptr` or `force_ptr`. - #[inline(always)] - pub fn to_ptr(self) -> InterpResult<'tcx, Pointer> { - self.not_undef()?.to_ptr() - } - /// Do not call this method! Use either `assert_bits` or `force_bits`. #[inline(always)] pub fn to_bits(self, target_size: Size) -> InterpResult<'tcx, u128> { diff --git a/src/librustc/ty/relate.rs b/src/librustc/ty/relate.rs index 933358dce018b..120f05ba7d974 100644 --- a/src/librustc/ty/relate.rs +++ b/src/librustc/ty/relate.rs @@ -537,8 +537,8 @@ pub fn super_relate_consts>( Ok(ConstValue::Scalar(a_val)) } else if let ty::FnPtr(_) = a.ty.kind { let alloc_map = tcx.alloc_map.lock(); - let a_instance = alloc_map.unwrap_fn(a_val.to_ptr().unwrap().alloc_id); - let b_instance = alloc_map.unwrap_fn(b_val.to_ptr().unwrap().alloc_id); + let a_instance = alloc_map.unwrap_fn(a_val.assert_ptr().alloc_id); + let b_instance = alloc_map.unwrap_fn(b_val.assert_ptr().alloc_id); if a_instance == b_instance { Ok(ConstValue::Scalar(a_val)) } else { diff --git a/src/librustc_mir/const_eval/eval_queries.rs b/src/librustc_mir/const_eval/eval_queries.rs index 62ec4bbaec769..745b6aabfa6bb 100644 --- a/src/librustc_mir/const_eval/eval_queries.rs +++ b/src/librustc_mir/const_eval/eval_queries.rs @@ -119,7 +119,7 @@ pub(super) fn op_to_const<'tcx>( }; let val = match immediate { Ok(mplace) => { - let ptr = mplace.ptr.to_ptr().unwrap(); + let ptr = mplace.ptr.assert_ptr(); let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id); ConstValue::ByRef { alloc, offset: ptr.offset } } @@ -133,7 +133,7 @@ pub(super) fn op_to_const<'tcx>( // 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.to_ptr().unwrap(); + let ptr = mplace.ptr.assert_ptr(); let alloc = ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id); ConstValue::ByRef { alloc, offset: ptr.offset } } @@ -176,7 +176,7 @@ fn validate_and_turn_into_const<'tcx>( // Statics/promoteds are always `ByRef`, for the rest `op_to_const` decides // whether they become immediates. if is_static || cid.promoted.is_some() { - let ptr = mplace.ptr.to_ptr()?; + let ptr = mplace.ptr.assert_ptr(); Ok(tcx.mk_const(ty::Const { val: ty::ConstKind::Value(ConstValue::ByRef { alloc: ecx.tcx.alloc_map.lock().unwrap_memory(ptr.alloc_id), diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index ad2af8d7aca52..e8576b198dc24 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -743,7 +743,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // FIXME: should we tell the user that there was a local which was never written to? if let LocalValue::Live(Operand::Indirect(MemPlace { ptr, .. })) = local { trace!("deallocating local"); - let ptr = ptr.to_ptr()?; + // All locals have a backing allocation, even if the allocation is empty + // due to the local having ZST type. + let ptr = ptr.assert_ptr(); if log_enabled!(::log::Level::Trace) { self.memory.dump_alloc(ptr.alloc_id); } diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index b53741e9e43ff..aaeff02fc052a 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -191,11 +191,12 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx if let ty::Dynamic(..) = self.ecx.tcx.struct_tail_erasing_lifetimes(referenced_ty, self.ecx.param_env).kind { - if let Ok(vtable) = mplace.meta.unwrap().to_ptr() { - // explitly 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)?; - } + // Validation has already errored on an invalid vtable pointer so this `assert_ptr` + // will never panic. + let vtable = mplace.meta.unwrap().assert_ptr(); + // explitly 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)?; } // Check if we have encountered this pointer+layout combination before. // Only recurse for allocation-backed pointers. @@ -280,7 +281,9 @@ pub fn intern_const_alloc_recursive>( ecx, leftover_allocations, base_intern_mode, - ret.ptr.to_ptr()?.alloc_id, + // The outermost allocation must exist, because we allocated it with + // `Memory::allocate`. + ret.ptr.assert_ptr().alloc_id, base_mutability, Some(ret.layout.ty), )?; From bb1ecee5b6d96a8b045a6f44c85d738429b3d6c4 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 14 Dec 2019 00:05:04 +0100 Subject: [PATCH 03/22] Simplify `force_allocation_maybe_sized` --- src/librustc_mir/interpret/eval_context.rs | 2 +- src/librustc_mir/interpret/place.rs | 17 +++-------------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index e8576b198dc24..5ba9dcd3aa5c8 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -118,7 +118,7 @@ pub struct LocalState<'tcx, Tag = (), Id = AllocId> { } /// Current value of a local variable -#[derive(Clone, PartialEq, Eq, Debug, HashStable)] // Miri debug-prints these +#[derive(Copy, Clone, PartialEq, Eq, Debug, HashStable)] // Miri debug-prints these pub enum LocalValue { /// This local is not currently alive, and cannot be used at all. Dead, diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index a558f0671e182..c46075699331f 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -974,31 +974,20 @@ where let (mplace, size) = match place.place { Place::Local { frame, local } => { match self.stack[frame].locals[local].access_mut()? { - Ok(local_val) => { + Ok(&mut local_val) => { // We need to make an allocation. - // FIXME: Consider not doing anything for a ZST, and just returning - // a fake pointer? Are we even called for ZST? - - // We cannot hold on to the reference `local_val` while allocating, - // but we can hold on to the value in there. - let old_val = - if let LocalValue::Live(Operand::Immediate(value)) = *local_val { - Some(value) - } else { - None - }; // We need the layout of the local. We can NOT use the layout we got, // that might e.g., be an inner field of a struct with `Scalar` layout, // that has different alignment than the outer field. - // We also need to support unsized types, and hence cannot use `allocate`. let local_layout = self.layout_of_local(&self.stack[frame], local, None)?; + // We also need to support unsized types, and hence cannot use `allocate`. let (size, align) = self .size_and_align_of(meta, local_layout)? .expect("Cannot allocate for non-dyn-sized type"); let ptr = self.memory.allocate(size, align, MemoryKind::Stack); let mplace = MemPlace { ptr: ptr.into(), align, meta }; - if let Some(value) = old_val { + if let LocalValue::Live(Operand::Immediate(value)) = local_val { // Preserve old value. // We don't have to validate as we can assume the local // was already valid for its type. From 13694de4a260e461bd832bd7bc09eb7c3a367c2a Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 14 Dec 2019 00:05:28 +0100 Subject: [PATCH 04/22] Comment on a few odd things that we should look at --- src/librustc_mir/interpret/eval_context.rs | 2 ++ src/librustc_mir/interpret/operand.rs | 7 ++----- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 5ba9dcd3aa5c8..7a26857c560e0 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -761,6 +761,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // FIXME(oli-obk): make this check an assertion that it's not a static here // FIXME(RalfJ, oli-obk): document that `Place::Static` can never be anything but a static // and `ConstValue::Unevaluated` can never be a static + // FIXME(oli-obk, spastorino): the above FIXME is not true anymore, PlaceBase::Static does + // not exist anymore (except for promoteds but it's going away soon). let param_env = if self.tcx.is_static(gid.instance.def_id()) { ty::ParamEnv::reveal_all() } else { diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index a89abe71c654f..82974f338d2ab 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -578,13 +578,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::ConstKind::Param(_) => throw_inval!(TooGeneric), ty::ConstKind::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; + // FIXME: don't use `const_eval_raw` for regular constants, instead use `const_eval` + // which immediately validates the result before we continue with it here. return Ok(OpTy::from(self.const_eval_raw(GlobalId { instance, promoted: None })?)); } - ty::ConstKind::Infer(..) - | ty::ConstKind::Bound(..) - | ty::ConstKind::Placeholder(..) => { - bug!("eval_const_to_op: Unexpected ConstKind {:?}", val) - } ty::ConstKind::Value(val_val) => val_val, }; // Other cases need layout. From a0bd1a695d20da88668a4549f71e86d0f976de15 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 14 Dec 2019 00:19:24 +0100 Subject: [PATCH 05/22] Prevent an ICE on invalid transmutes --- src/librustc/mir/interpret/error.rs | 6 + src/librustc_mir/interpret/place.rs | 10 +- .../transmute-size-mismatch-before-typeck.rs | 15 ++ ...ansmute-size-mismatch-before-typeck.stderr | 136 ++++++++++++++++++ 4 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 src/test/ui/consts/transmute-size-mismatch-before-typeck.rs create mode 100644 src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 843880200f3d7..51f0818fe0be1 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -432,6 +432,7 @@ pub enum UnsupportedOpInfo<'tcx> { HeapAllocNonPowerOfTwoAlignment(u64), ReadFromReturnPointer, PathNotFound(Vec), + TransmuteSizeDiff(Ty<'tcx>, Ty<'tcx>), } impl fmt::Debug for UnsupportedOpInfo<'tcx> { @@ -460,6 +461,11 @@ impl fmt::Debug for UnsupportedOpInfo<'tcx> { passing data of type {:?}", callee_ty, caller_ty ), + TransmuteSizeDiff(from_ty, to_ty) => write!( + f, + "tried to transmute from {:?} to {:?}, but their sizes differed", + from_ty, to_ty + ), FunctionRetMismatch(caller_ty, callee_ty) => write!( f, "tried to call a function with return type {:?} \ diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index c46075699331f..50953348fdc2e 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -923,12 +923,10 @@ where return self.copy_op(src, dest); } // We still require the sizes to match. - assert!( - src.layout.size == dest.layout.size, - "Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}", - src, - dest - ); + if src.layout.size != dest.layout.size { + error!("Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}", src, dest); + throw_unsup!(TransmuteSizeDiff(src.layout.ty, dest.layout.ty)); + } // Unsized copies rely on interpreting `src.meta` with `dest.layout`, we want // to avoid that here. assert!( diff --git a/src/test/ui/consts/transmute-size-mismatch-before-typeck.rs b/src/test/ui/consts/transmute-size-mismatch-before-typeck.rs new file mode 100644 index 0000000000000..1235dd8dcbd98 --- /dev/null +++ b/src/test/ui/consts/transmute-size-mismatch-before-typeck.rs @@ -0,0 +1,15 @@ +#![feature(const_transmute)] + +fn main() { + match &b""[..] { + ZST => {} + //~^ ERROR could not evaluate constant pattern + } +} + +const ZST: &[u8] = unsafe { std::mem::transmute(1usize) }; +//~^ ERROR any use of this value will cause an error +//~| ERROR cannot transmute between types of different sizes + +// Once the `any use of this value will cause an error` disappears in this test, make sure to +// remove the `TransmuteSizeDiff` error variant and make its emitter site an assertion again. diff --git a/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr b/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr new file mode 100644 index 0000000000000..29aeb42898739 --- /dev/null +++ b/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr @@ -0,0 +1,136 @@ +[ERROR rustc_mir::interpret::place] Size mismatch when transmuting! + src: OpTy { + op: Immediate( + Scalar( + 0x0000000000000001, + ), + ), + layout: TyLayout { + ty: usize, + details: LayoutDetails { + variants: Single { + index: 0, + }, + fields: Union( + 0, + ), + abi: Scalar( + Scalar { + value: Int( + I64, + false, + ), + valid_range: 0..=18446744073709551615, + }, + ), + largest_niche: None, + align: AbiAndPrefAlign { + abi: Align { + pow2: 3, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 8, + }, + }, + }, + } + dest: PlaceTy { + place: Ptr( + MemPlace { + ptr: AllocId(0).0x0, + align: Align { + pow2: 3, + }, + meta: None, + }, + ), + layout: TyLayout { + ty: &[u8], + details: LayoutDetails { + variants: Single { + index: 0, + }, + fields: Arbitrary { + offsets: [ + Size { + raw: 0, + }, + Size { + raw: 8, + }, + ], + memory_index: [ + 0, + 1, + ], + }, + abi: ScalarPair( + Scalar { + value: Pointer, + valid_range: 1..=18446744073709551615, + }, + Scalar { + value: Int( + I64, + false, + ), + valid_range: 0..=18446744073709551615, + }, + ), + largest_niche: Some( + Niche { + offset: Size { + raw: 0, + }, + scalar: Scalar { + value: Pointer, + valid_range: 1..=18446744073709551615, + }, + }, + ), + align: AbiAndPrefAlign { + abi: Align { + pow2: 3, + }, + pref: Align { + pow2: 3, + }, + }, + size: Size { + raw: 16, + }, + }, + }, + } +error: any use of this value will cause an error + --> $DIR/transmute-size-mismatch-before-typeck.rs:10:29 + | +LL | const ZST: &[u8] = unsafe { std::mem::transmute(1usize) }; + | ----------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^--- + | | + | tried to transmute from usize to &[u8], but their sizes differed + | + = note: `#[deny(const_err)]` on by default + +error: could not evaluate constant pattern + --> $DIR/transmute-size-mismatch-before-typeck.rs:5:9 + | +LL | ZST => {} + | ^^^ + +error[E0512]: cannot transmute between types of different sizes, or dependently-sized types + --> $DIR/transmute-size-mismatch-before-typeck.rs:10:29 + | +LL | const ZST: &[u8] = unsafe { std::mem::transmute(1usize) }; + | ^^^^^^^^^^^^^^^^^^^ + | + = note: source type: `usize` (64 bits) + = note: target type: `&'static [u8]` (128 bits) + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0512`. From 0e969b73f6f633187a829111d3e80423e85fd513 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 14 Dec 2019 12:15:37 +0100 Subject: [PATCH 06/22] Interning even happens when validation of a constant fails --- src/librustc_mir/interpret/intern.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index aaeff02fc052a..2d66fb463115c 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -191,12 +191,18 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx if let ty::Dynamic(..) = self.ecx.tcx.struct_tail_erasing_lifetimes(referenced_ty, self.ecx.param_env).kind { - // Validation has already errored on an invalid vtable pointer so this `assert_ptr` - // will never panic. - let vtable = mplace.meta.unwrap().assert_ptr(); - // explitly 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)?; + // 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() { + // explitly 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)?; + } else { + self.ecx().tcx.sess.delay_span_bug( + syntax_pos::DUMMY_SP, + "vtables pointers cannot be integer pointers", + ); + } } // Check if we have encountered this pointer+layout combination before. // Only recurse for allocation-backed pointers. From a7a011d2fa97530f30ff2e7112c04723ba611152 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 20 Dec 2019 12:31:52 +0100 Subject: [PATCH 07/22] Immediately evaluate and validate constants when we want them as operands --- src/librustc_mir/interpret/operand.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 82974f338d2ab..110dadbfb378f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -578,9 +578,16 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::ConstKind::Param(_) => throw_inval!(TooGeneric), ty::ConstKind::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; - // FIXME: don't use `const_eval_raw` for regular constants, instead use `const_eval` - // which immediately validates the result before we continue with it here. - return Ok(OpTy::from(self.const_eval_raw(GlobalId { instance, promoted: None })?)); + let param_env = if self.tcx.is_static(def_id) { + ty::ParamEnv::reveal_all() + } else { + self.param_env + }; + let val = + self.tcx.const_eval(param_env.and(GlobalId { instance, promoted: None }))?; + // "recurse". This is only ever going into a recusion depth of 1, because after + // `const_eval` we don't have `Unevaluated` anymore. + return self.eval_const_to_op(val, layout); } ty::ConstKind::Value(val_val) => val_val, }; From 6b651b1a88024df0cda937445593fd360e457ad0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 20 Dec 2019 12:37:05 +0100 Subject: [PATCH 08/22] Add regression test for ZST statics being allowed to "read" from themselves --- src/test/ui/consts/recursive-zst-static.rs | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 src/test/ui/consts/recursive-zst-static.rs diff --git a/src/test/ui/consts/recursive-zst-static.rs b/src/test/ui/consts/recursive-zst-static.rs new file mode 100644 index 0000000000000..df7562bd9f5d2 --- /dev/null +++ b/src/test/ui/consts/recursive-zst-static.rs @@ -0,0 +1,7 @@ +// build-pass + +static FOO: () = FOO; + +fn main() { + FOO +} From 41d58185dd797b0223bdf12463f7a8718b8f6738 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 12:54:48 +0100 Subject: [PATCH 09/22] Explain ParamEnv::reveal_all usage --- src/librustc_mir/interpret/operand.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 110dadbfb378f..78638a399482b 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -578,6 +578,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::ConstKind::Param(_) => throw_inval!(TooGeneric), ty::ConstKind::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; + // For statics we pick `ParamEnv::reveal_all`, because statics don't have generics + // and thus don't care about the parameter environment. While we could just use + // `self.param_env`, that would mean we invoke the query to evaluate the static + // with different parameter environments, thus causing the static to be evaluated + // multiple times. let param_env = if self.tcx.is_static(def_id) { ty::ParamEnv::reveal_all() } else { From 8a88ff1006189de229c0c92c48e1ddd7a3144e81 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 12:55:31 +0100 Subject: [PATCH 10/22] Comments should start capitalized and end in a period --- src/librustc_mir/interpret/intern.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 2d66fb463115c..3ab7c6b891c70 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -187,15 +187,15 @@ impl<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx if let ty::Ref(_, referenced_ty, mutability) = ty.kind { let value = self.ecx.read_immediate(mplace.into())?; let mplace = self.ecx.ref_to_mplace(value)?; - // Handle trait object vtables + // Handle trait object vtables. if let ty::Dynamic(..) = self.ecx.tcx.struct_tail_erasing_lifetimes(referenced_ty, self.ecx.param_env).kind { // Validation has already errored on an invalid vtable pointer so we can safely not - // do anything if this is not a real pointer + // do anything if this is not a real pointer. if let Scalar::Ptr(vtable) = mplace.meta.unwrap() { - // explitly choose `Immutable` here, since vtables are immutable, even - // if the reference of the fat pointer is mutable + // Explitly 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)?; } else { self.ecx().tcx.sess.delay_span_bug( From cb8d1c3c65a0ce88df45acb63fbeb31e19de36ea Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 12:59:15 +0100 Subject: [PATCH 11/22] Explain what we are doing with parameter environments for statics --- src/librustc_mir/interpret/eval_context.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 7a26857c560e0..76f74a3ecaa69 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -758,11 +758,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &self, gid: GlobalId<'tcx>, ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { - // FIXME(oli-obk): make this check an assertion that it's not a static here - // FIXME(RalfJ, oli-obk): document that `Place::Static` can never be anything but a static - // and `ConstValue::Unevaluated` can never be a static - // FIXME(oli-obk, spastorino): the above FIXME is not true anymore, PlaceBase::Static does - // not exist anymore (except for promoteds but it's going away soon). + // For statics we pick `ParamEnv::reveal_all`, because statics don't have generics + // and thus don't care about the parameter environment. While we could just use + // `self.param_env`, that would mean we invoke the query to evaluate the static + // with different parameter environments, thus causing the static to be evaluated + // multiple times. let param_env = if self.tcx.is_static(gid.instance.def_id()) { ty::ParamEnv::reveal_all() } else { From 6937ca2c90961d12cf84fa743cfc34f206d7d75c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 13:06:50 +0100 Subject: [PATCH 12/22] Explain the currently necessary existance of `TransmuteSizeDiff` --- src/librustc_mir/interpret/place.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 50953348fdc2e..b384de571fa41 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -924,6 +924,10 @@ where } // We still require the sizes to match. if src.layout.size != dest.layout.size { + // FIXME: This should be an assert instead of an error, but if we transmute within an + // array length computation, `typeck` may not have yet been run and errored out. In fact + // most likey we *are* running `typeck` right now. Investigate whether we can bail out + // on `typeck_tables().has_errors` at all const eval entry points. error!("Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}", src, dest); throw_unsup!(TransmuteSizeDiff(src.layout.ty, dest.layout.ty)); } From 72ebce0e1b145b58722be88858eea7efbf05566b Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 13:10:56 +0100 Subject: [PATCH 13/22] Remove unintended noisy log statement --- src/librustc_mir/interpret/place.rs | 2 +- ...ansmute-size-mismatch-before-typeck.stderr | 108 ------------------ 2 files changed, 1 insertion(+), 109 deletions(-) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index b384de571fa41..f4ac7de852af0 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -928,7 +928,7 @@ where // array length computation, `typeck` may not have yet been run and errored out. In fact // most likey we *are* running `typeck` right now. Investigate whether we can bail out // on `typeck_tables().has_errors` at all const eval entry points. - error!("Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}", src, dest); + debug!("Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}", src, dest); throw_unsup!(TransmuteSizeDiff(src.layout.ty, dest.layout.ty)); } // Unsized copies rely on interpreting `src.meta` with `dest.layout`, we want diff --git a/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr b/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr index 29aeb42898739..74de5dc9aaf82 100644 --- a/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr +++ b/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr @@ -1,111 +1,3 @@ -[ERROR rustc_mir::interpret::place] Size mismatch when transmuting! - src: OpTy { - op: Immediate( - Scalar( - 0x0000000000000001, - ), - ), - layout: TyLayout { - ty: usize, - details: LayoutDetails { - variants: Single { - index: 0, - }, - fields: Union( - 0, - ), - abi: Scalar( - Scalar { - value: Int( - I64, - false, - ), - valid_range: 0..=18446744073709551615, - }, - ), - largest_niche: None, - align: AbiAndPrefAlign { - abi: Align { - pow2: 3, - }, - pref: Align { - pow2: 3, - }, - }, - size: Size { - raw: 8, - }, - }, - }, - } - dest: PlaceTy { - place: Ptr( - MemPlace { - ptr: AllocId(0).0x0, - align: Align { - pow2: 3, - }, - meta: None, - }, - ), - layout: TyLayout { - ty: &[u8], - details: LayoutDetails { - variants: Single { - index: 0, - }, - fields: Arbitrary { - offsets: [ - Size { - raw: 0, - }, - Size { - raw: 8, - }, - ], - memory_index: [ - 0, - 1, - ], - }, - abi: ScalarPair( - Scalar { - value: Pointer, - valid_range: 1..=18446744073709551615, - }, - Scalar { - value: Int( - I64, - false, - ), - valid_range: 0..=18446744073709551615, - }, - ), - largest_niche: Some( - Niche { - offset: Size { - raw: 0, - }, - scalar: Scalar { - value: Pointer, - valid_range: 1..=18446744073709551615, - }, - }, - ), - align: AbiAndPrefAlign { - abi: Align { - pow2: 3, - }, - pref: Align { - pow2: 3, - }, - }, - size: Size { - raw: 16, - }, - }, - }, - } error: any use of this value will cause an error --> $DIR/transmute-size-mismatch-before-typeck.rs:10:29 | From 0e3fafaea7e6e5d12ef075ff37cb60af34ca6d05 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 13:13:08 +0100 Subject: [PATCH 14/22] Typo --- src/librustc_mir/interpret/intern.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 3ab7c6b891c70..dffc8e8256c42 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -194,7 +194,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() { - // Explitly choose `Immutable` here, since vtables are immutable, even + // 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)?; } else { From 95205518dd255d3b71161baf35c1d3f5850ccb46 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 13:30:47 +0100 Subject: [PATCH 15/22] Explain why `const_eval` is ok here --- src/librustc_mir/interpret/operand.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 78638a399482b..694ffaa83e40b 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -588,6 +588,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } else { self.param_env }; + // We use `const_eval` here and `const_eval_raw` elsewhere in mir interpretation. + // The reason we use `const_eval_raw` everywhere else is to prevent cycles during + // validation, because validation automatically reads through any references, thus + // potentially requiring the current static to be evaluated again. This is not a + // problem here, because we need an operand and operands are always reads. + // FIXME(oli-obk): eliminate all the `const_eval_raw` usages when we get rid of + // `StaticKind` once and for all. let val = self.tcx.const_eval(param_env.and(GlobalId { instance, promoted: None }))?; // "recurse". This is only ever going into a recusion depth of 1, because after From 1acbf4b8028ae4eb86128c07945ac0cdc8f7d811 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 20:39:35 +0100 Subject: [PATCH 16/22] Early abort instead of building up zero sized values --- src/librustc/mir/interpret/allocation.rs | 4 ---- src/librustc_mir/hair/pattern/_match.rs | 20 +++++++++++--------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 957a028a59ebb..67f1c8072d680 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -127,10 +127,6 @@ impl Allocation { extra: (), } } - - pub fn zst(align: Align) -> Self { - Self::undef(Size::ZERO, align) - } } impl Allocation<(), ()> { diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 9851f24689744..c6efbe8833279 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -237,11 +237,11 @@ use super::{FieldPat, Pat, PatKind, PatRange}; use rustc::hir::def_id::DefId; use rustc::hir::{HirId, RangeEnd}; -use rustc::ty::layout::{Align, Integer, IntegerExt, Size, VariantIdx}; +use rustc::ty::layout::{Integer, IntegerExt, Size, VariantIdx}; use rustc::ty::{self, Const, Ty, TyCtxt, TypeFoldable, VariantDef}; use rustc::lint; -use rustc::mir::interpret::{truncate, AllocId, Allocation, ConstValue, Pointer, Scalar}; +use rustc::mir::interpret::{truncate, AllocId, ConstValue, Pointer, Scalar}; use rustc::mir::Field; use rustc::util::captures::Captures; use rustc::util::common::ErrorReported; @@ -2366,17 +2366,19 @@ fn specialize_one_pattern<'p, 'tcx>( let (alloc, offset, n, ty) = match value.ty.kind { ty::Array(t, n) => { let n = n.eval_usize(cx.tcx, cx.param_env); + // Shortcut for `n == 0` where no matter what `alloc` and `offset` we produce, + // the result would be exactly what we early return here. + if n == 0 { + if ctor_wild_subpatterns.len() as u64 == 0 { + return Some(PatStack::from_slice(&[])); + } else { + return None; + } + } match value.val { ty::ConstKind::Value(ConstValue::ByRef { offset, alloc, .. }) => { (Cow::Borrowed(alloc), offset, n, t) } - ty::ConstKind::Value(ConstValue::Scalar(Scalar::Raw { data, .. })) - if n == 0 => - { - let align = Align::from_bytes(data as u64).unwrap(); - // empty array - (Cow::Owned(Allocation::zst(align)), Size::ZERO, 0, t) - } _ => span_bug!(pat.span, "array pattern is {:?}", value,), } } From 20c1b3fb49385bed0fc1fc539f573d3695a94753 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 20:56:01 +0100 Subject: [PATCH 17/22] Add a `const_eval` helper to `InterpCx` --- src/librustc_mir/interpret/eval_context.rs | 22 ++++++++++++++++++++-- src/librustc_mir/interpret/intrinsics.rs | 5 ++--- src/librustc_mir/interpret/operand.rs | 16 +--------------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 76f74a3ecaa69..da4156c2719fd 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -20,8 +20,8 @@ use rustc_macros::HashStable; use syntax::source_map::{self, Span, DUMMY_SP}; use super::{ - Immediate, MPlaceTy, Machine, MemPlace, Memory, Operand, Place, PlaceTy, ScalarMaybeUndef, - StackPopInfo, + Immediate, MPlaceTy, Machine, MemPlace, Memory, OpTy, Operand, Place, PlaceTy, + ScalarMaybeUndef, StackPopInfo, }; pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> { @@ -754,6 +754,24 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } + pub(super) fn const_eval( + &self, + gid: GlobalId<'tcx>, + ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { + // For statics we pick `ParamEnv::reveal_all`, because statics don't have generics + // and thus don't care about the parameter environment. While we could just use + // `self.param_env`, that would mean we invoke the query to evaluate the static + // with different parameter environments, thus causing the static to be evaluated + // multiple times. + let param_env = if self.tcx.is_static(gid.instance.def_id()) { + ty::ParamEnv::reveal_all() + } else { + self.param_env + }; + let val = self.tcx.const_eval(param_env.and(gid))?; + self.eval_const_to_op(val, None) + } + pub fn const_eval_raw( &self, gid: GlobalId<'tcx>, diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 8e4dc87451c32..eb7657f780c56 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -118,9 +118,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | sym::size_of | sym::type_id | sym::type_name => { - let val = - self.tcx.const_eval_instance(self.param_env, instance, Some(self.tcx.span))?; - let val = self.eval_const_to_op(val, None)?; + let gid = GlobalId { instance, promoted: None }; + let val = self.const_eval(gid)?; self.copy_op(val, dest)?; } diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 694ffaa83e40b..fc63847433b5c 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -578,16 +578,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ty::ConstKind::Param(_) => throw_inval!(TooGeneric), ty::ConstKind::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; - // For statics we pick `ParamEnv::reveal_all`, because statics don't have generics - // and thus don't care about the parameter environment. While we could just use - // `self.param_env`, that would mean we invoke the query to evaluate the static - // with different parameter environments, thus causing the static to be evaluated - // multiple times. - let param_env = if self.tcx.is_static(def_id) { - ty::ParamEnv::reveal_all() - } else { - self.param_env - }; // We use `const_eval` here and `const_eval_raw` elsewhere in mir interpretation. // The reason we use `const_eval_raw` everywhere else is to prevent cycles during // validation, because validation automatically reads through any references, thus @@ -595,11 +585,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // problem here, because we need an operand and operands are always reads. // FIXME(oli-obk): eliminate all the `const_eval_raw` usages when we get rid of // `StaticKind` once and for all. - let val = - self.tcx.const_eval(param_env.and(GlobalId { instance, promoted: None }))?; - // "recurse". This is only ever going into a recusion depth of 1, because after - // `const_eval` we don't have `Unevaluated` anymore. - return self.eval_const_to_op(val, layout); + return self.const_eval(GlobalId { instance, promoted: None }); } ty::ConstKind::Value(val_val) => val_val, }; From 1531c3937bb93778dad60c0352891464aa0e00bd Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 21:28:25 +0100 Subject: [PATCH 18/22] Documentation nit --- src/librustc_mir/interpret/operand.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index fc63847433b5c..e37aa69dfde83 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -582,7 +582,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // The reason we use `const_eval_raw` everywhere else is to prevent cycles during // validation, because validation automatically reads through any references, thus // potentially requiring the current static to be evaluated again. This is not a - // problem here, because we need an operand and operands are always reads. + // problem here, because we are building an operand which means an actual read is + // happening. // FIXME(oli-obk): eliminate all the `const_eval_raw` usages when we get rid of // `StaticKind` once and for all. return self.const_eval(GlobalId { instance, promoted: None }); From b476344ccc877cd0a2bdb5673928f51fd80b4a79 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 22 Dec 2019 21:31:24 +0100 Subject: [PATCH 19/22] Reintroduce the recursion comment --- src/librustc_mir/interpret/eval_context.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index da4156c2719fd..bc158a767cef5 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -769,6 +769,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.param_env }; let val = self.tcx.const_eval(param_env.and(gid))?; + // Even though `ecx.const_eval` is called from `eval_const_to_op` we can never have a + // recursion deeper than one level, because the `tcx.const_eval` above is guaranteed to not + // return `ConstValue::Unevaluated`, which is the only way that `eval_const_to_op` will call + // `ecx.const_eval`. self.eval_const_to_op(val, None) } From aaffe12453bd98268fda8b5397eff7998ef2ea55 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 23 Dec 2019 17:13:25 +0100 Subject: [PATCH 20/22] Use the targetted const eval functions --- src/librustc_mir/interpret/eval_context.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index bc158a767cef5..766ef6ab6feac 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -758,17 +758,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &self, gid: GlobalId<'tcx>, ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { - // For statics we pick `ParamEnv::reveal_all`, because statics don't have generics - // and thus don't care about the parameter environment. While we could just use - // `self.param_env`, that would mean we invoke the query to evaluate the static - // with different parameter environments, thus causing the static to be evaluated - // multiple times. - let param_env = if self.tcx.is_static(gid.instance.def_id()) { - ty::ParamEnv::reveal_all() + let val = if self.tcx.is_static(gid.instance.def_id()) { + self.tcx.const_eval_poly(gid.instance.def_id())? + } else if let Some(promoted) = gid.promoted { + self.tcx.const_eval_promoted(gid.instance, promoted)? } else { - self.param_env + self.tcx.const_eval_instance(self.param_env, gid.instance, Some(self.tcx.span))? }; - let val = self.tcx.const_eval(param_env.and(gid))?; // Even though `ecx.const_eval` is called from `eval_const_to_op` we can never have a // recursion deeper than one level, because the `tcx.const_eval` above is guaranteed to not // return `ConstValue::Unevaluated`, which is the only way that `eval_const_to_op` will call From 12a4c2ca18fc5392945099170d1f32d8f296b2a4 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 23 Dec 2019 17:13:50 +0100 Subject: [PATCH 21/22] Fix rebase fallout --- src/librustc_mir/interpret/intrinsics.rs | 2 +- src/librustc_mir/interpret/operand.rs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index eb7657f780c56..da7cff97ee2c2 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -5,7 +5,7 @@ use rustc::hir::def_id::DefId; use rustc::mir::{ self, - interpret::{ConstValue, InterpResult, Scalar}, + interpret::{ConstValue, GlobalId, InterpResult, Scalar}, BinOp, }; use rustc::ty; diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index e37aa69dfde83..def979b63b52a 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -588,6 +588,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // `StaticKind` once and for all. return self.const_eval(GlobalId { instance, promoted: None }); } + ty::ConstKind::Infer(..) + | ty::ConstKind::Bound(..) + | ty::ConstKind::Placeholder(..) => { + bug!("eval_const_to_op: Unexpected ConstKind {:?}", val) + } ty::ConstKind::Value(val_val) => val_val, }; // Other cases need layout. From f65a91eb47453448e27a03eccaad581ebc130d1a Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 24 Dec 2019 14:31:36 +0100 Subject: [PATCH 22/22] Make ui test bitwidth independent --- .../ui/consts/transmute-size-mismatch-before-typeck.rs | 8 ++++++-- .../transmute-size-mismatch-before-typeck.stderr | 10 +++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/test/ui/consts/transmute-size-mismatch-before-typeck.rs b/src/test/ui/consts/transmute-size-mismatch-before-typeck.rs index 1235dd8dcbd98..b5d2b4396f9a9 100644 --- a/src/test/ui/consts/transmute-size-mismatch-before-typeck.rs +++ b/src/test/ui/consts/transmute-size-mismatch-before-typeck.rs @@ -1,9 +1,13 @@ #![feature(const_transmute)] +// normalize-stderr-64bit "64 bits" -> "word size" +// normalize-stderr-32bit "32 bits" -> "word size" +// normalize-stderr-64bit "128 bits" -> "2 * word size" +// normalize-stderr-32bit "64 bits" -> "2 * word size" + fn main() { match &b""[..] { - ZST => {} - //~^ ERROR could not evaluate constant pattern + ZST => {} //~ ERROR could not evaluate constant pattern } } diff --git a/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr b/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr index 74de5dc9aaf82..5f84204f4086a 100644 --- a/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr +++ b/src/test/ui/consts/transmute-size-mismatch-before-typeck.stderr @@ -1,5 +1,5 @@ error: any use of this value will cause an error - --> $DIR/transmute-size-mismatch-before-typeck.rs:10:29 + --> $DIR/transmute-size-mismatch-before-typeck.rs:14:29 | LL | const ZST: &[u8] = unsafe { std::mem::transmute(1usize) }; | ----------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^--- @@ -9,19 +9,19 @@ LL | const ZST: &[u8] = unsafe { std::mem::transmute(1usize) }; = note: `#[deny(const_err)]` on by default error: could not evaluate constant pattern - --> $DIR/transmute-size-mismatch-before-typeck.rs:5:9 + --> $DIR/transmute-size-mismatch-before-typeck.rs:10:9 | LL | ZST => {} | ^^^ error[E0512]: cannot transmute between types of different sizes, or dependently-sized types - --> $DIR/transmute-size-mismatch-before-typeck.rs:10:29 + --> $DIR/transmute-size-mismatch-before-typeck.rs:14:29 | LL | const ZST: &[u8] = unsafe { std::mem::transmute(1usize) }; | ^^^^^^^^^^^^^^^^^^^ | - = note: source type: `usize` (64 bits) - = note: target type: `&'static [u8]` (128 bits) + = note: source type: `usize` (word size) + = note: target type: `&'static [u8]` (2 * word size) error: aborting due to 3 previous errors