diff options
| -rw-r--r-- | src/librustc_mir/transform/promote_consts.rs | 604 | ||||
| -rw-r--r-- | src/librustc_mir/transform/qualify_consts.rs | 117 |
2 files changed, 691 insertions, 30 deletions
diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 7a9c489fa79..4d411ad4f53 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -12,15 +12,22 @@ //! initialization and can otherwise silence errors, if //! move analysis runs after promotion on broken MIR. +use rustc::hir; use rustc::hir::def_id::DefId; use rustc::mir::*; +use rustc::mir::interpret::ConstValue; use rustc::mir::visit::{PlaceContext, MutatingUseContext, MutVisitor, Visitor}; use rustc::mir::traversal::ReversePostorder; +use rustc::ty::{self, List, TyCtxt}; use rustc::ty::subst::InternalSubsts; -use rustc::ty::{List, TyCtxt}; -use syntax_pos::Span; +use rustc::ty::cast::CastTy; +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}; @@ -57,7 +64,7 @@ impl TempState { /// A "root candidate" for promotion, which will become the /// returned value in a promoted MIR, unless it's a subset /// of a larger candidate. -#[derive(Debug)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum Candidate { /// Borrow of a constant temporary. Ref(Location), @@ -73,13 +80,28 @@ pub enum Candidate { Argument { bb: BasicBlock, index: usize }, } -struct TempCollector<'tcx> { +fn args_required_const(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Vec<usize>> { + let attrs = tcx.get_attrs(def_id); + let attr = attrs.iter().find(|a| a.check_name(sym::rustc_args_required_const))?; + let mut ret = vec![]; + for meta in attr.meta_item_list()? { + match meta.literal()?.kind { + LitKind::Int(a, _) => { ret.push(a as usize); } + _ => return None, + } + } + Some(ret) +} + +struct Collector<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + body: &'a Body<'tcx>, temps: IndexVec<Local, TempState>, + candidates: Vec<Candidate>, span: Span, - body: &'tcx Body<'tcx>, } -impl<'tcx> Visitor<'tcx> for TempCollector<'tcx> { +impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> { fn visit_local(&mut self, &index: &Local, context: PlaceContext, @@ -134,22 +156,576 @@ impl<'tcx> Visitor<'tcx> for TempCollector<'tcx> { *temp = TempState::Unpromotable; } + fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) { + self.super_rvalue(rvalue, location); + + match *rvalue { + Rvalue::Ref(..) => { + self.candidates.push(Candidate::Ref(location)); + } + Rvalue::Repeat(..) if self.tcx.features().const_in_array_repeat_expressions => { + // FIXME(#49147) only promote the element when it isn't `Copy` + // (so that code that can copy it at runtime is unaffected). + self.candidates.push(Candidate::Repeat(location)); + } + _ => {} + } + } + + fn visit_terminator_kind(&mut self, + kind: &TerminatorKind<'tcx>, + location: Location) { + self.super_terminator_kind(kind, location); + + if let TerminatorKind::Call { ref func, .. } = *kind { + if let ty::FnDef(def_id, _) = func.ty(self.body, self.tcx).kind { + let fn_sig = self.tcx.fn_sig(def_id); + if let Abi::RustIntrinsic | Abi::PlatformIntrinsic = fn_sig.abi() { + let name = self.tcx.item_name(def_id); + // FIXME(eddyb) use `#[rustc_args_required_const(2)]` for shuffles. + if name.as_str().starts_with("simd_shuffle") { + self.candidates.push(Candidate::Argument { + bb: location.block, + index: 2, + }); + } + } + + if let Some(constant_args) = args_required_const(self.tcx, def_id) { + for index in constant_args { + self.candidates.push(Candidate::Argument { bb: location.block, index }); + } + } + } + } + } + fn visit_source_info(&mut self, source_info: &SourceInfo) { self.span = source_info.span; } } -pub fn collect_temps(body: &Body<'_>, - rpo: &mut ReversePostorder<'_, '_>) -> IndexVec<Local, TempState> { - let mut collector = TempCollector { +pub fn collect_temps_and_candidates( + tcx: TyCtxt<'tcx>, + body: &Body<'tcx>, + rpo: &mut ReversePostorder<'_, 'tcx>, +) -> (IndexVec<Local, TempState>, Vec<Candidate>) { + let mut collector = Collector { + tcx, + body, temps: IndexVec::from_elem(TempState::Undefined, &body.local_decls), + candidates: vec![], span: body.span, - body, }; for (bb, data) in rpo { collector.visit_basic_block_data(bb, data); } - collector.temps + (collector.temps, collector.candidates) +} + +struct Validator<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + body: &'a Body<'tcx>, + is_static: bool, + 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>, + + /// Explicit promotion happens e.g. for constant arguments declared via + /// `rustc_args_required_const`. + /// Implicit promotion has almost the same rules, except that disallows `const fn` + /// except for those marked `#[rustc_promotable]`. This is to avoid changing + /// a legitimate run-time operation into a failing compile-time operation + /// e.g. due to addresses being compared inside the function. + explicit: bool, +} + +struct Unpromotable; + +impl<'tcx> Validator<'_, 'tcx> { + fn is_const_panic_fn(&self, def_id: DefId) -> bool { + Some(def_id) == self.tcx.lang_items().panic_fn() || + Some(def_id) == self.tcx.lang_items().begin_panic_fn() + } + + fn validate_candidate(&self, candidate: Candidate) -> Result<(), Unpromotable> { + match candidate { + Candidate::Ref(loc) => { + assert!(!self.explicit); + + let statement = &self.body[loc.block].statements[loc.statement_index]; + match &statement.kind { + StatementKind::Assign(box(_, Rvalue::Ref(_, kind, place))) => { + match kind { + BorrowKind::Shared | BorrowKind::Mut { .. } => {} + + // FIXME(eddyb) these aren't promoted here but *could* + // be promoted as part of a larger value because + // `validate_rvalue` doesn't check them, need to + // figure out what is the intended behavior. + BorrowKind::Shallow | BorrowKind::Unique => return Err(Unpromotable), + } + + // We can only promote interior borrows of promotable temps (non-temps + // don't get promoted anyway). + let base = match place.base { + PlaceBase::Local(local) => local, + _ => return Err(Unpromotable), + }; + 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); + // HACK(eddyb) this should compute the same thing as + // `<HasMutInterior as Qualif>::in_projection` from + // `qualify_consts` but without recursion. + if has_mut_interior { + // This allows borrowing fields which don't have + // `HasMutInterior`, from a type that does, e.g.: + // `let _: &'static _ = &(Cell::new(1), 2).1;` + let mut place_projection = &place.projection[..]; + // FIXME(eddyb) use a forward loop instead of a reverse one. + while let [proj_base @ .., elem] = place_projection { + // FIXME(eddyb) this is probably excessive, with + // the exception of `union` member accesses. + let ty = + Place::ty_from(&place.base, proj_base, self.body, self.tcx) + .projection_ty(self.tcx, elem) + .ty; + if ty.is_freeze(self.tcx, self.param_env, DUMMY_SP) { + has_mut_interior = false; + break; + } + + place_projection = proj_base; + } + } + + // FIXME(eddyb) this duplicates part of `validate_rvalue`. + if has_mut_interior { + return Err(Unpromotable); + } + // FIXME(eddyb) compute this on the fly. + if self.needs_drop.contains(base) { + return Err(Unpromotable); + } + if let BorrowKind::Mut { .. } = kind { + let ty = place.ty(self.body, self.tcx).ty; + + // In theory, any zero-sized value could be borrowed + // mutably without consequences. However, only &mut [] + // is allowed right now, and only in functions. + if self.is_static_mut { + // Inside a `static mut`, &mut [...] is also allowed. + match ty.kind { + ty::Array(..) | ty::Slice(_) => {} + _ => return Err(Unpromotable), + } + } else if let ty::Array(_, len) = ty.kind { + // FIXME(eddyb) the `self.is_non_const_fn` condition + // seems unnecessary, given that this is merely a ZST. + match len.try_eval_usize(self.tcx, self.param_env) { + Some(0) if self.is_non_const_fn => {}, + _ => return Err(Unpromotable), + } + } else { + return Err(Unpromotable); + } + } + + self.validate_local(base) + } + _ => bug!() + } + } + Candidate::Repeat(loc) => { + assert!(!self.explicit); + + let statement = &self.body[loc.block].statements[loc.statement_index]; + match &statement.kind { + StatementKind::Assign(box(_, Rvalue::Repeat(ref operand, _))) => { + if !self.tcx.features().const_in_array_repeat_expressions { + return Err(Unpromotable); + } + + self.validate_operand(operand) + } + _ => bug!() + } + }, + Candidate::Argument { bb, index } => { + assert!(self.explicit); + + let terminator = self.body[bb].terminator(); + match &terminator.kind { + TerminatorKind::Call { args, .. } => { + self.validate_operand(&args[index]) + } + _ => bug!() + } + } + } + } + + // FIXME(eddyb) maybe cache this? + fn validate_local(&self, local: Local) -> Result<(), Unpromotable> { + 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)) => self.validate_rvalue(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, .. } => self.validate_call(func, args), + kind => { + span_bug!(terminator.source_info.span, "{:?} not promotable", kind); + } + } + } + } else { + Err(Unpromotable) + } + } + + fn validate_place(&self, place: PlaceRef<'_, 'tcx>) -> Result<(), Unpromotable> { + match place { + PlaceRef { + base: PlaceBase::Local(local), + projection: [], + } => self.validate_local(*local), + PlaceRef { + base: PlaceBase::Static(box Static { + kind: StaticKind::Promoted { .. }, + .. + }), + projection: [], + } => bug!("qualifying already promoted MIR"), + PlaceRef { + base: PlaceBase::Static(box Static { + kind: StaticKind::Static, + def_id, + .. + }), + projection: [], + } => { + // Only allow statics (not consts) to refer to other statics. + // FIXME(eddyb) does this matter at all for promotion? + let allowed = self.is_static || self.is_static_mut; + if !allowed { + return Err(Unpromotable); + } + + let is_thread_local = self.tcx.has_attr(*def_id, sym::thread_local); + if is_thread_local { + return Err(Unpromotable); + } + + Ok(()) + } + PlaceRef { + base: _, + projection: [proj_base @ .., elem], + } => { + match *elem { + ProjectionElem::Deref | + ProjectionElem::Downcast(..) => return Err(Unpromotable), + + ProjectionElem::ConstantIndex {..} | + ProjectionElem::Subslice {..} => {} + + ProjectionElem::Index(local) => { + self.validate_local(local)?; + } + + ProjectionElem::Field(..) => { + if self.is_non_const_fn { + let base_ty = + Place::ty_from(place.base, proj_base, self.body, self.tcx).ty; + if let Some(def) = base_ty.ty_adt_def() { + // No promotion of union field accesses. + if def.is_union() { + return Err(Unpromotable); + } + } + } + } + } + + self.validate_place(PlaceRef { + base: place.base, + projection: proj_base, + }) + } + } + } + + fn validate_operand(&self, operand: &Operand<'tcx>) -> Result<(), Unpromotable> { + match operand { + Operand::Copy(place) | + Operand::Move(place) => self.validate_place(place.as_ref()), + + Operand::Constant(constant) => { + if let ConstValue::Unevaluated(def_id, _) = constant.literal.val { + if self.tcx.trait_of_item(def_id).is_some() { + // Don't peek inside trait associated constants. + // (see below what we do for other consts, for now) + } else { + // HACK(eddyb) ensure that errors propagate correctly. + // FIXME(eddyb) remove this once the old promotion logic + // is gone - we can always promote constants even if they + // fail to pass const-checking, as compilation would've + // errored independently and promotion can't change that. + let (bits, _) = self.tcx.at(constant.span).mir_const_qualif(def_id); + if bits == super::qualify_consts::QUALIF_ERROR_BIT { + self.tcx.sess.delay_span_bug( + constant.span, + "promote_consts: MIR had errors", + ); + return Err(Unpromotable); + } + } + } + + Ok(()) + } + } + } + + fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> { + match *rvalue { + Rvalue::Cast(CastKind::Misc, ref operand, cast_ty) if self.is_non_const_fn => { + let operand_ty = operand.ty(self.body, self.tcx); + let cast_in = CastTy::from_ty(operand_ty).expect("bad input type for cast"); + let cast_out = CastTy::from_ty(cast_ty).expect("bad output type for cast"); + match (cast_in, cast_out) { + (CastTy::Ptr(_), CastTy::Int(_)) | + (CastTy::FnPtr, CastTy::Int(_)) => { + // in normal functions, mark such casts as not promotable + return Err(Unpromotable); + } + _ => {} + } + } + + Rvalue::BinaryOp(op, ref lhs, _) if self.is_non_const_fn => { + if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.body, self.tcx).kind { + assert!(op == BinOp::Eq || op == BinOp::Ne || + op == BinOp::Le || op == BinOp::Lt || + op == BinOp::Ge || op == BinOp::Gt || + op == BinOp::Offset); + + // raw pointer operations are not allowed inside promoteds + return Err(Unpromotable); + } + } + + Rvalue::NullaryOp(NullOp::Box, _) => return Err(Unpromotable), + + _ => {} + } + + match rvalue { + Rvalue::NullaryOp(..) => Ok(()), + + Rvalue::Discriminant(place) | + Rvalue::Len(place) => self.validate_place(place.as_ref()), + + Rvalue::Use(operand) | + Rvalue::Repeat(operand, _) | + Rvalue::UnaryOp(_, operand) | + Rvalue::Cast(_, operand, _) => self.validate_operand(operand), + + Rvalue::BinaryOp(_, lhs, rhs) | + Rvalue::CheckedBinaryOp(_, lhs, rhs) => { + self.validate_operand(lhs)?; + self.validate_operand(rhs) + } + + Rvalue::Ref(_, kind, place) => { + if let BorrowKind::Mut { .. } = kind { + let ty = place.ty(self.body, self.tcx).ty; + + // In theory, any zero-sized value could be borrowed + // mutably without consequences. However, only &mut [] + // is allowed right now, and only in functions. + if self.is_static_mut { + // Inside a `static mut`, &mut [...] is also allowed. + match ty.kind { + ty::Array(..) | ty::Slice(_) => {} + _ => return Err(Unpromotable), + } + } else if let ty::Array(_, len) = ty.kind { + // FIXME(eddyb) the `self.is_non_const_fn` condition + // seems unnecessary, given that this is merely a ZST. + match len.try_eval_usize(self.tcx, self.param_env) { + Some(0) if self.is_non_const_fn => {}, + _ => return Err(Unpromotable), + } + } else { + return Err(Unpromotable); + } + } + + // Special-case reborrows to be more like a copy of the reference. + let mut place = place.as_ref(); + if let [proj_base @ .., ProjectionElem::Deref] = &place.projection { + let base_ty = + Place::ty_from(&place.base, proj_base, self.body, self.tcx).ty; + if let ty::Ref(..) = base_ty.kind { + place = PlaceRef { + base: &place.base, + projection: proj_base, + }; + } + } + + // HACK(eddyb) this should compute the same thing as + // `<HasMutInterior as Qualif>::in_projection` from + // `qualify_consts` 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) + } + PlaceBase::Static(_) => false, + }; + if has_mut_interior { + let mut place_projection = place.projection; + // FIXME(eddyb) use a forward loop instead of a reverse one. + while let [proj_base @ .., elem] = place_projection { + // FIXME(eddyb) this is probably excessive, with + // the exception of `union` member accesses. + let ty = Place::ty_from(place.base, proj_base, self.body, self.tcx) + .projection_ty(self.tcx, elem) + .ty; + if ty.is_freeze(self.tcx, self.param_env, DUMMY_SP) { + has_mut_interior = false; + break; + } + + place_projection = proj_base; + } + } + if has_mut_interior { + return Err(Unpromotable); + } + + self.validate_place(place) + } + + Rvalue::Aggregate(_, ref operands) => { + for o in operands { + self.validate_operand(o)?; + } + + Ok(()) + } + } + } + + fn validate_call( + &self, + callee: &Operand<'tcx>, + args: &[Operand<'tcx>], + ) -> Result<(), Unpromotable> { + let fn_ty = callee.ty(self.body, self.tcx); + + if !self.explicit && self.is_non_const_fn { + if let ty::FnDef(def_id, _) = fn_ty.kind { + // Never promote runtime `const fn` calls of + // functions without `#[rustc_promotable]`. + if !self.tcx.is_promotable_const_fn(def_id) { + return Err(Unpromotable); + } + } + } + + let is_const_fn = match fn_ty.kind { + ty::FnDef(def_id, _) => { + self.tcx.is_const_fn(def_id) || + self.tcx.is_unstable_const_fn(def_id).is_some() || + self.is_const_panic_fn(def_id) + } + _ => false, + }; + if !is_const_fn { + return Err(Unpromotable); + } + + self.validate_operand(callee)?; + for arg in args { + self.validate_operand(arg)?; + } + + Ok(()) + } +} + +pub fn validate_candidates( + tcx: TyCtxt<'tcx>, + 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 { + tcx, + param_env: tcx.param_env(def_id), + body, + is_static: false, + is_static_mut: false, + is_non_const_fn: false, + temps, + // FIXME(eddyb) compute these 2 on the fly. + has_mut_interior, + needs_drop, + + explicit: false, + }; + + // FIXME(eddyb) remove the distinctions that make this necessary. + let id = tcx.hir().as_local_hir_id(def_id).unwrap(); + match tcx.hir().body_owner_kind(id) { + hir::BodyOwnerKind::Closure => validator.is_non_const_fn = true, + hir::BodyOwnerKind::Fn => { + if !tcx.is_const_fn(def_id) { + validator.is_non_const_fn = true; + } + }, + hir::BodyOwnerKind::Static(hir::MutImmutable) => validator.is_static = true, + hir::BodyOwnerKind::Static(hir::MutMutable) => validator.is_static_mut = true, + _ => {} + } + + candidates.iter().copied().filter(|&candidate| { + validator.explicit = match candidate { + Candidate::Ref(_) | + Candidate::Repeat(_) => false, + Candidate::Argument { .. } => true, + }; + + // FIXME(eddyb) also emit the errors for shuffle indices + // and `#[rustc_args_required_const]` arguments here. + + validator.validate_candidate(candidate).is_ok() + }).collect() } struct Promoter<'a, 'tcx> { @@ -215,17 +791,17 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> { self.temps[temp] = TempState::PromotedOut; } - let no_stmts = self.source[loc.block].statements.len(); + let num_stmts = self.source[loc.block].statements.len(); let new_temp = self.promoted.local_decls.push( LocalDecl::new_temp(self.source.local_decls[temp].ty, self.source.local_decls[temp].source_info.span)); debug!("promote({:?} @ {:?}/{:?}, {:?})", - temp, loc, no_stmts, self.keep_original); + temp, loc, num_stmts, self.keep_original); // First, take the Rvalue or Call out of the source MIR, // or duplicate it, depending on keep_original. - if loc.statement_index < no_stmts { + if loc.statement_index < num_stmts { let (mut rvalue, source_info) = { let statement = &mut self.source[loc.block].statements[loc.statement_index]; let rhs = match statement.kind { diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 6aba91f4162..1c5c84c6923 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -206,6 +206,9 @@ trait Qualif { ProjectionElem::ConstantIndex { .. } | ProjectionElem::Downcast(..) => qualif, + // FIXME(eddyb) shouldn't this be masked *after* including the + // index local? Then again, it's `usize` which is neither + // `HasMutInterior` nor `NeedsDrop`. ProjectionElem::Index(local) => qualif || Self::in_local(cx, *local), } } else { @@ -442,6 +445,7 @@ impl Qualif for IsNotPromotable { StaticKind::Promoted(_, _) => unreachable!(), StaticKind::Static => { // Only allow statics (not consts) to refer to other statics. + // FIXME(eddyb) does this matter at all for promotion? let allowed = cx.mode == Mode::Static || cx.mode == Mode::StaticMut; !allowed || @@ -674,6 +678,7 @@ struct Checker<'a, 'tcx> { temp_promotion_state: IndexVec<Local, TempState>, promotion_candidates: Vec<Candidate>, + unchecked_promotion_candidates: Vec<Candidate>, /// If `true`, do not emit errors to the user, merely collect them in `errors`. suppress_errors: bool, @@ -703,7 +708,8 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { fn new(tcx: TyCtxt<'tcx>, def_id: DefId, body: &'a Body<'tcx>, mode: Mode) -> Self { assert!(def_id.is_local()); let mut rpo = traversal::reverse_postorder(body); - let temps = promote_consts::collect_temps(body, &mut rpo); + let (temps, unchecked_promotion_candidates) = + promote_consts::collect_temps_and_candidates(tcx, body, &mut rpo); rpo.reset(); let param_env = tcx.param_env(def_id); @@ -742,6 +748,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { rpo, temp_promotion_state: temps, promotion_candidates: vec![], + unchecked_promotion_candidates, errors: vec![], suppress_errors: false, } @@ -828,6 +835,10 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { } else if let BorrowKind::Mut { .. } | BorrowKind::Shared = kind { // Don't promote BorrowKind::Shallow borrows, as they don't // reach codegen. + // FIXME(eddyb) the two other kinds of borrow (`Shallow` and `Unique`) + // aren't promoted here but *could* be promoted as part of a larger + // value because `IsNotPromotable` isn't being set for them, + // need to figure out what is the intended behavior. // We might have a candidate for promotion. let candidate = Candidate::Ref(location); @@ -967,6 +978,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { let mut seen_blocks = BitSet::new_empty(body.basic_blocks().len()); let mut bb = START_BLOCK; + let mut has_controlflow_error = false; loop { seen_blocks.insert(bb.index()); @@ -1007,6 +1019,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { bb = target; } _ => { + has_controlflow_error = true; self.not_const(ops::Loop); validator.check_op(ops::Loop); break; @@ -1036,9 +1049,28 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { // Collect all the temps we need to promote. let mut promoted_temps = BitSet::new_empty(self.temp_promotion_state.len()); - debug!("qualify_const: promotion_candidates={:?}", self.promotion_candidates); - for candidate in &self.promotion_candidates { - match *candidate { + // HACK(eddyb) don't try to validate promotion candidates if any + // parts of the control-flow graph were skipped due to an error. + let promotion_candidates = if has_controlflow_error { + let unleash_miri = self + .tcx + .sess + .opts + .debugging_opts + .unleash_the_miri_inside_of_you; + if !unleash_miri { + self.tcx.sess.delay_span_bug( + body.span, + "check_const: expected control-flow error(s)", + ); + } + self.promotion_candidates.clone() + } else { + self.valid_promotion_candidates() + }; + debug!("qualify_const: promotion_candidates={:?}", promotion_candidates); + for candidate in promotion_candidates { + match candidate { Candidate::Repeat(Location { block: bb, statement_index: stmt_idx }) => { if let StatementKind::Assign(box(_, Rvalue::Repeat( Operand::Move(place), @@ -1075,6 +1107,51 @@ impl<'a, 'tcx> Checker<'a, 'tcx> { (qualifs.encode_to_bits(), self.tcx.arena.alloc(promoted_temps)) } + + /// Get the subset of `unchecked_promotion_candidates` that are eligible + /// for promotion. + // FIXME(eddyb) replace the old candidate gathering with this. + fn valid_promotion_candidates(&self) -> Vec<Candidate> { + // Sanity-check the promotion candidates. + let candidates = promote_consts::validate_candidates( + self.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, + ); + + if candidates != self.promotion_candidates { + let report = |msg, candidate| { + let span = match candidate { + Candidate::Ref(loc) | + Candidate::Repeat(loc) => self.body.source_info(loc).span, + Candidate::Argument { bb, .. } => { + self.body[bb].terminator().source_info.span + } + }; + self.tcx.sess.span_err(span, &format!("{}: {:?}", msg, candidate)); + }; + + for &c in &self.promotion_candidates { + if !candidates.contains(&c) { + report("invalidated old candidate", c); + } + } + + for &c in &candidates { + if !self.promotion_candidates.contains(&c) { + report("extra new candidate", c); + } + } + + bug!("promotion candidate validation mismatches (see above)"); + } + + candidates + } } impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> { @@ -1644,6 +1721,10 @@ pub fn provide(providers: &mut Providers<'_>) { }; } +// FIXME(eddyb) this is only left around for the validation logic +// in `promote_consts`, see the comment in `validate_operand`. +pub(super) const QUALIF_ERROR_BIT: u8 = 1 << IsNotPromotable::IDX; + fn mir_const_qualif(tcx: TyCtxt<'_>, def_id: DefId) -> (u8, &BitSet<Local>) { // N.B., this `borrow()` is guaranteed to be valid (i.e., the value // cannot yet be stolen), because `mir_validated()`, which steals @@ -1653,7 +1734,7 @@ fn mir_const_qualif(tcx: TyCtxt<'_>, def_id: DefId) -> (u8, &BitSet<Local>) { if body.return_ty().references_error() { tcx.sess.delay_span_bug(body.span, "mir_const_qualif: MIR had errors"); - return (1 << IsNotPromotable::IDX, tcx.arena.alloc(BitSet::new_empty(0))); + return (QUALIF_ERROR_BIT, tcx.arena.alloc(BitSet::new_empty(0))); } Checker::new(tcx, def_id, body, Mode::Const).check_const() @@ -1695,29 +1776,33 @@ impl<'tcx> MirPass<'tcx> for QualifyAndPromoteConstants<'tcx> { let (temps, candidates) = { let mut checker = Checker::new(tcx, def_id, body, mode); if let Mode::ConstFn = mode { - if tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { - checker.check_const(); - } else if tcx.is_min_const_fn(def_id) { + let use_min_const_fn_checks = + !tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you && + tcx.is_min_const_fn(def_id); + if use_min_const_fn_checks { // Enforce `min_const_fn` for stable `const fn`s. use super::qualify_min_const_fn::is_min_const_fn; if let Err((span, err)) = is_min_const_fn(tcx, def_id, body) { error_min_const_fn_violation(tcx, span, err); - } else { - // this should not produce any errors, but better safe than sorry - // FIXME(#53819) - checker.check_const(); + return; } - } else { - // Enforce a constant-like CFG for `const fn`. - checker.check_const(); + + // `check_const` should not produce any errors, but better safe than sorry + // FIXME(#53819) + // NOTE(eddyb) `check_const` is actually needed for promotion inside + // `min_const_fn` functions. } + + // Enforce a constant-like CFG for `const fn`. + checker.check_const(); } else { while let Some((bb, data)) = checker.rpo.next() { checker.visit_basic_block_data(bb, data); } } - (checker.temp_promotion_state, checker.promotion_candidates) + let promotion_candidates = checker.valid_promotion_candidates(); + (checker.temp_promotion_state, promotion_candidates) }; // Do the actual promotion, now that we know what's viable. |