From 798be90648d88f902f6381c859df451d7ddf05d2 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 8 Jun 2016 19:26:19 +0300 Subject: [PATCH] introduce an `unreachable` terminator Use it instead of a `panic` for inexhaustive matches and correct the comment. I think we trust our match-generation algorithm enough to generate these blocks, and not generating an `unreachable` means that LLVM won't optimize `match void() {}` to an `unreachable`. --- src/librustc/mir/repr.rs | 8 +- src/librustc/mir/visit.rs | 3 +- .../borrowck/mir/dataflow/mod.rs | 3 +- .../borrowck/mir/gather_moves.rs | 4 +- src/librustc_mir/build/matches/mod.rs | 16 ++-- src/librustc_mir/build/scope.rs | 84 +------------------ src/librustc_mir/transform/no_landing_pads.rs | 1 + src/librustc_mir/transform/qualify_consts.rs | 3 +- src/librustc_mir/transform/type_check.rs | 2 + src/librustc_trans/mir/analyze.rs | 1 + src/librustc_trans/mir/block.rs | 4 + 11 files changed, 37 insertions(+), 92 deletions(-) diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs index dad330aa59ead..03ae91fefb925 100644 --- a/src/librustc/mir/repr.rs +++ b/src/librustc/mir/repr.rs @@ -374,6 +374,9 @@ pub enum TerminatorKind<'tcx> { /// have been filled in by now. This should occur at most once. Return, + /// Indicates a terminator that can never be reached. + Unreachable, + /// Drop the Lvalue Drop { location: Lvalue<'tcx>, @@ -432,6 +435,7 @@ impl<'tcx> TerminatorKind<'tcx> { SwitchInt { targets: ref b, .. } => b[..].into_cow(), Resume => (&[]).into_cow(), Return => (&[]).into_cow(), + Unreachable => (&[]).into_cow(), Call { destination: Some((_, t)), cleanup: Some(c), .. } => vec![t, c].into_cow(), Call { destination: Some((_, ref t)), cleanup: None, .. } => slice::ref_slice(t).into_cow(), @@ -461,6 +465,7 @@ impl<'tcx> TerminatorKind<'tcx> { SwitchInt { targets: ref mut b, .. } => b.iter_mut().collect(), Resume => Vec::new(), Return => Vec::new(), + Unreachable => Vec::new(), Call { destination: Some((_, ref mut t)), cleanup: Some(ref mut c), .. } => vec![t, c], Call { destination: Some((_, ref mut t)), cleanup: None, .. } => vec![t], Call { destination: None, cleanup: Some(ref mut c), .. } => vec![c], @@ -539,6 +544,7 @@ impl<'tcx> TerminatorKind<'tcx> { SwitchInt { discr: ref lv, .. } => write!(fmt, "switchInt({:?})", lv), Return => write!(fmt, "return"), Resume => write!(fmt, "resume"), + Unreachable => write!(fmt, "unreachable"), Drop { ref location, .. } => write!(fmt, "drop({:?})", location), DropAndReplace { ref location, ref value, .. } => write!(fmt, "replace({:?} <- {:?})", location, value), @@ -582,7 +588,7 @@ impl<'tcx> TerminatorKind<'tcx> { pub fn fmt_successor_labels(&self) -> Vec> { use self::TerminatorKind::*; match *self { - Return | Resume => vec![], + Return | Resume | Unreachable => vec![], Goto { .. } => vec!["".into()], If { .. } => vec!["true".into(), "false".into()], Switch { ref adt_def, .. } => { diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index fcc7715a8af31..bc45a730c2e21 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -386,7 +386,8 @@ macro_rules! make_mir_visitor { } TerminatorKind::Resume | - TerminatorKind::Return => { + TerminatorKind::Return | + TerminatorKind::Unreachable => { } TerminatorKind::Drop { ref $($mutability)* location, diff --git a/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs b/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs index d42103d5d25a2..a9b4de450967c 100644 --- a/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs +++ b/src/librustc_borrowck/borrowck/mir/dataflow/mod.rs @@ -449,7 +449,8 @@ impl<'a, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> { match bb_data.terminator().kind { repr::TerminatorKind::Return | - repr::TerminatorKind::Resume => {} + repr::TerminatorKind::Resume | + repr::TerminatorKind::Unreachable => {} repr::TerminatorKind::Goto { ref target } | repr::TerminatorKind::Assert { ref target, cleanup: None, .. } | repr::TerminatorKind::Drop { ref target, location: _, unwind: None } | diff --git a/src/librustc_borrowck/borrowck/mir/gather_moves.rs b/src/librustc_borrowck/borrowck/mir/gather_moves.rs index 7a3f467a627f8..05412216d487c 100644 --- a/src/librustc_borrowck/borrowck/mir/gather_moves.rs +++ b/src/librustc_borrowck/borrowck/mir/gather_moves.rs @@ -621,7 +621,9 @@ fn gather_moves<'a, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> MoveD debug!("gather_moves({:?})", bb_data.terminator()); match bb_data.terminator().kind { - TerminatorKind::Goto { target: _ } | TerminatorKind::Resume => { } + TerminatorKind::Goto { target: _ } | + TerminatorKind::Resume | + TerminatorKind::Unreachable => { } TerminatorKind::Return => { let source = Location { block: bb, diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index ab102460603da..c57913c8e6068 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -78,12 +78,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // branch to the appropriate arm block let otherwise = self.match_candidates(span, &mut arm_blocks, candidates, block); - // because all matches are exhaustive, in principle we expect - // an empty vector to be returned here, but the algorithm is - // not entirely precise if !otherwise.is_empty() { - let join_block = self.join_otherwise_blocks(span, otherwise); - self.panic(join_block, "something about matches algorithm not being precise", span); + // All matches are exhaustive. However, because some matches + // only have exponentially-large exhaustive decision trees, we + // sometimes generate an inexhaustive decision tree. + // + // In that case, the inexhaustive tips of the decision tree + // can't be reached - terminate them with an `unreachable`. + let source_info = self.source_info(span); + + for block in otherwise { + self.cfg.terminate(block, source_info, TerminatorKind::Unreachable); + } } // all the arm blocks will rejoin here diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 2c34b13671b0c..9cc6b60eec005 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -90,12 +90,9 @@ use build::{BlockAnd, BlockAndExtension, Builder, CFG, ScopeAuxiliary, ScopeId}; use rustc::middle::region::{CodeExtent, CodeExtentData}; use rustc::middle::lang_items; use rustc::ty::subst::{Substs, Subst, VecPerParamSpace}; -use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::{Ty, TyCtxt}; use rustc::mir::repr::*; -use syntax::codemap::{Span, DUMMY_SP}; -use syntax::parse::token::intern_and_get_ident; -use rustc::middle::const_val::ConstVal; -use rustc_const_math::ConstInt; +use syntax::codemap::Span; use rustc_data_structures::indexed_vec::Idx; pub struct Scope<'tcx> { @@ -556,50 +553,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { next_target.unit() } - /// Create diverge cleanup and branch to it from `block`. - // FIXME: Remove this (used only for unreachable cases in match). - pub fn panic(&mut self, block: BasicBlock, msg: &'static str, span: Span) { - // fn(&(msg: &'static str filename: &'static str, line: u32)) -> ! - let region = ty::ReStatic; // FIXME(mir-borrowck): use a better region? - let func = self.lang_function(lang_items::PanicFnLangItem); - let args = self.hir.tcx().replace_late_bound_regions(&func.ty.fn_args(), |_| region).0; - - let ref_ty = args[0]; - let tup_ty = if let ty::TyRef(_, tyandmut) = ref_ty.sty { - tyandmut.ty - } else { - span_bug!(span, "unexpected panic type: {:?}", func.ty); - }; - - let (tuple, tuple_ref) = (self.temp(tup_ty), self.temp(ref_ty)); - let (file, line) = self.span_to_fileline_args(span); - let message = Constant { - span: span, - ty: self.hir.tcx().mk_static_str(), - literal: self.hir.str_literal(intern_and_get_ident(msg)) - }; - let elems = vec![Operand::Constant(message), - Operand::Constant(file), - Operand::Constant(line)]; - let source_info = self.source_info(span); - // FIXME: We should have this as a constant, rather than a stack variable (to not pollute - // icache with cold branch code), however to achieve that we either have to rely on rvalue - // promotion or have some way, in MIR, to create constants. - self.cfg.push_assign(block, source_info, &tuple, // [1] - Rvalue::Aggregate(AggregateKind::Tuple, elems)); - // [1] tuple = (message_arg, file_arg, line_arg); - // FIXME: is this region really correct here? - self.cfg.push_assign(block, source_info, &tuple_ref, // tuple_ref = &tuple; - Rvalue::Ref(region, BorrowKind::Shared, tuple)); - let cleanup = self.diverge_cleanup(); - self.cfg.terminate(block, source_info, TerminatorKind::Call { - func: Operand::Constant(func), - args: vec![Operand::Consume(tuple_ref)], - cleanup: cleanup, - destination: None, - }); - } - /// Create an Assert terminator and return the success block. /// If the boolean condition operand is not the expected value, /// a runtime panic will be caused with the given message. @@ -625,39 +578,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { success_block } - - fn lang_function(&mut self, lang_item: lang_items::LangItem) -> Constant<'tcx> { - let funcdid = match self.hir.tcx().lang_items.require(lang_item) { - Ok(d) => d, - Err(m) => { - self.hir.tcx().sess.fatal(&m) - } - }; - Constant { - span: DUMMY_SP, - ty: self.hir.tcx().lookup_item_type(funcdid).ty, - literal: Literal::Item { - def_id: funcdid, - substs: self.hir.tcx().mk_substs(Substs::empty()) - } - } - } - - fn span_to_fileline_args(&mut self, span: Span) -> (Constant<'tcx>, Constant<'tcx>) { - let span_lines = self.hir.tcx().sess.codemap().lookup_char_pos(span.lo); - (Constant { - span: span, - ty: self.hir.tcx().mk_static_str(), - literal: self.hir.str_literal(intern_and_get_ident(&span_lines.file.name)) - }, Constant { - span: span, - ty: self.hir.tcx().types.u32, - literal: Literal::Value { - value: ConstVal::Integral(ConstInt::U32(span_lines.line as u32)), - }, - }) - } - } /// Builds drops for pop_scope and exit_scope. diff --git a/src/librustc_mir/transform/no_landing_pads.rs b/src/librustc_mir/transform/no_landing_pads.rs index 590106e0a225b..818f060ed445c 100644 --- a/src/librustc_mir/transform/no_landing_pads.rs +++ b/src/librustc_mir/transform/no_landing_pads.rs @@ -24,6 +24,7 @@ impl<'tcx> MutVisitor<'tcx> for NoLandingPads { TerminatorKind::Goto { .. } | TerminatorKind::Resume | TerminatorKind::Return | + TerminatorKind::Unreachable | TerminatorKind::If { .. } | TerminatorKind::Switch { .. } | TerminatorKind::SwitchInt { .. } => { diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 73ecac553894c..5ea5949e10703 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -362,7 +362,8 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { TerminatorKind::Switch {..} | TerminatorKind::SwitchInt {..} | TerminatorKind::DropAndReplace { .. } | - TerminatorKind::Resume => None, + TerminatorKind::Resume | + TerminatorKind::Unreachable => None, TerminatorKind::Return => { // Check for unused values. This usually means diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 16ea4780bb346..e4398fcab3163 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -379,6 +379,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { TerminatorKind::Goto { .. } | TerminatorKind::Resume | TerminatorKind::Return | + TerminatorKind::Unreachable | TerminatorKind::Drop { .. } => { // no checks needed for these } @@ -595,6 +596,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { span_mirbug!(self, block, "return on cleanup block") } } + TerminatorKind::Unreachable => {} TerminatorKind::Drop { target, unwind, .. } | TerminatorKind::DropAndReplace { target, unwind, .. } | TerminatorKind::Assert { target, cleanup: unwind, .. } => { diff --git a/src/librustc_trans/mir/analyze.rs b/src/librustc_trans/mir/analyze.rs index df00abf7b1c5f..d1c1053ac46b2 100644 --- a/src/librustc_trans/mir/analyze.rs +++ b/src/librustc_trans/mir/analyze.rs @@ -173,6 +173,7 @@ pub fn cleanup_kinds<'bcx,'tcx>(_bcx: Block<'bcx,'tcx>, TerminatorKind::Goto { .. } | TerminatorKind::Resume | TerminatorKind::Return | + TerminatorKind::Unreachable | TerminatorKind::If { .. } | TerminatorKind::Switch { .. } | TerminatorKind::SwitchInt { .. } => { diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs index 8dabce2ec4eb9..bdcf3dd8cd418 100644 --- a/src/librustc_trans/mir/block.rs +++ b/src/librustc_trans/mir/block.rs @@ -191,6 +191,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { }) } + mir::TerminatorKind::Unreachable => { + bcx.unreachable(); + } + mir::TerminatorKind::Drop { ref location, target, unwind } => { let lvalue = self.trans_lvalue(&bcx, location); let ty = lvalue.ty.to_ty(bcx.tcx());