Skip to content

Commit

Permalink
rustc_mir: use the new validator's Qualif in promotion.
Browse files Browse the repository at this point in the history
  • Loading branch information
eddyb committed Oct 25, 2019
1 parent 6c55fb8 commit f2c8628
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 49 deletions.
25 changes: 24 additions & 1 deletion src/librustc_mir/transform/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc::ty::{self, TyCtxt};
pub use self::qualifs::Qualif;

pub mod ops;
mod qualifs;
pub mod qualifs;
mod resolver;
pub mod validation;

Expand All @@ -23,6 +23,7 @@ pub struct Item<'mir, 'tcx> {
def_id: DefId,
param_env: ty::ParamEnv<'tcx>,
mode: validation::Mode,
for_promotion: bool,
}

impl Item<'mir, 'tcx> {
Expand All @@ -41,6 +42,28 @@ impl Item<'mir, 'tcx> {
def_id,
param_env,
mode,
for_promotion: false,
}
}

// HACK(eddyb) this is to get around the panic for a runtime fn from `Item::new`.
// Also, it allows promoting `&mut []`.
pub fn for_promotion(
tcx: TyCtxt<'tcx>,
def_id: DefId,
body: &'mir mir::Body<'tcx>,
) -> Self {
let param_env = tcx.param_env(def_id);
let mode = validation::Mode::for_item(tcx, def_id)
.unwrap_or(validation::Mode::ConstFn);

Item {
body,
tcx,
def_id,
param_env,
mode,
for_promotion: true,
}
}
}
Expand Down
46 changes: 28 additions & 18 deletions src/librustc_mir/transform/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use rustc::mir::*;
use rustc::mir::interpret::ConstValue;
use rustc::ty::{self, Ty};
use rustc_index::bit_set::BitSet;
use syntax_pos::DUMMY_SP;

use super::Item as ConstCx;
Expand Down Expand Up @@ -44,7 +43,7 @@ pub trait Qualif {

fn in_projection_structurally(
cx: &ConstCx<'_, 'tcx>,
per_local: &BitSet<Local>,
per_local: &impl Fn(Local) -> bool,
place: PlaceRef<'_, 'tcx>,
) -> bool {
if let [proj_base @ .., elem] = place.projection {
Expand All @@ -65,7 +64,7 @@ pub trait Qualif {
ProjectionElem::ConstantIndex { .. } |
ProjectionElem::Downcast(..) => qualif,

ProjectionElem::Index(local) => qualif || per_local.contains(*local),
ProjectionElem::Index(local) => qualif || per_local(*local),
}
} else {
bug!("This should be called if projection is not empty");
Expand All @@ -74,22 +73,22 @@ pub trait Qualif {

fn in_projection(
cx: &ConstCx<'_, 'tcx>,
per_local: &BitSet<Local>,
per_local: &impl Fn(Local) -> bool,
place: PlaceRef<'_, 'tcx>,
) -> bool {
Self::in_projection_structurally(cx, per_local, place)
}

fn in_place(
cx: &ConstCx<'_, 'tcx>,
per_local: &BitSet<Local>,
per_local: &impl Fn(Local) -> bool,
place: PlaceRef<'_, 'tcx>,
) -> bool {
match place {
PlaceRef {
base: PlaceBase::Local(local),
projection: [],
} => per_local.contains(*local),
} => per_local(*local),
PlaceRef {
base: PlaceBase::Static(box Static {
kind: StaticKind::Promoted(..),
Expand All @@ -112,7 +111,7 @@ pub trait Qualif {

fn in_operand(
cx: &ConstCx<'_, 'tcx>,
per_local: &BitSet<Local>,
per_local: &impl Fn(Local) -> bool,
operand: &Operand<'tcx>,
) -> bool {
match *operand {
Expand Down Expand Up @@ -143,7 +142,7 @@ pub trait Qualif {

fn in_rvalue_structurally(
cx: &ConstCx<'_, 'tcx>,
per_local: &BitSet<Local>,
per_local: &impl Fn(Local) -> bool,
rvalue: &Rvalue<'tcx>,
) -> bool {
match *rvalue {
Expand Down Expand Up @@ -185,13 +184,17 @@ pub trait Qualif {
}
}

fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet<Local>, rvalue: &Rvalue<'tcx>) -> bool {
fn in_rvalue(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
rvalue: &Rvalue<'tcx>,
) -> bool {
Self::in_rvalue_structurally(cx, per_local, rvalue)
}

fn in_call(
cx: &ConstCx<'_, 'tcx>,
_per_local: &BitSet<Local>,
_per_local: &impl Fn(Local) -> bool,
_callee: &Operand<'tcx>,
_args: &[Operand<'tcx>],
return_ty: Ty<'tcx>,
Expand All @@ -216,7 +219,11 @@ impl Qualif for HasMutInterior {
!ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP)
}

fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet<Local>, rvalue: &Rvalue<'tcx>) -> bool {
fn in_rvalue(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
rvalue: &Rvalue<'tcx>,
) -> bool {
match *rvalue {
// Returning `true` for `Rvalue::Ref` indicates the borrow isn't
// allowed in constants (and the `Checker` will error), and/or it
Expand All @@ -231,12 +238,11 @@ impl Qualif for HasMutInterior {
// Inside a `static mut`, &mut [...] is also allowed.
ty::Array(..) | ty::Slice(_) if cx.mode == Mode::StaticMut => {},

// FIXME(ecstaticmorse): uncomment the following match arm to stop marking
// `&mut []` as `HasMutInterior`.
/*
ty::Array(_, len) if len.try_eval_usize(cx.tcx, cx.param_env) == Some(0)
=> {},
*/
// FIXME(eddyb) the `cx.for_promotion` condition
// seems unnecessary, given that this is merely a ZST.
ty::Array(_, len)
if len.try_eval_usize(cx.tcx, cx.param_env) == Some(0)
&& cx.for_promotion => {},

_ => return true,
}
Expand Down Expand Up @@ -275,7 +281,11 @@ impl Qualif for NeedsDrop {
ty.needs_drop(cx.tcx, cx.param_env)
}

fn in_rvalue(cx: &ConstCx<'_, 'tcx>, per_local: &BitSet<Local>, rvalue: &Rvalue<'tcx>) -> bool {
fn in_rvalue(
cx: &ConstCx<'_, 'tcx>,
per_local: &impl Fn(Local) -> bool,
rvalue: &Rvalue<'tcx>,
) -> bool {
if let Rvalue::Aggregate(ref kind, _) = *rvalue {
if let AggregateKind::Adt(def, ..) = **kind {
if def.has_dtor(cx.tcx) {
Expand Down
12 changes: 9 additions & 3 deletions src/librustc_mir/transform/check_consts/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,13 @@ where
return_place: &mir::Place<'tcx>,
) {
let return_ty = return_place.ty(self.item.body, self.item.tcx).ty;
let qualif = Q::in_call(self.item, &mut self.qualifs_per_local, func, args, return_ty);
let qualif = Q::in_call(
self.item,
&|l| self.qualifs_per_local.contains(l),
func,
args,
return_ty,
);
if !return_place.is_indirect() {
self.assign_qualif_direct(return_place, qualif);
}
Expand Down Expand Up @@ -114,7 +120,7 @@ where
rvalue: &mir::Rvalue<'tcx>,
location: Location,
) {
let qualif = Q::in_rvalue(self.item, self.qualifs_per_local, rvalue);
let qualif = Q::in_rvalue(self.item, &|l| self.qualifs_per_local.contains(l), rvalue);
if !place.is_indirect() {
self.assign_qualif_direct(place, qualif);
}
Expand All @@ -129,7 +135,7 @@ where
// here; that occurs in `apply_call_return_effect`.

if let mir::TerminatorKind::DropAndReplace { value, location: dest, .. } = kind {
let qualif = Q::in_operand(self.item, self.qualifs_per_local, value);
let qualif = Q::in_operand(self.item, &|l| self.qualifs_per_local.contains(l), value);
if !dest.is_indirect() {
self.assign_qualif_direct(dest, qualif);
}
Expand Down
9 changes: 4 additions & 5 deletions src/librustc_mir/transform/check_consts/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,10 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> {
// it depends on `HasMutInterior` being set for mutable borrows as well as values with
// interior mutability.
if let Rvalue::Ref(_, kind, ref borrowed_place) = *rvalue {
let rvalue_has_mut_interior = HasMutInterior::in_rvalue(
&self.item,
self.qualifs.has_mut_interior.get(),
rvalue,
);
let rvalue_has_mut_interior = {
let has_mut_interior = self.qualifs.has_mut_interior.get();
HasMutInterior::in_rvalue(&self.item, &|l| has_mut_interior.contains(l), rvalue)
};

if rvalue_has_mut_interior {
let is_derived_from_illegal_borrow = match borrowed_place.as_local() {
Expand Down
75 changes: 55 additions & 20 deletions src/librustc_mir/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@ use syntax::ast::LitKind;
use syntax::symbol::sym;
use syntax_pos::{Span, DUMMY_SP};

use rustc_index::bit_set::BitSet;
use rustc_index::vec::{IndexVec, Idx};
use rustc_target::spec::abi::Abi;

use std::{iter, mem, usize};

use crate::transform::check_consts::{qualifs, Item as ConstCx};

/// State of a temporary during collection and promotion.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum TempState {
Expand Down Expand Up @@ -231,9 +232,9 @@ struct Validator<'a, 'tcx> {
is_static_mut: bool,
is_non_const_fn: bool,
temps: &'a IndexVec<Local, TempState>,
// FIXME(eddyb) compute these 2 on the fly.
has_mut_interior: &'a BitSet<Local>,
needs_drop: &'a BitSet<Local>,

// FIXME(eddyb) deduplicate the data in this vs other fields.
const_cx: ConstCx<'a, 'tcx>,

/// Explicit promotion happens e.g. for constant arguments declared via
/// `rustc_args_required_const`.
Expand Down Expand Up @@ -276,15 +277,17 @@ impl<'tcx> Validator<'_, 'tcx> {
PlaceBase::Local(local) => local,
_ => return Err(Unpromotable),
};
self.validate_local(base)?;

if place.projection.contains(&ProjectionElem::Deref) {
return Err(Unpromotable);
}

// FIXME(eddyb) compute this on the fly.
let mut has_mut_interior = self.has_mut_interior.contains(base);
let mut has_mut_interior =
self.qualif_local::<qualifs::HasMutInterior>(base);
// HACK(eddyb) this should compute the same thing as
// `<HasMutInterior as Qualif>::in_projection` from
// `qualify_consts` but without recursion.
// `check_consts::qualifs` but without recursion.
if has_mut_interior {
// This allows borrowing fields which don't have
// `HasMutInterior`, from a type that does, e.g.:
Expand All @@ -311,8 +314,7 @@ impl<'tcx> Validator<'_, 'tcx> {
if has_mut_interior {
return Err(Unpromotable);
}
// FIXME(eddyb) compute this on the fly.
if self.needs_drop.contains(base) {
if self.qualif_local::<qualifs::NeedsDrop>(base) {
return Err(Unpromotable);
}
if let BorrowKind::Mut { .. } = kind {
Expand All @@ -339,7 +341,7 @@ impl<'tcx> Validator<'_, 'tcx> {
}
}

self.validate_local(base)
Ok(())
}
_ => bug!()
}
Expand Down Expand Up @@ -373,6 +375,42 @@ impl<'tcx> Validator<'_, 'tcx> {
}
}

// FIXME(eddyb) maybe cache this?
fn qualif_local<Q: qualifs::Qualif>(&self, local: Local) -> bool {
let per_local = &|l| self.qualif_local::<Q>(l);

if let TempState::Defined { location: loc, .. } = self.temps[local] {
let num_stmts = self.body[loc.block].statements.len();

if loc.statement_index < num_stmts {
let statement = &self.body[loc.block].statements[loc.statement_index];
match &statement.kind {
StatementKind::Assign(box(_, rhs)) => {
Q::in_rvalue(&self.const_cx, per_local, rhs)
}
_ => {
span_bug!(statement.source_info.span, "{:?} is not an assignment",
statement);
}
}
} else {
let terminator = self.body[loc.block].terminator();
match &terminator.kind {
TerminatorKind::Call { func, args, .. } => {
let return_ty = self.body.local_decls[local].ty;
Q::in_call(&self.const_cx, per_local, func, args, return_ty)
}
kind => {
span_bug!(terminator.source_info.span, "{:?} not promotable", kind);
}
}
}
} else {
let span = self.body.local_decls[local].source_info.span;
span_bug!(span, "{:?} not promotable, qualif_local shouldn't have been called", local);
}
}

// FIXME(eddyb) maybe cache this?
fn validate_local(&self, local: Local) -> Result<(), Unpromotable> {
if let TempState::Defined { location: loc, .. } = self.temps[local] {
Expand Down Expand Up @@ -593,13 +631,14 @@ impl<'tcx> Validator<'_, 'tcx> {
}
}

self.validate_place(place)?;

// HACK(eddyb) this should compute the same thing as
// `<HasMutInterior as Qualif>::in_projection` from
// `qualify_consts` but without recursion.
// `check_consts::qualifs` but without recursion.
let mut has_mut_interior = match place.base {
PlaceBase::Local(local) => {
// FIXME(eddyb) compute this on the fly.
self.has_mut_interior.contains(*local)
self.qualif_local::<qualifs::HasMutInterior>(*local)
}
PlaceBase::Static(_) => false,
};
Expand All @@ -624,7 +663,7 @@ impl<'tcx> Validator<'_, 'tcx> {
return Err(Unpromotable);
}

self.validate_place(place)
Ok(())
}

Rvalue::Aggregate(_, ref operands) => {
Expand Down Expand Up @@ -680,9 +719,6 @@ pub fn validate_candidates(
body: &Body<'tcx>,
def_id: DefId,
temps: &IndexVec<Local, TempState>,
// FIXME(eddyb) compute these 2 on the fly.
has_mut_interior: &BitSet<Local>,
needs_drop: &BitSet<Local>,
candidates: &[Candidate],
) -> Vec<Candidate> {
let mut validator = Validator {
Expand All @@ -693,9 +729,8 @@ pub fn validate_candidates(
is_static_mut: false,
is_non_const_fn: false,
temps,
// FIXME(eddyb) compute these 2 on the fly.
has_mut_interior,
needs_drop,

const_cx: ConstCx::for_promotion(tcx, def_id, body),

explicit: false,
};
Expand Down
2 changes: 0 additions & 2 deletions src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1118,8 +1118,6 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
self.body,
self.def_id,
&self.temp_promotion_state,
&self.per_local.0[HasMutInterior::IDX],
&self.per_local.0[NeedsDrop::IDX],
&self.unchecked_promotion_candidates,
);

Expand Down

0 comments on commit f2c8628

Please sign in to comment.