From 0918dc4e5990592c28484d1d64bec41cb9a4a301 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 21 Sep 2019 15:49:15 +0200 Subject: [PATCH] or-patterns: middle/dead: remove `top_pats_hack`. Also tweak walkers on `Pat`. --- src/librustc/hir/mod.rs | 62 ++++++++++++------- src/librustc/hir/pat_util.rs | 112 ++++++++++++++++------------------- src/librustc/middle/dead.rs | 21 +++---- 3 files changed, 100 insertions(+), 95 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index edb55ab3caaf4..87fc385acd64d 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -882,39 +882,61 @@ impl fmt::Debug for Pat { impl Pat { // FIXME(#19596) this is a workaround, but there should be a better way - fn walk_(&self, it: &mut G) -> bool - where G: FnMut(&Pat) -> bool - { + fn walk_short_(&self, it: &mut impl FnMut(&Pat) -> bool) -> bool { if !it(self) { return false; } + use PatKind::*; match &self.node { - PatKind::Binding(.., Some(p)) => p.walk_(it), - PatKind::Struct(_, fields, _) => fields.iter().all(|field| field.pat.walk_(it)), - PatKind::TupleStruct(_, s, _) | PatKind::Tuple(s, _) | PatKind::Or(s) => { - s.iter().all(|p| p.walk_(it)) - } - PatKind::Box(s) | PatKind::Ref(s, _) => s.walk_(it), - PatKind::Slice(before, slice, after) => { + Wild | Lit(_) | Range(..) | Binding(.., None) | Path(_) => true, + Box(s) | Ref(s, _) | Binding(.., Some(s)) => s.walk_short_(it), + Struct(_, fields, _) => fields.iter().all(|field| field.pat.walk_short_(it)), + TupleStruct(_, s, _) | Tuple(s, _) | Or(s) => s.iter().all(|p| p.walk_short_(it)), + Slice(before, slice, after) => { before.iter() .chain(slice.iter()) .chain(after.iter()) - .all(|p| p.walk_(it)) + .all(|p| p.walk_short_(it)) } - PatKind::Wild | - PatKind::Lit(_) | - PatKind::Range(..) | - PatKind::Binding(..) | - PatKind::Path(_) => { - true + } + } + + /// Walk the pattern in left-to-right order, + /// short circuiting (with `.all(..)`) if `false` is returned. + /// + /// Note that when visiting e.g. `Tuple(ps)`, + /// if visiting `ps[0]` returns `false`, + /// then `ps[1]` will not be visited. + pub fn walk_short(&self, mut it: impl FnMut(&Pat) -> bool) -> bool { + self.walk_short_(&mut it) + } + + // FIXME(#19596) this is a workaround, but there should be a better way + fn walk_(&self, it: &mut impl FnMut(&Pat) -> bool) { + if !it(self) { + return; + } + + use PatKind::*; + match &self.node { + Wild | Lit(_) | Range(..) | Binding(.., None) | Path(_) => {}, + Box(s) | Ref(s, _) | Binding(.., Some(s)) => s.walk_(it), + Struct(_, fields, _) => fields.iter().for_each(|field| field.pat.walk_(it)), + TupleStruct(_, s, _) | Tuple(s, _) | Or(s) => s.iter().for_each(|p| p.walk_(it)), + Slice(before, slice, after) => { + before.iter() + .chain(slice.iter()) + .chain(after.iter()) + .for_each(|p| p.walk_(it)) } } } - pub fn walk(&self, mut it: F) -> bool - where F: FnMut(&Pat) -> bool - { + /// Walk the pattern in left-to-right order. + /// + /// If `it(pat)` returns `false`, the children are not visited. + pub fn walk(&self, mut it: impl FnMut(&Pat) -> bool) { self.walk_(&mut it) } } diff --git a/src/librustc/hir/pat_util.rs b/src/librustc/hir/pat_util.rs index ea35418bc1ba7..118e168f87767 100644 --- a/src/librustc/hir/pat_util.rs +++ b/src/librustc/hir/pat_util.rs @@ -66,9 +66,7 @@ impl hir::Pat { /// Call `f` on every "binding" in a pattern, e.g., on `a` in /// `match foo() { Some(a) => (), None => () }` - pub fn each_binding(&self, mut f: F) - where F: FnMut(hir::BindingAnnotation, HirId, Span, ast::Ident), - { + pub fn each_binding(&self, mut f: impl FnMut(hir::BindingAnnotation, HirId, Span, ast::Ident)) { self.walk(|p| { if let PatKind::Binding(binding_mode, _, ident, _) = p.node { f(binding_mode, p.hir_id, p.span, ident); @@ -81,59 +79,53 @@ impl hir::Pat { /// `match foo() { Some(a) => (), None => () }`. /// /// When encountering an or-pattern `p_0 | ... | p_n` only `p_0` will be visited. - pub fn each_binding_or_first(&self, c: &mut F) - where F: FnMut(hir::BindingAnnotation, HirId, Span, ast::Ident), - { - match &self.node { - PatKind::Binding(bm, _, ident, sub) => { - c(*bm, self.hir_id, self.span, *ident); - sub.iter().for_each(|p| p.each_binding_or_first(c)); - } - PatKind::Or(ps) => ps[0].each_binding_or_first(c), - PatKind::Struct(_, fs, _) => fs.iter().for_each(|f| f.pat.each_binding_or_first(c)), - PatKind::TupleStruct(_, ps, _) | PatKind::Tuple(ps, _) => { - ps.iter().for_each(|p| p.each_binding_or_first(c)); - } - PatKind::Box(p) | PatKind::Ref(p, _) => p.each_binding_or_first(c), - PatKind::Slice(before, slice, after) => { - before.iter() - .chain(slice.iter()) - .chain(after.iter()) - .for_each(|p| p.each_binding_or_first(c)); + pub fn each_binding_or_first( + &self, + f: &mut impl FnMut(hir::BindingAnnotation, HirId, Span, ast::Ident), + ) { + self.walk(|p| match &p.node { + PatKind::Or(ps) => { + ps[0].each_binding_or_first(f); + false + }, + PatKind::Binding(bm, _, ident, _) => { + f(*bm, p.hir_id, p.span, *ident); + true } - PatKind::Wild | PatKind::Lit(_) | PatKind::Range(..) | PatKind::Path(_) => {} - } + _ => true, + }) } /// Checks if the pattern contains any patterns that bind something to /// an ident, e.g., `foo`, or `Foo(foo)` or `foo @ Bar(..)`. pub fn contains_bindings(&self) -> bool { - let mut contains_bindings = false; - self.walk(|p| { - if let PatKind::Binding(..) = p.node { - contains_bindings = true; - false // there's at least one binding, can short circuit now. - } else { - true - } - }); - contains_bindings + self.satisfies(|p| match p.node { + PatKind::Binding(..) => true, + _ => false, + }) } /// Checks if the pattern contains any patterns that bind something to /// an ident or wildcard, e.g., `foo`, or `Foo(_)`, `foo @ Bar(..)`, pub fn contains_bindings_or_wild(&self) -> bool { - let mut contains_bindings = false; - self.walk(|p| { - match p.node { - PatKind::Binding(..) | PatKind::Wild => { - contains_bindings = true; - false // there's at least one binding/wildcard, can short circuit now. - } - _ => true + self.satisfies(|p| match p.node { + PatKind::Binding(..) | PatKind::Wild => true, + _ => false, + }) + } + + /// Checks if the pattern satisfies the given predicate on some sub-pattern. + fn satisfies(&self, pred: impl Fn(&Self) -> bool) -> bool { + let mut satisfies = false; + self.walk_short(|p| { + if pred(p) { + satisfies = true; + false // Found one, can short circuit now. + } else { + true } }); - contains_bindings + satisfies } pub fn simple_ident(&self) -> Option { @@ -147,20 +139,20 @@ impl hir::Pat { /// Returns variants that are necessary to exist for the pattern to match. pub fn necessary_variants(&self) -> Vec { let mut variants = vec![]; - self.walk(|p| { - match p.node { - PatKind::Path(hir::QPath::Resolved(_, ref path)) | - PatKind::TupleStruct(hir::QPath::Resolved(_, ref path), ..) | - PatKind::Struct(hir::QPath::Resolved(_, ref path), ..) => { - match path.res { - Res::Def(DefKind::Variant, id) => variants.push(id), - Res::Def(DefKind::Ctor(CtorOf::Variant, ..), id) => variants.push(id), - _ => () - } + self.walk(|p| match &p.node { + PatKind::Or(_) => false, + PatKind::Path(hir::QPath::Resolved(_, path)) | + PatKind::TupleStruct(hir::QPath::Resolved(_, path), ..) | + PatKind::Struct(hir::QPath::Resolved(_, path), ..) => { + if let Res::Def(DefKind::Variant, id) + | Res::Def(DefKind::Ctor(CtorOf::Variant, ..), id) + = path.res + { + variants.push(id); } - _ => () + true } - true + _ => true, }); variants.sort(); variants.dedup(); @@ -176,14 +168,12 @@ impl hir::Pat { let mut result = None; self.each_binding(|annotation, _, _, _| { match annotation { - hir::BindingAnnotation::Ref => { - match result { - None | Some(hir::MutImmutable) => result = Some(hir::MutImmutable), - _ => (), - } + hir::BindingAnnotation::Ref => match result { + None | Some(hir::MutImmutable) => result = Some(hir::MutImmutable), + _ => {} } hir::BindingAnnotation::RefMut => result = Some(hir::MutMutable), - _ => (), + _ => {} } }); result diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index 1818dede3e229..2746794e2a2c1 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -259,20 +259,13 @@ impl<'a, 'tcx> Visitor<'tcx> for MarkSymbolVisitor<'a, 'tcx> { } fn visit_arm(&mut self, arm: &'tcx hir::Arm) { - let pats = arm.top_pats_hack(); - if let [pat] = pats { - let variants = pat.necessary_variants(); - - // Inside the body, ignore constructions of variants - // necessary for the pattern to match. Those construction sites - // can't be reached unless the variant is constructed elsewhere. - let len = self.ignore_variant_stack.len(); - self.ignore_variant_stack.extend_from_slice(&variants); - intravisit::walk_arm(self, arm); - self.ignore_variant_stack.truncate(len); - } else { - intravisit::walk_arm(self, arm); - } + // Inside the body, ignore constructions of variants + // necessary for the pattern to match. Those construction sites + // can't be reached unless the variant is constructed elsewhere. + let len = self.ignore_variant_stack.len(); + self.ignore_variant_stack.extend(arm.pat.necessary_variants()); + intravisit::walk_arm(self, arm); + self.ignore_variant_stack.truncate(len); } fn visit_pat(&mut self, pat: &'tcx hir::Pat) {