Skip to content

Commit

Permalink
Rollup merge of rust-lang#72879 - RalfJung:miri-tctx-at, r=oli-obk
Browse files Browse the repository at this point in the history
Miri: avoid tracking current location three times

Miri tracks the current instruction to execute in the call stack, but it also additionally has two `TyCtxtAt` that carry a `Span` that also tracks the current instruction. That is quite silly, so this PR uses `TyCtxt` instead, and then uses a method for computing the current span when a `TyCtxtAt` is needed. Having less redundant (semi-)global state seems like a good improvement to me. :D

To keep the ConstProp errors the same, I had to add the option to `error_to_const_error` to overwrite the span. Also for some reason this changes cycle errors a bit -- not sure if we are now better or worse as giving those queries the right span. (It is unfortunately quite easy to accidentally use `DUMMY_SP` by calling the query on a `TyCtxt` instead of a `TyCtxtAt`.)

r? @oli-obk @eddyb
  • Loading branch information
RalfJung authored Jun 15, 2020
2 parents d97e8ca + 2210abe commit f9c8a67
Show file tree
Hide file tree
Showing 23 changed files with 141 additions and 123 deletions.
1 change: 1 addition & 0 deletions src/librustc_middle/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,7 @@ impl<'tcx> ty::TyS<'tcx> {
/// optimization as well as the rules around static values. Note
/// that the `Freeze` trait is not exposed to end users and is
/// effectively an implementation detail.
// FIXME: use `TyCtxtAt` instead of separate `Span`.
pub fn is_freeze(
&'tcx self,
tcx: TyCtxt<'tcx>,
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_mir/const_eval/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::error::Error;
use std::fmt;

use rustc_middle::mir::AssertKind;
use rustc_span::Symbol;
use rustc_span::{Span, Symbol};

use super::InterpCx;
use crate::interpret::{ConstEvalErr, InterpErrorInfo, Machine};
Expand Down Expand Up @@ -53,8 +53,9 @@ impl Error for ConstEvalErrKind {}
pub fn error_to_const_error<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>(
ecx: &InterpCx<'mir, 'tcx, M>,
error: InterpErrorInfo<'tcx>,
span: Option<Span>,
) -> ConstEvalErr<'tcx> {
error.print_backtrace();
let stacktrace = ecx.generate_stacktrace();
ConstEvalErr { error: error.kind, stacktrace, span: ecx.tcx.span }
ConstEvalErr { error: error.kind, stacktrace, span: span.unwrap_or_else(|| ecx.cur_span()) }
}
29 changes: 18 additions & 11 deletions src/librustc_mir/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
body: &'mir mir::Body<'tcx>,
) -> InterpResult<'tcx, MPlaceTy<'tcx>> {
debug!("eval_body_using_ecx: {:?}, {:?}", cid, ecx.param_env);
let tcx = ecx.tcx.tcx;
let tcx = *ecx.tcx;
let layout = ecx.layout_of(body.return_ty().subst(tcx, cid.instance.substs))?;
assert!(!layout.is_unsized());
let ret = ecx.allocate(layout, MemoryKind::Stack);
Expand Down Expand Up @@ -81,13 +81,14 @@ fn eval_body_using_ecx<'mir, 'tcx>(
/// parameter. These bounds are passed to `mk_eval_cx` via the `ParamEnv` argument.
pub(super) fn mk_eval_cx<'mir, 'tcx>(
tcx: TyCtxt<'tcx>,
span: Span,
root_span: Span,
param_env: ty::ParamEnv<'tcx>,
can_access_statics: bool,
) -> CompileTimeEvalContext<'mir, 'tcx> {
debug!("mk_eval_cx: {:?}", param_env);
InterpCx::new(
tcx.at(span),
tcx,
root_span,
param_env,
CompileTimeInterpreter::new(tcx.sess.const_eval_limit()),
MemoryExtra { can_access_statics },
Expand Down Expand Up @@ -163,7 +164,7 @@ pub(super) fn op_to_const<'tcx>(
0,
),
};
let len = b.to_machine_usize(&ecx.tcx.tcx).unwrap();
let len = b.to_machine_usize(ecx).unwrap();
let start = start.try_into().unwrap();
let len: usize = len.try_into().unwrap();
ConstValue::Slice { data, start, end: start + len }
Expand Down Expand Up @@ -212,7 +213,7 @@ fn validate_and_turn_into_const<'tcx>(
})();

val.map_err(|error| {
let err = error_to_const_error(&ecx, error);
let err = error_to_const_error(&ecx, error, None);
err.struct_error(ecx.tcx, "it is undefined behavior to use this value", |mut diag| {
diag.note(note_on_undefined_behavior_error());
diag.emit();
Expand Down Expand Up @@ -299,9 +300,9 @@ pub fn const_eval_raw_provider<'tcx>(

let is_static = tcx.is_static(def_id);

let span = tcx.def_span(cid.instance.def_id());
let mut ecx = InterpCx::new(
tcx.at(span),
tcx,
tcx.def_span(cid.instance.def_id()),
key.param_env,
CompileTimeInterpreter::new(tcx.sess.const_eval_limit()),
MemoryExtra { can_access_statics: is_static },
Expand All @@ -311,12 +312,15 @@ pub fn const_eval_raw_provider<'tcx>(
res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, &body))
.map(|place| RawConst { alloc_id: place.ptr.assert_ptr().alloc_id, ty: place.layout.ty })
.map_err(|error| {
let err = error_to_const_error(&ecx, error);
let err = error_to_const_error(&ecx, error, None);
// errors in statics are always emitted as fatal errors
if is_static {
// Ensure that if the above error was either `TooGeneric` or `Reported`
// an error must be reported.
let v = err.report_as_error(ecx.tcx, "could not evaluate static initializer");
let v = err.report_as_error(
ecx.tcx.at(ecx.cur_span()),
"could not evaluate static initializer",
);

// If this is `Reveal:All`, then we need to make sure an error is reported but if
// this is `Reveal::UserFacing`, then it's expected that we could get a
Expand Down Expand Up @@ -372,13 +376,16 @@ pub fn const_eval_raw_provider<'tcx>(
// anything else (array lengths, enum initializers, constant patterns) are
// reported as hard errors
} else {
err.report_as_error(ecx.tcx, "evaluation of constant value failed")
err.report_as_error(
ecx.tcx.at(ecx.cur_span()),
"evaluation of constant value failed",
)
}
}
}
} else {
// use of broken constant from other crate
err.report_as_error(ecx.tcx, "could not evaluate constant")
err.report_as_error(ecx.tcx.at(ecx.cur_span()), "could not evaluate constant")
}
})
}
7 changes: 2 additions & 5 deletions src/librustc_mir/interpret/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
(&ty::Array(_, length), &ty::Slice(_)) => {
let ptr = self.read_immediate(src)?.to_scalar()?;
// u64 cast is from usize to u64, which is always good
let val = Immediate::new_slice(
ptr,
length.eval_usize(self.tcx.tcx, self.param_env),
self,
);
let val =
Immediate::new_slice(ptr, length.eval_usize(*self.tcx, self.param_env), self);
self.write_immediate(val, dest)
}
(&ty::Dynamic(..), &ty::Dynamic(..)) => {
Expand Down
24 changes: 15 additions & 9 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
pub machine: M,

/// The results of the type checker, from rustc.
/// The span in this is the "root" of the evaluation, i.e., the const
/// we are evaluating (if this is CTFE).
pub tcx: TyCtxtAt<'tcx>,

/// Bounds in scope for polymorphic evaluations.
Expand Down Expand Up @@ -202,7 +204,7 @@ where
}
}

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> LayoutOf for InterpCx<'mir, 'tcx, M> {
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> LayoutOf for InterpCx<'mir, 'tcx, M> {
type Ty = Ty<'tcx>;
type TyAndLayout = InterpResult<'tcx, TyAndLayout<'tcx>>;

Expand Down Expand Up @@ -285,24 +287,28 @@ pub(super) fn from_known_layout<'tcx>(

impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
pub fn new(
tcx: TyCtxtAt<'tcx>,
tcx: TyCtxt<'tcx>,
root_span: Span,
param_env: ty::ParamEnv<'tcx>,
machine: M,
memory_extra: M::MemoryExtra,
) -> Self {
InterpCx {
machine,
tcx,
tcx: tcx.at(root_span),
param_env,
memory: Memory::new(tcx, memory_extra),
vtables: FxHashMap::default(),
}
}

#[inline(always)]
pub fn set_span(&mut self, span: Span) {
self.tcx.span = span;
self.memory.tcx.span = span;
pub fn cur_span(&self) -> Span {
self.stack()
.last()
.and_then(|f| f.current_source_info())
.map(|si| si.span)
.unwrap_or(self.tcx.span)
}

#[inline(always)]
Expand Down Expand Up @@ -385,7 +391,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

#[inline]
pub fn type_is_freeze(&self, ty: Ty<'tcx>) -> bool {
ty.is_freeze(*self.tcx, self.param_env, DUMMY_SP)
ty.is_freeze(*self.tcx, self.param_env, self.tcx.span)
}

pub fn load_mir(
Expand Down Expand Up @@ -554,7 +560,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let size = size.align_to(align);

// Check if this brought us over the size limit.
if size.bytes() >= self.tcx.data_layout().obj_size_bound() {
if size.bytes() >= self.tcx.data_layout.obj_size_bound() {
throw_ub!(InvalidMeta("total size is bigger than largest supported object"));
}
Ok(Some((size, align)))
Expand All @@ -570,7 +576,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let elem = layout.field(self, 0)?;

// Make sure the slice is not too big.
let size = elem.size.checked_mul(len, &*self.tcx).ok_or_else(|| {
let size = elem.size.checked_mul(len, self).ok_or_else(|| {
err_ub!(InvalidMeta("slice is bigger than largest supported object"))
})?;
Ok(Some((size, elem.align.abi)))
Expand Down
5 changes: 2 additions & 3 deletions src/librustc_mir/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>(
if let InternMode::Static(mutability) = mode {
// For this, we need to take into account `UnsafeCell`. When `ty` is `None`, we assume
// no interior mutability.
let frozen = ty.map_or(true, |ty| ty.is_freeze(ecx.tcx.tcx, ecx.param_env, ecx.tcx.span));
let frozen = ty.map_or(true, |ty| ty.is_freeze(*ecx.tcx, ecx.param_env, ecx.tcx.span));
// For statics, allocation mutability is the combination of the place mutability and
// the type mutability.
// The entire allocation needs to be mutable if it contains an `UnsafeCell` anywhere.
Expand Down Expand Up @@ -253,8 +253,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx>> ValueVisitor<'mir
// caused (by somehow getting a mutable reference in a `const`).
if ref_mutability == Mutability::Mut {
match referenced_ty.kind {
ty::Array(_, n)
if n.eval_usize(tcx.tcx, self.ecx.param_env) == 0 => {}
ty::Array(_, n) if n.eval_usize(*tcx, self.ecx.param_env) == 0 => {}
ty::Slice(_)
if mplace.meta.unwrap_meta().to_machine_usize(self.ecx)?
== 0 => {}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let index = u64::from(self.read_scalar(args[1])?.to_u32()?);
let elem = args[2];
let input = args[0];
let (len, e_ty) = input.layout.ty.simd_size_and_type(self.tcx.tcx);
let (len, e_ty) = input.layout.ty.simd_size_and_type(*self.tcx);
assert!(
index < len,
"Index `{}` must be in bounds of vector type `{}`: `[0, {})`",
Expand All @@ -374,7 +374,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
sym::simd_extract => {
let index = u64::from(self.read_scalar(args[1])?.to_u32()?);
let (len, e_ty) = args[0].layout.ty.simd_size_and_type(self.tcx.tcx);
let (len, e_ty) = args[0].layout.ty.simd_size_and_type(*self.tcx);
assert!(
index < len,
"index `{}` is out-of-bounds of vector type `{}` with length `{}`",
Expand Down
20 changes: 10 additions & 10 deletions src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use std::ptr;

use rustc_ast::ast::Mutability;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_middle::ty::{self, query::TyCtxtAt, Instance, ParamEnv};
use rustc_middle::ty::{self, Instance, ParamEnv, TyCtxt};
use rustc_target::abi::{Align, HasDataLayout, Size, TargetDataLayout};

use super::{
Expand Down Expand Up @@ -115,7 +115,7 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
pub extra: M::MemoryExtra,

/// Lets us implement `HasDataLayout`, which is awfully convenient.
pub tcx: TyCtxtAt<'tcx>,
pub tcx: TyCtxt<'tcx>,
}

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for Memory<'mir, 'tcx, M> {
Expand All @@ -126,7 +126,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for Memory<'mir, 'tcx, M>
}

impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
pub fn new(tcx: TyCtxtAt<'tcx>, extra: M::MemoryExtra) -> Self {
pub fn new(tcx: TyCtxt<'tcx>, extra: M::MemoryExtra) -> Self {
Memory {
alloc_map: M::MemoryMap::default(),
extra_fn_ptr_map: FxHashMap::default(),
Expand Down Expand Up @@ -425,7 +425,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
/// `M::tag_allocation`.
fn get_global_alloc(
memory_extra: &M::MemoryExtra,
tcx: TyCtxtAt<'tcx>,
tcx: TyCtxt<'tcx>,
id: AllocId,
is_write: bool,
) -> InterpResult<'tcx, Cow<'tcx, Allocation<M::PointerTag, M::AllocExtra>>> {
Expand Down Expand Up @@ -455,7 +455,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
throw_unsup!(ReadForeignStatic(def_id))
}
trace!("get_global_alloc: Need to compute {:?}", def_id);
let instance = Instance::mono(tcx.tcx, def_id);
let instance = Instance::mono(tcx, def_id);
let gid = GlobalId { instance, promoted: None };
// Use the raw query here to break validation cycles. Later uses of the static
// will call the full query anyway.
Expand Down Expand Up @@ -664,14 +664,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
pub fn dump_allocs(&self, mut allocs: Vec<AllocId>) {
// Cannot be a closure because it is generic in `Tag`, `Extra`.
fn write_allocation_track_relocs<'tcx, Tag: Copy + fmt::Debug, Extra>(
tcx: TyCtxtAt<'tcx>,
tcx: TyCtxt<'tcx>,
allocs_to_print: &mut VecDeque<AllocId>,
alloc: &Allocation<Tag, Extra>,
) {
for &(_, target_id) in alloc.relocations().values() {
allocs_to_print.push_back(target_id);
}
pretty::write_allocation(tcx.tcx, alloc, &mut std::io::stderr()).unwrap();
pretty::write_allocation(tcx, alloc, &mut std::io::stderr()).unwrap();
}

allocs.sort();
Expand Down Expand Up @@ -820,7 +820,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
return Ok(());
}
};
let tcx = self.tcx.tcx;
let tcx = self.tcx;
self.get_raw_mut(ptr.alloc_id)?.write_bytes(&tcx, ptr, src)
}

Expand All @@ -846,7 +846,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
return Ok(());
}
};
let tcx = self.tcx.tcx;
let tcx = self.tcx;
let allocation = self.get_raw_mut(ptr.alloc_id)?;

for idx in 0..len {
Expand Down Expand Up @@ -888,7 +888,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
let relocations =
self.get_raw(src.alloc_id)?.prepare_relocation_copy(self, src, size, dest, length);

let tcx = self.tcx.tcx;
let tcx = self.tcx;

// This checks relocation edges on the src.
let src_bytes =
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,12 +651,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Convert discriminant to variant index, and catch invalid discriminants.
let index = match op.layout.ty.kind {
ty::Adt(adt, _) => {
adt.discriminants(self.tcx.tcx).find(|(_, var)| var.val == discr_bits)
adt.discriminants(*self.tcx).find(|(_, var)| var.val == discr_bits)
}
ty::Generator(def_id, substs, _) => {
let substs = substs.as_generator();
substs
.discriminants(def_id, self.tcx.tcx)
.discriminants(def_id, *self.tcx)
.find(|(_, var)| var.val == discr_bits)
}
_ => bug!("tagged layout for non-adt non-generator"),
Expand Down
Loading

0 comments on commit f9c8a67

Please sign in to comment.