From c56b45da11866a7cfb9eafd60b6a3675d4f5acab Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Sun, 11 Sep 2016 12:30:22 -0700 Subject: [PATCH 1/4] Remove unnecessary use of P --- src/librustc_const_eval/check_match.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/librustc_const_eval/check_match.rs b/src/librustc_const_eval/check_match.rs index 9281d8aa44a56..e49262ae84d98 100644 --- a/src/librustc_const_eval/check_match.rs +++ b/src/librustc_const_eval/check_match.rs @@ -134,7 +134,7 @@ pub enum Constructor { #[derive(Clone, PartialEq)] enum Usefulness { Useful, - UsefulWithWitness(Vec>), + UsefulWithWitness(Vec), NotUseful } @@ -403,7 +403,7 @@ fn check_exhaustive<'a, 'tcx>(cx: &MatchCheckCtxt<'a, 'tcx>, let witnesses = if pats.is_empty() { vec![DUMMY_WILD_PAT] } else { - pats.iter().map(|w| &**w).collect() + pats.iter().collect() }; match source { hir::MatchSource::ForLoopDesugar => { @@ -577,7 +577,7 @@ impl<'a, 'tcx> StaticInliner<'a, 'tcx> { /// left_ty: struct X { a: (bool, &'static str), b: usize} /// pats: [(false, "foo"), 42] => X { a: (false, "foo"), b: 42 } fn construct_witness<'a,'tcx>(cx: &MatchCheckCtxt<'a,'tcx>, ctor: &Constructor, - pats: Vec<&Pat>, left_ty: Ty<'tcx>) -> P { + pats: Vec<&Pat>, left_ty: Ty<'tcx>) -> Pat { let pats_len = pats.len(); let mut pats = pats.into_iter().map(|p| P((*p).clone())); let pat = match left_ty.sty { @@ -636,11 +636,11 @@ fn construct_witness<'a,'tcx>(cx: &MatchCheckCtxt<'a,'tcx>, ctor: &Constructor, } }; - P(hir::Pat { + hir::Pat { id: DUMMY_NODE_ID, node: pat, span: DUMMY_SP - }) + } } impl Constructor { @@ -737,9 +737,8 @@ fn is_useful<'a, 'tcx>(cx: &MatchCheckCtxt<'a, 'tcx>, UsefulWithWitness(pats) => UsefulWithWitness({ let arity = constructor_arity(cx, &c, left_ty); let mut result = { - let pat_slice = &pats[..]; let subpats: Vec<_> = (0..arity).map(|i| { - pat_slice.get(i).map_or(DUMMY_WILD_PAT, |p| &**p) + pats.get(i).map_or(DUMMY_WILD_PAT, |p| &*p) }).collect(); vec![construct_witness(cx, &c, subpats, left_ty)] }; From 81a263467a29b6196ff29b7565d6c57dcca1cdcf Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 14 Sep 2016 08:16:43 -0700 Subject: [PATCH 2/4] Reduce rightward drift with if lets --- src/librustc_const_eval/check_match.rs | 164 ++++++++++++------------- 1 file changed, 79 insertions(+), 85 deletions(-) diff --git a/src/librustc_const_eval/check_match.rs b/src/librustc_const_eval/check_match.rs index e49262ae84d98..e769ab7f1e695 100644 --- a/src/librustc_const_eval/check_match.rs +++ b/src/librustc_const_eval/check_match.rs @@ -171,78 +171,75 @@ fn create_e0004<'a>(sess: &'a Session, sp: Span, error_message: String) -> Diagn fn check_expr(cx: &mut MatchCheckCtxt, ex: &hir::Expr) { intravisit::walk_expr(cx, ex); - match ex.node { - hir::ExprMatch(ref scrut, ref arms, source) => { - for arm in arms { - // First, check legality of move bindings. - check_legality_of_move_bindings(cx, - arm.guard.is_some(), - &arm.pats); - - // Second, if there is a guard on each arm, make sure it isn't - // assigning or borrowing anything mutably. - if let Some(ref guard) = arm.guard { - check_for_mutation_in_guard(cx, &guard); - } + if let hir::ExprMatch(ref scrut, ref arms, source) = ex.node { + for arm in arms { + // First, check legality of move bindings. + check_legality_of_move_bindings(cx, + arm.guard.is_some(), + &arm.pats); + + // Second, if there is a guard on each arm, make sure it isn't + // assigning or borrowing anything mutably. + if let Some(ref guard) = arm.guard { + check_for_mutation_in_guard(cx, &guard); } + } - let mut static_inliner = StaticInliner::new(cx.tcx); - let inlined_arms = arms.iter().map(|arm| { - (arm.pats.iter().map(|pat| { - static_inliner.fold_pat((*pat).clone()) - }).collect(), arm.guard.as_ref().map(|e| &**e)) - }).collect::>, Option<&hir::Expr>)>>(); + let mut static_inliner = StaticInliner::new(cx.tcx); + let inlined_arms = arms.iter().map(|arm| { + (arm.pats.iter().map(|pat| { + static_inliner.fold_pat((*pat).clone()) + }).collect(), arm.guard.as_ref().map(|e| &**e)) + }).collect::>, Option<&hir::Expr>)>>(); - // Bail out early if inlining failed. - if static_inliner.failed { - return; - } + // Bail out early if inlining failed. + if static_inliner.failed { + return; + } - for pat in inlined_arms - .iter() - .flat_map(|&(ref pats, _)| pats) { - // Third, check legality of move bindings. - check_legality_of_bindings_in_at_patterns(cx, &pat); + for pat in inlined_arms + .iter() + .flat_map(|&(ref pats, _)| pats) { + // Third, check legality of move bindings. + check_legality_of_bindings_in_at_patterns(cx, &pat); - // Fourth, check if there are any references to NaN that we should warn about. - check_for_static_nan(cx, &pat); + // Fourth, check if there are any references to NaN that we should warn about. + check_for_static_nan(cx, &pat); - // Fifth, check if for any of the patterns that match an enumerated type - // are bindings with the same name as one of the variants of said type. - check_for_bindings_named_the_same_as_variants(cx, &pat); - } + // Fifth, check if for any of the patterns that match an enumerated type + // are bindings with the same name as one of the variants of said type. + check_for_bindings_named_the_same_as_variants(cx, &pat); + } - // Fourth, check for unreachable arms. - check_arms(cx, &inlined_arms[..], source); - - // Finally, check if the whole match expression is exhaustive. - // Check for empty enum, because is_useful only works on inhabited types. - let pat_ty = cx.tcx.node_id_to_type(scrut.id); - if inlined_arms.is_empty() { - if !pat_ty.is_uninhabited(cx.tcx) { - // We know the type is inhabited, so this must be wrong - let mut err = create_e0004(cx.tcx.sess, ex.span, - format!("non-exhaustive patterns: type {} \ - is non-empty", - pat_ty)); - span_help!(&mut err, ex.span, - "Please ensure that all possible cases are being handled; \ - possibly adding wildcards or more match arms."); - err.emit(); - } - // If the type *is* uninhabited, it's vacuously exhaustive - return; + // Fourth, check for unreachable arms. + check_arms(cx, &inlined_arms[..], source); + + // Finally, check if the whole match expression is exhaustive. + // Check for empty enum, because is_useful only works on inhabited types. + let pat_ty = cx.tcx.node_id_to_type(scrut.id); + if inlined_arms.is_empty() { + if !pat_ty.is_uninhabited(cx.tcx) { + // We know the type is inhabited, so this must be wrong + let mut err = create_e0004(cx.tcx.sess, ex.span, + format!("non-exhaustive patterns: type {} \ + is non-empty", + pat_ty)); + span_help!(&mut err, ex.span, + "Please ensure that all possible cases are being handled; \ + possibly adding wildcards or more match arms."); + err.emit(); } + // If the type *is* uninhabited, it's vacuously exhaustive + return; + } - let matrix: Matrix = inlined_arms - .iter() - .filter(|&&(_, guard)| guard.is_none()) - .flat_map(|arm| &arm.0) - .map(|pat| vec![wrap_pat(cx, &pat)]) - .collect(); - check_exhaustive(cx, scrut.span, &matrix, source); - }, - _ => () + let matrix: Matrix = inlined_arms + .iter() + .filter(|&&(_, guard)| guard.is_none()) + .flat_map(|arm| &arm.0) + .map(|pat| vec![wrap_pat(cx, &pat)]) + .collect(); + check_exhaustive(cx, scrut.span, &matrix, source); } } @@ -489,33 +486,30 @@ impl<'a, 'tcx> StaticInliner<'a, 'tcx> { impl<'a, 'tcx> StaticInliner<'a, 'tcx> { fn fold_pat(&mut self, pat: P) -> P { - match pat.node { - PatKind::Path(..) => { - match self.tcx.expect_def(pat.id) { - Def::AssociatedConst(did) | Def::Const(did) => { - let substs = Some(self.tcx.node_id_item_substs(pat.id).substs); - if let Some((const_expr, _)) = lookup_const_by_id(self.tcx, did, substs) { - match const_expr_to_pat(self.tcx, const_expr, pat.id, pat.span) { - Ok(new_pat) => return new_pat, - Err(def_id) => { - self.failed = true; - self.tcx.sess.span_err( - pat.span, - &format!("constants of the type `{}` \ - cannot be used in patterns", - self.tcx.item_path_str(def_id))); - } + if let PatKind::Path(..) = pat.node { + match self.tcx.expect_def(pat.id) { + Def::AssociatedConst(did) | Def::Const(did) => { + let substs = Some(self.tcx.node_id_item_substs(pat.id).substs); + if let Some((const_expr, _)) = lookup_const_by_id(self.tcx, did, substs) { + match const_expr_to_pat(self.tcx, const_expr, pat.id, pat.span) { + Ok(new_pat) => return new_pat, + Err(def_id) => { + self.failed = true; + self.tcx.sess.span_err( + pat.span, + &format!("constants of the type `{}` \ + cannot be used in patterns", + self.tcx.item_path_str(def_id))); } - } else { - self.failed = true; - span_err!(self.tcx.sess, pat.span, E0158, - "statics cannot be referenced in patterns"); } + } else { + self.failed = true; + span_err!(self.tcx.sess, pat.span, E0158, + "statics cannot be referenced in patterns"); } - _ => {} } + _ => {} } - _ => {} } pat.map(|Pat { id, node, span }| { From 4873d2795a48b7a908d2a636fe513c4d2135a63f Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Sun, 9 Oct 2016 16:37:05 -0700 Subject: [PATCH 3/4] Remove unused exports --- src/librustc_const_eval/check_match.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustc_const_eval/check_match.rs b/src/librustc_const_eval/check_match.rs index e769ab7f1e695..97d190e08e209 100644 --- a/src/librustc_const_eval/check_match.rs +++ b/src/librustc_const_eval/check_match.rs @@ -46,7 +46,7 @@ use syntax::ptr::P; use syntax::util::move_map::MoveMap; use rustc::util::common::ErrorReported; -pub const DUMMY_WILD_PAT: &'static Pat = &Pat { +const DUMMY_WILD_PAT: &'static Pat = &Pat { id: DUMMY_NODE_ID, node: PatKind::Wild, span: DUMMY_SP @@ -109,13 +109,13 @@ impl<'a, 'tcx> FromIterator>)>> for Matrix<'a, 'tc } //NOTE: appears to be the only place other then InferCtxt to contain a ParamEnv -pub struct MatchCheckCtxt<'a, 'tcx: 'a> { - pub tcx: TyCtxt<'a, 'tcx, 'tcx>, - pub param_env: ty::ParameterEnvironment<'tcx>, +struct MatchCheckCtxt<'a, 'tcx: 'a> { + tcx: TyCtxt<'a, 'tcx, 'tcx>, + param_env: ty::ParameterEnvironment<'tcx>, } #[derive(Clone, Debug, PartialEq)] -pub enum Constructor { +enum Constructor { /// The constructor of all patterns that don't vary by constructor, /// e.g. struct patterns and fixed-length arrays. Single, @@ -476,7 +476,7 @@ struct StaticInliner<'a, 'tcx: 'a> { } impl<'a, 'tcx> StaticInliner<'a, 'tcx> { - pub fn new<'b>(tcx: TyCtxt<'b, 'tcx, 'tcx>) -> StaticInliner<'b, 'tcx> { + fn new<'b>(tcx: TyCtxt<'b, 'tcx, 'tcx>) -> StaticInliner<'b, 'tcx> { StaticInliner { tcx: tcx, failed: false @@ -837,7 +837,7 @@ fn pat_constructors(cx: &MatchCheckCtxt, p: &Pat, /// /// For instance, a tuple pattern (_, 42, Some([])) has the arity of 3. /// A struct pattern's arity is the number of fields it contains, etc. -pub fn constructor_arity(_cx: &MatchCheckCtxt, ctor: &Constructor, ty: Ty) -> usize { +fn constructor_arity(_cx: &MatchCheckCtxt, ctor: &Constructor, ty: Ty) -> usize { debug!("constructor_arity({:?}, {:?})", ctor, ty); match ty.sty { ty::TyTuple(ref fs) => fs.len(), @@ -892,7 +892,7 @@ fn wrap_pat<'a, 'b, 'tcx>(cx: &MatchCheckCtxt<'b, 'tcx>, /// different patterns. /// Structure patterns with a partial wild pattern (Foo { a: 42, .. }) have their missing /// fields filled with wild patterns. -pub fn specialize<'a, 'b, 'tcx>( +fn specialize<'a, 'b, 'tcx>( cx: &MatchCheckCtxt<'b, 'tcx>, r: &[(&'a Pat, Option>)], constructor: &Constructor, col: usize, arity: usize) From d4e989c628251b1feda5672e4246b1749bfe3d05 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Sun, 9 Oct 2016 16:53:16 -0700 Subject: [PATCH 4/4] Remove unused Constructor variant --- src/librustc_const_eval/check_match.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/librustc_const_eval/check_match.rs b/src/librustc_const_eval/check_match.rs index 97d190e08e209..5b4d142b1a594 100644 --- a/src/librustc_const_eval/check_match.rs +++ b/src/librustc_const_eval/check_match.rs @@ -126,9 +126,7 @@ enum Constructor { /// Ranges of literal values (2..5). ConstantRange(ConstVal, ConstVal), /// Array patterns of length n. - Slice(usize), - /// Array patterns with a subslice. - SliceWithSubslice(usize, usize) + Slice(usize) } #[derive(Clone, PartialEq)] @@ -1027,16 +1025,6 @@ fn specialize<'a, 'b, 'tcx>( after.iter().map(|p| wpat(p)) ).collect()) } - SliceWithSubslice(prefix, suffix) - if before.len() == prefix - && after.len() == suffix - && slice.is_some() => { - // this is used by trans::_match only - let mut pats: Vec<_> = before.iter() - .map(|p| (&**p, None)).collect(); - pats.extend(after.iter().map(|p| (&**p, None))); - Some(pats) - } _ => None } }