diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs index 1651e71c49a2a..5e67abaa4739c 100644 --- a/src/librustc/middle/const_eval.rs +++ b/src/librustc/middle/const_eval.rs @@ -39,6 +39,7 @@ use std::borrow::{Cow, IntoCow}; use std::num::wrapping::OverflowingOps; use std::cmp::Ordering; use std::collections::hash_map::Entry::Vacant; +use std::hash; use std::mem::transmute; use std::{i8, i16, i32, i64, u8, u16, u32, u64}; use std::rc::Rc; @@ -257,6 +258,22 @@ pub enum ConstVal { Function(DefId), } +impl hash::Hash for ConstVal { + fn hash(&self, state: &mut H) { + match *self { + Float(a) => unsafe { transmute::<_,u64>(a) }.hash(state), + Int(a) => a.hash(state), + Uint(a) => a.hash(state), + Str(ref a) => a.hash(state), + ByteStr(ref a) => a.hash(state), + Bool(a) => a.hash(state), + Struct(a) => a.hash(state), + Tuple(a) => a.hash(state), + Function(a) => a.hash(state), + } + } +} + /// Note that equality for `ConstVal` means that the it is the same /// constant, not that the rust values are equal. In particular, `NaN /// == NaN` (at least if it's the same NaN; distinct encodings for NaN @@ -278,6 +295,8 @@ impl PartialEq for ConstVal { } } +impl Eq for ConstVal { } + impl ConstVal { pub fn description(&self) -> &'static str { match *self { diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 6e0b05d5dea79..e72d903756a1d 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -15,6 +15,8 @@ use build::{BlockAnd, Builder}; use repr::*; +use rustc_data_structures::fnv::FnvHashMap; +use rustc::middle::const_eval::ConstVal; use rustc::middle::region::CodeExtent; use rustc::middle::ty::{AdtDef, Ty}; use hair::*; @@ -241,6 +243,13 @@ enum TestKind<'tcx> { adt_def: AdtDef<'tcx>, }, + // test the branches of enum + SwitchInt { + switch_ty: Ty<'tcx>, + options: Vec, + indices: FnvHashMap, + }, + // test for equality Eq { value: Literal<'tcx>, @@ -315,20 +324,36 @@ impl<'a,'tcx> Builder<'a,'tcx> { // otherwise, extract the next match pair and construct tests let match_pair = &candidates.last().unwrap().match_pairs[0]; - let test = self.test(match_pair); + let mut test = self.test(match_pair); + + // most of the time, the test to perform is simply a function + // of the main candidate; but for a test like SwitchInt, we + // may want to add cases based on the candidates that are + // available + match test.kind { + TestKind::SwitchInt { switch_ty, ref mut options, ref mut indices } => { + for candidate in &candidates { + self.add_cases_to_switch(&match_pair.lvalue, + candidate, + switch_ty, + options, + indices); + } + } + _ => { } + } + debug!("match_candidates: test={:?} match_pair={:?}", test, match_pair); let target_blocks = self.perform_test(block, &match_pair.lvalue, &test); - for (outcome, mut target_block) in target_blocks.into_iter().enumerate() { + for (outcome, target_block) in target_blocks.into_iter().enumerate() { let applicable_candidates: Vec> = candidates.iter() .filter_map(|candidate| { - unpack!(target_block = - self.candidate_under_assumption(target_block, - &match_pair.lvalue, - &test.kind, - outcome, - candidate)) + self.candidate_under_assumption(&match_pair.lvalue, + &test.kind, + outcome, + candidate) }) .collect(); self.match_candidates(span, arm_blocks, applicable_candidates, target_block); diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index e035f53dacf41..280b5f1d67ff7 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -15,10 +15,13 @@ // identify what tests are needed, perform the tests, and then filter // the candidates based on the result. -use build::{BlockAnd, Builder}; +use build::Builder; use build::matches::{Candidate, MatchPair, Test, TestKind}; use hair::*; use repr::*; +use rustc_data_structures::fnv::FnvHashMap; +use rustc::middle::const_eval::ConstVal; +use rustc::middle::ty::Ty; use syntax::codemap::Span; impl<'a,'tcx> Builder<'a,'tcx> { @@ -34,13 +37,31 @@ impl<'a,'tcx> Builder<'a,'tcx> { } } + PatternKind::Constant { value: Literal::Value { .. } } + if is_switch_ty(match_pair.pattern.ty) => { + // for integers, we use a SwitchInt match, which allows + // us to handle more cases + Test { + span: match_pair.pattern.span, + kind: TestKind::SwitchInt { + switch_ty: match_pair.pattern.ty, + + // these maps are empty to start; cases are + // added below in add_cases_to_switch + options: vec![], + indices: FnvHashMap(), + } + } + } + PatternKind::Constant { ref value } => { + // for other types, we use an equality comparison Test { span: match_pair.pattern.span, kind: TestKind::Eq { value: value.clone(), ty: match_pair.pattern.ty.clone(), - }, + } } } @@ -78,13 +99,52 @@ impl<'a,'tcx> Builder<'a,'tcx> { } } + pub fn add_cases_to_switch(&mut self, + test_lvalue: &Lvalue<'tcx>, + candidate: &Candidate<'tcx>, + switch_ty: Ty<'tcx>, + options: &mut Vec, + indices: &mut FnvHashMap) + { + let match_pair = match candidate.match_pairs.iter().find(|mp| mp.lvalue == *test_lvalue) { + Some(match_pair) => match_pair, + _ => { return; } + }; + + match match_pair.pattern.kind { + PatternKind::Constant { value: Literal::Value { ref value } } => { + // if the lvalues match, the type should match + assert_eq!(match_pair.pattern.ty, switch_ty); + + indices.entry(value.clone()) + .or_insert_with(|| { + options.push(value.clone()); + options.len() - 1 + }); + } + + PatternKind::Range { .. } => { + } + + PatternKind::Constant { .. } | + PatternKind::Variant { .. } | + PatternKind::Slice { .. } | + PatternKind::Array { .. } | + PatternKind::Wild | + PatternKind::Binding { .. } | + PatternKind::Leaf { .. } | + PatternKind::Deref { .. } => { + } + } + } + /// Generates the code to perform a test. pub fn perform_test(&mut self, block: BasicBlock, lvalue: &Lvalue<'tcx>, test: &Test<'tcx>) -> Vec { - match test.kind.clone() { + match test.kind { TestKind::Switch { adt_def } => { let num_enum_variants = self.hir.num_variants(adt_def); let target_blocks: Vec<_> = @@ -98,18 +158,34 @@ impl<'a,'tcx> Builder<'a,'tcx> { target_blocks } - TestKind::Eq { value, ty } => { + TestKind::SwitchInt { switch_ty, ref options, indices: _ } => { + let otherwise = self.cfg.start_new_block(); + let targets: Vec<_> = + options.iter() + .map(|_| self.cfg.start_new_block()) + .chain(Some(otherwise)) + .collect(); + self.cfg.terminate(block, Terminator::SwitchInt { + discr: lvalue.clone(), + switch_ty: switch_ty, + values: options.clone(), + targets: targets.clone(), + }); + targets + } + + TestKind::Eq { ref value, ty } => { // call PartialEq::eq(discrim, constant) - let constant = self.literal_operand(test.span, ty.clone(), value); + let constant = self.literal_operand(test.span, ty.clone(), value.clone()); let item_ref = self.hir.partial_eq(ty); self.call_comparison_fn(block, test.span, item_ref, Operand::Consume(lvalue.clone()), constant) } - TestKind::Range { lo, hi, ty } => { + TestKind::Range { ref lo, ref hi, ty } => { // Test `v` by computing `PartialOrd::le(lo, v) && PartialOrd::le(v, hi)`. - let lo = self.literal_operand(test.span, ty.clone(), lo); - let hi = self.literal_operand(test.span, ty.clone(), hi); + let lo = self.literal_operand(test.span, ty.clone(), lo.clone()); + let hi = self.literal_operand(test.span, ty.clone(), hi.clone()); let item_ref = self.hir.partial_le(ty); let lo_blocks = self.call_comparison_fn(block, @@ -207,34 +283,31 @@ impl<'a,'tcx> Builder<'a,'tcx> { /// were `Ok`, we would return `Some([x.0.downcast.0 @ P1, x.1 /// @ 22])`. pub fn candidate_under_assumption(&mut self, - mut block: BasicBlock, test_lvalue: &Lvalue<'tcx>, test_kind: &TestKind<'tcx>, test_outcome: usize, candidate: &Candidate<'tcx>) - -> BlockAnd>> { + -> Option> { let candidate = candidate.clone(); let match_pairs = candidate.match_pairs; - let result = unpack!(block = self.match_pairs_under_assumption(block, - test_lvalue, - test_kind, - test_outcome, - match_pairs)); - block.and(match result { + let result = self.match_pairs_under_assumption(test_lvalue, + test_kind, + test_outcome, + match_pairs); + match result { Some(match_pairs) => Some(Candidate { match_pairs: match_pairs, ..candidate }), None => None, - }) + } } /// Helper for candidate_under_assumption that does the actual /// work of transforming the list of match pairs. fn match_pairs_under_assumption(&mut self, - mut block: BasicBlock, test_lvalue: &Lvalue<'tcx>, test_kind: &TestKind<'tcx>, test_outcome: usize, match_pairs: Vec>) - -> BlockAnd>>> { + -> Option>> { let mut result = vec![]; for match_pair in match_pairs { @@ -245,30 +318,17 @@ impl<'a,'tcx> Builder<'a,'tcx> { continue; } - let desired_test = self.test(&match_pair); - - if *test_kind != desired_test.kind { - // if the match pair wants to (e.g.) test for - // equality against some particular constant, but - // we did a switch, then we can't say whether it - // matches or not, so we still have to include it - // as a possibility. - // - // For example, we have a constant `FOO: - // Option = Some(22)`, and `match_pair` is `x - // @ FOO`, but we did a switch on the variant - // (`Some` vs `None`). (OK, in principle this - // could tell us something, but we're not that - // smart yet to actually dig into the constant - // itself) + // if this test doesn't tell us anything about this match-pair, then hang onto it. + if !self.test_informs_match_pair(&match_pair, test_kind, test_outcome) { result.push(match_pair); continue; } + // otherwise, build up the consequence match pairs let opt_consequent_match_pairs = - unpack!(block = self.consequent_match_pairs_under_assumption(block, - match_pair, - test_outcome)); + self.consequent_match_pairs_under_assumption(match_pair, + test_kind, + test_outcome); match opt_consequent_match_pairs { None => { // Right kind of test, but wrong outcome. That @@ -276,7 +336,7 @@ impl<'a,'tcx> Builder<'a,'tcx> { // inapplicable, since the candidate is only // applicable if all of its match-pairs apply (and // this one doesn't). - return block.and(None); + return None; } Some(consequent_match_pairs) => { @@ -285,23 +345,122 @@ impl<'a,'tcx> Builder<'a,'tcx> { } } } - block.and(Some(result)) + + Some(result) } - /// Identifies what test is needed to decide if `match_pair` is applicable. + /// Given that we executed `test` to `match_pair.lvalue` with + /// outcome `test_outcome`, does that tell us anything about + /// whether `match_pair` applies? /// - /// It is a bug to call this with a simplifyable pattern. + /// Often it does not. For example, if we are testing whether + /// the discriminant equals 4, and we find that it does not, + /// but the `match_pair` is testing if the discriminant equals 5, + /// that does not help us. + fn test_informs_match_pair(&mut self, + match_pair: &MatchPair<'tcx>, + test_kind: &TestKind<'tcx>, + _test_outcome: usize) + -> bool { + match match_pair.pattern.kind { + PatternKind::Variant { .. } => { + match *test_kind { + TestKind::Switch { .. } => true, + _ => false, + } + } + + PatternKind::Constant { value: Literal::Value { .. } } + if is_switch_ty(match_pair.pattern.ty) => { + match *test_kind { + TestKind::SwitchInt { .. } => true, + + // Did not do an integer equality test (which is always a SwitchInt). + // So we learned nothing relevant to this match-pair. + // + // FIXME(#29623) we could use TestKind::Range to rule + // things out here, in some cases. + _ => false, + } + } + + PatternKind::Constant { .. } | + PatternKind::Range { .. } | + PatternKind::Slice { .. } => { + let pattern_test = self.test(&match_pair); + if pattern_test.kind == *test_kind { + true + } else { + // FIXME(#29623) in all 3 cases, we could sometimes do + // better here. For example, if we are checking + // whether the value is equal to X, and we find + // that it is, that (may) imply value is not equal + // to Y. Or, if the range tested is `3..5`, and + // our range is `4..5`, then we know that our + // range also does not apply. Similarly, if we + // test that length is >= 5, and it fails, we also + // know that length is not >= 7. etc. + false + } + } + + PatternKind::Array { .. } | + PatternKind::Wild | + PatternKind::Binding { .. } | + PatternKind::Leaf { .. } | + PatternKind::Deref { .. } => { + self.error_simplifyable(&match_pair) + } + } + } + + /// Given that we executed `test` with outcome `test_outcome`, + /// what are the resulting match pairs? This can return either: + /// + /// - None, meaning that the test indicated that this outcome + /// means that this match-pair is not the current one for the + /// current discriminant (which rules out the enclosing + /// candidate); + /// - Some(...), meaning that either the test didn't tell us whether this + /// match-pair is correct or not, or that we DID match and now have + /// subsequent matches to perform. + /// + /// As an example, consider: + /// + /// ``` + /// match option { + /// Ok() => ..., + /// Err(_) => ..., + /// } + /// ``` + /// + /// Suppose that the `test` is a `Switch` and the outcome is + /// `Ok`. Then in that case, the first arm will have a match-pair + /// of `option @ Ok()`. In that case we will return + /// `Some(vec![(option as Ok) @ ])`. The `Some` reuslt + /// indicates that the match-pair still applies, and we now have + /// to test `(option as Ok) @ `. + /// + /// On the second arm, a `None` will be returned, because if we + /// observed that `option` has the discriminant `Ok`, then the + /// second arm cannot apply. pub fn consequent_match_pairs_under_assumption(&mut self, - mut block: BasicBlock, match_pair: MatchPair<'tcx>, + test_kind: &TestKind<'tcx>, test_outcome: usize) - -> BlockAnd>>> { + -> Option>> { match match_pair.pattern.kind { PatternKind::Variant { adt_def, variant_index, subpatterns } => { + assert!(match *test_kind { TestKind::Switch { .. } => true, + _ => false }); + if test_outcome != variant_index { - return block.and(None); + return None; // Tested, but found the wrong variant. } + // Correct variant. Extract the subitems and match + // those. The lvalue goes gets downcast, so + // e.g. `foo.bar` becomes `foo.bar as Variant`. let elem = ProjectionElem::Downcast(adt_def, variant_index); let downcast_lvalue = match_pair.lvalue.clone().elem(elem); let consequent_match_pairs = @@ -313,33 +472,37 @@ impl<'a,'tcx> Builder<'a,'tcx> { self.match_pair(lvalue, subpattern.pattern) }) .collect(); - block.and(Some(consequent_match_pairs)) + Some(consequent_match_pairs) } - PatternKind::Constant { .. } | - PatternKind::Range { .. } => { - // these are boolean tests: if we are on the 0th - // successor, then they passed, and otherwise they - // failed, but there are never any more tests to come. - if test_outcome == 0 { - block.and(Some(vec![])) - } else { - block.and(None) + PatternKind::Constant { value: Literal::Value { ref value } } + if is_switch_ty(match_pair.pattern.ty) => { + match *test_kind { + TestKind::SwitchInt { switch_ty: _, options: _, ref indices } => { + let index = indices[value]; + if index == test_outcome { + Some(vec![]) // this value, nothing left to test + } else { + None // some other value, candidate is inapplicable + } + } + + _ => { + self.hir.span_bug( + match_pair.pattern.span, + &format!("did a switch-int, but value {:?} not found in cases", + value)); + } } } - PatternKind::Slice { prefix, slice, suffix } => { + PatternKind::Constant { .. } | + PatternKind::Range { .. } | + PatternKind::Slice { .. } => { if test_outcome == 0 { - let mut consequent_match_pairs = vec![]; - unpack!(block = self.prefix_suffix_slice(&mut consequent_match_pairs, - block, - match_pair.lvalue, - prefix, - slice, - suffix)); - block.and(Some(consequent_match_pairs)) + Some(vec![]) } else { - block.and(None) + None } } @@ -358,3 +521,7 @@ impl<'a,'tcx> Builder<'a,'tcx> { &format!("simplifyable pattern found: {:?}", match_pair.pattern)) } } + +fn is_switch_ty<'tcx>(ty: Ty<'tcx>) -> bool { + ty.is_integral() || ty.is_char() +} diff --git a/src/librustc_mir/repr.rs b/src/librustc_mir/repr.rs index 89b1afa872381..2871ebcaafc9a 100644 --- a/src/librustc_mir/repr.rs +++ b/src/librustc_mir/repr.rs @@ -251,6 +251,26 @@ pub enum Terminator<'tcx> { targets: Vec, }, + /// operand evaluates to an integer; jump depending on its value + /// to one of the targets, and otherwise fallback to `otherwise` + SwitchInt { + /// discriminant value being tested + discr: Lvalue<'tcx>, + + /// type of value being tested + switch_ty: Ty<'tcx>, + + /// Possible values. The locations to branch to in each case + /// are found in the corresponding indices from the `targets` vector. + values: Vec, + + /// Possible branch sites. The length of this vector should be + /// equal to the length of the `values` vector plus 1 -- the + /// extra item is the block to branch to if none of the values + /// fit. + targets: Vec, + }, + /// Indicates that the last statement in the block panics, aborts, /// etc. No successors. This terminator appears on exactly one /// basic block which we create in advance. However, during @@ -280,7 +300,8 @@ impl<'tcx> Terminator<'tcx> { Goto { target: ref b } => slice::ref_slice(b), Panic { target: ref b } => slice::ref_slice(b), If { cond: _, targets: ref b } => b, - Switch { discr: _, adt_def: _, targets: ref b } => b, + Switch { targets: ref b, .. } => b, + SwitchInt { targets: ref b, .. } => b, Diverge => &[], Return => &[], Call { data: _, targets: ref b } => b, @@ -321,6 +342,8 @@ impl<'tcx> Debug for Terminator<'tcx> { write!(fmt, "if({:?}) -> {:?}", lv, targets), Switch { discr: ref lv, adt_def: _, ref targets } => write!(fmt, "switch({:?}) -> {:?}", lv, targets), + SwitchInt { discr: ref lv, switch_ty: _, ref values, ref targets } => + write!(fmt, "switchInt({:?}, {:?}) -> {:?}", lv, values, targets), Diverge => write!(fmt, "diverge"), Return => diff --git a/src/librustc_mir/visit.rs b/src/librustc_mir/visit.rs index b4d6075d0adb7..06a5c9fa9b7ae 100644 --- a/src/librustc_mir/visit.rs +++ b/src/librustc_mir/visit.rs @@ -109,6 +109,13 @@ pub trait Visitor<'tcx> { } } + Terminator::SwitchInt { ref discr, switch_ty: _, values: _, ref targets } => { + self.visit_lvalue(discr, LvalueContext::Inspect); + for &target in targets { + self.visit_branch(block, target); + } + } + Terminator::Diverge | Terminator::Return => { } diff --git a/src/librustc_trans/trans/mir/block.rs b/src/librustc_trans/trans/mir/block.rs index c9f4123c17127..b47e66ef7f55a 100644 --- a/src/librustc_trans/trans/mir/block.rs +++ b/src/librustc_trans/trans/mir/block.rs @@ -50,6 +50,10 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { unimplemented!() } + mir::Terminator::SwitchInt { .. } => { + unimplemented!() + } + mir::Terminator::Diverge => { if let Some(llpersonalityslot) = self.llpersonalityslot { let lp = build::Load(bcx, llpersonalityslot);