Skip to content

Commit

Permalink
Rollup merge of rust-lang#63497 - eddyb:miri-subst, r=oli-obk
Browse files Browse the repository at this point in the history
rustc_mir: fix miri substitution/"universe" discipline.

Alternative to rust-lang#61041, based on @RalfJung's own attempt at it.
I haven't done a full audit, but I believe everything is fixed now.

Fixes rust-lang#61432.
Closes rust-lang#61336, as a drive-by fix (for a subset of rust-lang#43408, that is already special-cased).

r? @oli-obk / @RalfJung cc @varkor @yodaldevoid
  • Loading branch information
Centril authored Aug 20, 2019
2 parents c1b08dd + 96fc989 commit 5ae0710
Show file tree
Hide file tree
Showing 11 changed files with 159 additions and 76 deletions.
20 changes: 12 additions & 8 deletions src/librustc_mir/interpret/cast.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustc::ty::{self, Ty, TypeAndMut};
use rustc::ty::{self, Ty, TypeAndMut, TypeFoldable};
use rustc::ty::layout::{self, TyLayout, Size};
use rustc::ty::adjustment::{PointerCast};
use syntax::ast::FloatTy;
Expand Down Expand Up @@ -36,15 +36,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// The src operand does not matter, just its type
match src.layout.ty.sty {
ty::FnDef(def_id, substs) => {
// All reifications must be monomorphic, bail out otherwise.
if src.layout.ty.needs_subst() {
throw_inval!(TooGeneric);
}

if self.tcx.has_attr(def_id, sym::rustc_args_required_const) {
bug!("reifying a fn ptr that requires const arguments");
}
let instance = ty::Instance::resolve(
*self.tcx,
self.param_env,
def_id,
substs,
).ok_or_else(|| err_inval!(TooGeneric))?;
let instance = self.resolve(def_id, substs)?;
let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance));
self.write_scalar(Scalar::Ptr(fn_ptr.into()), dest)?;
}
Expand All @@ -67,7 +67,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// The src operand does not matter, just its type
match src.layout.ty.sty {
ty::Closure(def_id, substs) => {
let substs = self.subst_and_normalize_erasing_regions(substs)?;
// All reifications must be monomorphic, bail out otherwise.
if src.layout.ty.needs_subst() {
throw_inval!(TooGeneric);
}

let instance = ty::Instance::resolve_closure(
*self.tcx,
def_id,
Expand Down
89 changes: 29 additions & 60 deletions src/librustc_mir/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc::mir;
use rustc::ty::layout::{
self, Size, Align, HasDataLayout, LayoutOf, TyLayout
};
use rustc::ty::subst::{Subst, SubstsRef};
use rustc::ty::subst::SubstsRef;
use rustc::ty::{self, Ty, TyCtxt, TypeFoldable};
use rustc::ty::query::TyCtxtAt;
use rustc_data_structures::indexed_vec::IndexVec;
Expand Down Expand Up @@ -291,41 +291,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
ty.is_freeze(*self.tcx, self.param_env, DUMMY_SP)
}

pub(super) fn subst_and_normalize_erasing_regions<T: TypeFoldable<'tcx>>(
&self,
substs: T,
) -> InterpResult<'tcx, T> {
match self.stack.last() {
Some(frame) => Ok(self.tcx.subst_and_normalize_erasing_regions(
frame.instance.substs,
self.param_env,
&substs,
)),
None => if substs.needs_subst() {
throw_inval!(TooGeneric)
} else {
Ok(substs)
},
}
}

pub(super) fn resolve(
&self,
def_id: DefId,
substs: SubstsRef<'tcx>
) -> InterpResult<'tcx, ty::Instance<'tcx>> {
trace!("resolve: {:?}, {:#?}", def_id, substs);
trace!("param_env: {:#?}", self.param_env);
let substs = self.subst_and_normalize_erasing_regions(substs)?;
trace!("substs: {:#?}", substs);
ty::Instance::resolve(
*self.tcx,
self.param_env,
def_id,
substs,
).ok_or_else(|| err_inval!(TooGeneric).into())
}

pub fn load_mir(
&self,
instance: ty::InstanceDef<'tcx>,
Expand All @@ -349,34 +314,34 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

pub(super) fn monomorphize<T: TypeFoldable<'tcx> + Subst<'tcx>>(
/// Call this on things you got out of the MIR (so it is as generic as the current
/// stack frame), to bring it into the proper environment for this interpreter.
pub(super) fn subst_from_frame_and_normalize_erasing_regions<T: TypeFoldable<'tcx>>(
&self,
t: T,
) -> InterpResult<'tcx, T> {
match self.stack.last() {
Some(frame) => Ok(self.monomorphize_with_substs(t, frame.instance.substs)?),
None => if t.needs_subst() {
throw_inval!(TooGeneric)
} else {
Ok(t)
},
}
value: T,
) -> T {
self.tcx.subst_and_normalize_erasing_regions(
self.frame().instance.substs,
self.param_env,
&value,
)
}

fn monomorphize_with_substs<T: TypeFoldable<'tcx> + Subst<'tcx>>(
/// The `substs` are assumed to already be in our interpreter "universe" (param_env).
pub(super) fn resolve(
&self,
t: T,
def_id: DefId,
substs: SubstsRef<'tcx>
) -> InterpResult<'tcx, T> {
// miri doesn't care about lifetimes, and will choke on some crazy ones
// let's simply get rid of them
let substituted = t.subst(*self.tcx, substs);

if substituted.needs_subst() {
throw_inval!(TooGeneric)
}

Ok(self.tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), substituted))
) -> InterpResult<'tcx, ty::Instance<'tcx>> {
trace!("resolve: {:?}, {:#?}", def_id, substs);
trace!("param_env: {:#?}", self.param_env);
trace!("substs: {:#?}", substs);
ty::Instance::resolve(
*self.tcx,
self.param_env,
def_id,
substs,
).ok_or_else(|| err_inval!(TooGeneric).into())
}

pub fn layout_of_local(
Expand All @@ -391,7 +356,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
None => {
let layout = crate::interpret::operand::from_known_layout(layout, || {
let local_ty = frame.body.local_decls[local].ty;
let local_ty = self.monomorphize_with_substs(local_ty, frame.instance.substs)?;
let local_ty = self.tcx.subst_and_normalize_erasing_regions(
frame.instance.substs,
self.param_env,
&local_ty,
);
self.layout_of(local_ty)
})?;
if let Some(state) = frame.locals.get(local) {
Expand Down
10 changes: 7 additions & 3 deletions src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Move(ref place) =>
self.eval_place_to_op(place, layout)?,

Constant(ref constant) => self.eval_const_to_op(constant.literal, layout)?,
Constant(ref constant) => {
let val = self.subst_from_frame_and_normalize_erasing_regions(constant.literal);
self.eval_const_to_op(val, layout)?
}
};
trace!("{:?}: {:?}", mir_op, *op);
Ok(op)
Expand All @@ -540,6 +543,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

// Used when the miri-engine runs into a constant and for extracting information from constants
// in patterns via the `const_eval` module
/// The `val` and `layout` are assumed to already be in our interpreter
/// "universe" (param_env).
crate fn eval_const_to_op(
&self,
val: &'tcx ty::Const<'tcx>,
Expand All @@ -552,7 +557,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Early-return cases.
match val.val {
ConstValue::Param(_) =>
// FIXME(oli-obk): try to monomorphize
throw_inval!(TooGeneric),
ConstValue::Unevaluated(def_id, substs) => {
let instance = self.resolve(def_id, substs)?;
Expand All @@ -565,7 +569,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
// Other cases need layout.
let layout = from_known_layout(layout, || {
self.layout_of(self.monomorphize(val.ty)?)
self.layout_of(val.ty)
})?;
let op = match val.val {
ConstValue::ByRef { alloc, offset } => {
Expand Down
7 changes: 5 additions & 2 deletions src/librustc_mir/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,11 @@ where
// their layout on return.
PlaceTy {
place: *return_place,
layout: self
.layout_of(self.monomorphize(self.frame().body.return_ty())?)?,
layout: self.layout_of(
self.subst_from_frame_and_normalize_erasing_regions(
self.frame().body.return_ty()
)
)?,
}
}
None => throw_unsup!(InvalidNullPointerUsage),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

NullaryOp(mir::NullOp::SizeOf, ty) => {
let ty = self.monomorphize(ty)?;
let ty = self.subst_from_frame_and_normalize_erasing_regions(ty);
let layout = self.layout_of(ty)?;
assert!(!layout.is_unsized(),
"SizeOf nullary MIR operator called for unsized type");
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_mir/interpret/traits.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rustc::ty::{self, Ty, Instance};
use rustc::ty::{self, Ty, Instance, TypeFoldable};
use rustc::ty::layout::{Size, Align, LayoutOf};
use rustc::mir::interpret::{Scalar, Pointer, InterpResult, PointerArithmetic,};

Expand All @@ -20,6 +20,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

let (ty, poly_trait_ref) = self.tcx.erase_regions(&(ty, poly_trait_ref));

// All vtables must be monomorphic, bail out otherwise.
if ty.needs_subst() || poly_trait_ref.needs_subst() {
throw_inval!(TooGeneric);
}

if let Some(&vtable) = self.vtables.get(&(ty, poly_trait_ref)) {
// This means we guarantee that there are no duplicate vtables, we will
// always use the same vtable for the same (Type, Trait) combination.
Expand Down Expand Up @@ -77,7 +82,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
for (i, method) in methods.iter().enumerate() {
if let Some((def_id, substs)) = *method {
// resolve for vtable: insert shims where needed
let substs = self.subst_and_normalize_erasing_regions(substs)?;
let instance = ty::Instance::resolve_for_vtable(
*self.tcx,
self.param_env,
Expand Down
14 changes: 14 additions & 0 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,20 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics {
let parent_id = tcx.hir().get_parent_item(hir_id);
Some(tcx.hir().local_def_id(parent_id))
}
// FIXME(#43408) enable this in all cases when we get lazy normalization.
Node::AnonConst(&anon_const) => {
// HACK(eddyb) this provides the correct generics when the workaround
// for a const parameter `AnonConst` is being used elsewhere, as then
// there won't be the kind of cyclic dependency blocking #43408.
let expr = &tcx.hir().body(anon_const.body).value;
let icx = ItemCtxt::new(tcx, def_id);
if AstConv::const_param_def_id(&icx, expr).is_some() {
let parent_id = tcx.hir().get_parent_item(hir_id);
Some(tcx.hir().local_def_id(parent_id))
} else {
None
}
}
Node::Expr(&hir::Expr {
node: hir::ExprKind::Closure(..),
..
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/const-generics/issue-61432.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// run-pass

#![feature(const_generics)]
//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash

fn promote<const N: i32>() {
// works:
//
// let n = N;
// &n;

&N;
}

fn main() {
promote::<0>();
}
8 changes: 8 additions & 0 deletions src/test/ui/const-generics/issue-61432.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
warning: the feature `const_generics` is incomplete and may cause the compiler to crash
--> $DIR/issue-61432.rs:3:12
|
LL | #![feature(const_generics)]
| ^^^^^^^^^^^^^^
|
= note: `#[warn(incomplete_features)]` on by default

13 changes: 13 additions & 0 deletions src/test/ui/consts/too_generic_eval_ice.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
pub struct Foo<A, B>(A, B);

impl<A, B> Foo<A, B> {
const HOST_SIZE: usize = std::mem::size_of::<B>();

pub fn crash() -> bool {
[5; Self::HOST_SIZE] == [6; 0] //~ ERROR no associated item named `HOST_SIZE`
//~^ the size for values of type `A` cannot be known
//~| the size for values of type `B` cannot be known
}
}

fn main() {}
47 changes: 47 additions & 0 deletions src/test/ui/consts/too_generic_eval_ice.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
error[E0599]: no associated item named `HOST_SIZE` found for type `Foo<A, B>` in the current scope
--> $DIR/too_generic_eval_ice.rs:7:19
|
LL | pub struct Foo<A, B>(A, B);
| --------------------------- associated item `HOST_SIZE` not found for this
...
LL | [5; Self::HOST_SIZE] == [6; 0]
| ^^^^^^^^^ associated item not found in `Foo<A, B>`
|
= note: the method `HOST_SIZE` exists but the following trait bounds were not satisfied:
`A : std::marker::Sized`
`B : std::marker::Sized`

error[E0277]: the size for values of type `A` cannot be known at compilation time
--> $DIR/too_generic_eval_ice.rs:7:13
|
LL | [5; Self::HOST_SIZE] == [6; 0]
| ^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `A`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
= help: consider adding a `where A: std::marker::Sized` bound
note: required by `Foo`
--> $DIR/too_generic_eval_ice.rs:1:1
|
LL | pub struct Foo<A, B>(A, B);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0277]: the size for values of type `B` cannot be known at compilation time
--> $DIR/too_generic_eval_ice.rs:7:13
|
LL | [5; Self::HOST_SIZE] == [6; 0]
| ^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `B`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
= help: consider adding a `where B: std::marker::Sized` bound
note: required by `Foo`
--> $DIR/too_generic_eval_ice.rs:1:1
|
LL | pub struct Foo<A, B>(A, B);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0277, E0599.
For more information about an error, try `rustc --explain E0277`.

0 comments on commit 5ae0710

Please sign in to comment.