Skip to content

Commit

Permalink
Rollup merge of rust-lang#64508 - Centril:or-pat-hir, r=matthewjasper
Browse files Browse the repository at this point in the history
or-patterns: Push `PatKind/PatternKind::Or` at top level to HIR & HAIR

Following up on work in rust-lang#64111, rust-lang#63693, and rust-lang#61708, in this PR:

- We change `hair::Arm.patterns: Vec<Pattern<'_>>` into `hir::Arm.pattern: Pattern<'_>`.

   - `fn hair::Arm::top_pats_hack` is introduced as a temporary crutch in MIR building to avoid more changes.

- We change `hir::Arm.pats: HirVec<P<Pat>>` into `hir::Arm.pat: P<Pat>`.

   - The hacks in `rustc::hir::lowering` are removed since the representation hack is no longer necessary.

   - In some places, `fn hir::Arm::top_pats_hack` is introduced to leave some things as future work.

   - Misc changes: HIR pretty printing is adjusted to behave uniformly wrt. top/inner levels, rvalue promotion is adjusted, regionck, and dead_code is also.

   - Type checking is adjusted to uniformly handle or-patterns at top/inner levels.
      To make things compile, `p_0 | ... | p_n` is redefined as a "reference pattern" in [`fn is_non_ref_pat`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_typeck/check/struct.FnCtxt.html#method.is_non_ref_pat) for now. This is done so that reference types are not eagerly stripped from the `expected: Ty<'tcx>`.

    - Liveness is adjusted wrt. the `unused_variables` and `unused_assignments` lints to handle top/inner levels uniformly and the handling of `fn` parameters, `let` locals, and `match` arms are unified in this respect. This is not tested for now as exhaustiveness checks are reachable and will ICE.

    - In `check_match`, checking `@` and by-move bindings is adjusted. However, exhaustiveness checking is not adjusted the moment and is handled by @dlrobertson in rust-lang#63688.

    - AST borrowck (`construct.rs`) is not adjusted as AST borrowck will be removed soon.

r? @matthewjasper
cc @dlrobertson @varkor @oli-obk
  • Loading branch information
Centril authored Sep 25, 2019
2 parents b30238e + 0918dc4 commit da78127
Show file tree
Hide file tree
Showing 22 changed files with 373 additions and 485 deletions.
2 changes: 1 addition & 1 deletion src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1103,7 +1103,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {

pub fn walk_arm<'v, V: Visitor<'v>>(visitor: &mut V, arm: &'v Arm) {
visitor.visit_id(arm.hir_id);
walk_list!(visitor, visit_pat, &arm.pats);
visitor.visit_pat(&arm.pat);
if let Some(ref g) = arm.guard {
match g {
Guard::If(ref e) => visitor.visit_expr(e),
Expand Down
29 changes: 0 additions & 29 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,35 +434,6 @@ impl<'a> LoweringContext<'a> {
visit::walk_pat(self, p)
}

// HACK(or_patterns; Centril | dlrobertson): Avoid creating
// HIR nodes for `PatKind::Or` for the top level of a `ast::Arm`.
// This is a temporary hack that should go away once we push down
// `arm.pats: HirVec<P<Pat>>` -> `arm.pat: P<Pat>` to HIR. // Centril
fn visit_arm(&mut self, arm: &'tcx Arm) {
match &arm.pat.node {
PatKind::Or(pats) => pats.iter().for_each(|p| self.visit_pat(p)),
_ => self.visit_pat(&arm.pat),
}
walk_list!(self, visit_expr, &arm.guard);
self.visit_expr(&arm.body);
walk_list!(self, visit_attribute, &arm.attrs);
}

// HACK(or_patterns; Centril | dlrobertson): Same as above. // Centril
fn visit_expr(&mut self, e: &'tcx Expr) {
if let ExprKind::Let(pat, scrutinee) = &e.node {
walk_list!(self, visit_attribute, e.attrs.iter());
match &pat.node {
PatKind::Or(pats) => pats.iter().for_each(|p| self.visit_pat(p)),
_ => self.visit_pat(&pat),
}
self.visit_expr(scrutinee);
self.visit_expr_post(e);
return;
}
visit::walk_expr(self, e)
}

fn visit_item(&mut self, item: &'tcx Item) {
let hir_id = self.lctx.allocate_hir_id_counter(item.id);

Expand Down
51 changes: 19 additions & 32 deletions src/librustc/hir/lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,14 @@ impl LoweringContext<'_> {
// 4. The return type of the block is `bool` which seems like what the user wanted.
let scrutinee = self.lower_expr(scrutinee);
let then_arm = {
let pat = self.lower_pat_top_hack(pat);
let pat = self.lower_pat(pat);
let expr = self.expr_bool(span, true);
self.arm(pat, P(expr))
};
let else_arm = {
let pat = self.pat_wild(span);
let expr = self.expr_bool(span, false);
self.arm(hir_vec![pat], P(expr))
self.arm(pat, P(expr))
};
hir::ExprKind::Match(
P(scrutinee),
Expand All @@ -278,15 +278,15 @@ impl LoweringContext<'_> {
None => (self.expr_block_empty(span), false),
Some(els) => (self.lower_expr(els), true),
};
let else_arm = self.arm(hir_vec![else_pat], P(else_expr));
let else_arm = self.arm(else_pat, P(else_expr));

// Handle then + scrutinee:
let then_expr = self.lower_block_expr(then);
let (then_pat, scrutinee, desugar) = match cond.node {
// `<pat> => <then>`:
ExprKind::Let(ref pat, ref scrutinee) => {
let scrutinee = self.lower_expr(scrutinee);
let pat = self.lower_pat_top_hack(pat);
let pat = self.lower_pat(pat);
(pat, scrutinee, hir::MatchSource::IfLetDesugar { contains_else_clause })
}
// `true => <then>`:
Expand All @@ -303,7 +303,7 @@ impl LoweringContext<'_> {
// let temporaries live outside of `cond`.
let cond = self.expr_drop_temps(span_block, P(cond), ThinVec::new());
let pat = self.pat_bool(span, true);
(hir_vec![pat], cond, hir::MatchSource::IfDesugar { contains_else_clause })
(pat, cond, hir::MatchSource::IfDesugar { contains_else_clause })
}
};
let then_arm = self.arm(then_pat, P(then_expr));
Expand All @@ -327,7 +327,7 @@ impl LoweringContext<'_> {
let else_arm = {
let else_pat = self.pat_wild(span);
let else_expr = self.expr_break(span, ThinVec::new());
self.arm(hir_vec![else_pat], else_expr)
self.arm(else_pat, else_expr)
};

// Handle then + scrutinee:
Expand All @@ -343,7 +343,7 @@ impl LoweringContext<'_> {
// }
// }
let scrutinee = self.with_loop_condition_scope(|t| t.lower_expr(scrutinee));
let pat = self.lower_pat_top_hack(pat);
let pat = self.lower_pat(pat);
(pat, scrutinee, hir::MatchSource::WhileLetDesugar, hir::LoopSource::WhileLet)
}
_ => {
Expand Down Expand Up @@ -371,7 +371,7 @@ impl LoweringContext<'_> {
let cond = self.expr_drop_temps(span_block, P(cond), ThinVec::new());
// `true => <then>`:
let pat = self.pat_bool(span, true);
(hir_vec![pat], cond, hir::MatchSource::WhileDesugar, hir::LoopSource::While)
(pat, cond, hir::MatchSource::WhileDesugar, hir::LoopSource::While)
}
};
let then_arm = self.arm(then_pat, P(then_expr));
Expand Down Expand Up @@ -424,7 +424,7 @@ impl LoweringContext<'_> {
hir::Arm {
hir_id: self.next_id(),
attrs: self.lower_attrs(&arm.attrs),
pats: self.lower_pat_top_hack(&arm.pat),
pat: self.lower_pat(&arm.pat),
guard: match arm.guard {
Some(ref x) => Some(hir::Guard::If(P(self.lower_expr(x)))),
_ => None,
Expand All @@ -434,16 +434,6 @@ impl LoweringContext<'_> {
}
}

/// HACK(or_patterns; Centril | dlrobertson): For now we don't push down top level or-patterns
/// `p | q` into `hir::PatKind::Or(...)` as post-lowering bits of the compiler are not ready
/// to deal with it. This should by fixed by pushing it down to HIR and then HAIR.
fn lower_pat_top_hack(&mut self, pat: &Pat) -> HirVec<P<hir::Pat>> {
match pat.node {
PatKind::Or(ref ps) => ps.iter().map(|x| self.lower_pat(x)).collect(),
_ => hir_vec![self.lower_pat(pat)],
}
}

pub(super) fn make_async_expr(
&mut self,
capture_clause: CaptureBy,
Expand Down Expand Up @@ -592,7 +582,7 @@ impl LoweringContext<'_> {
);
P(this.expr(await_span, expr_break, ThinVec::new()))
});
self.arm(hir_vec![ready_pat], break_x)
self.arm(ready_pat, break_x)
};

// `::std::task::Poll::Pending => {}`
Expand All @@ -603,7 +593,7 @@ impl LoweringContext<'_> {
hir_vec![],
);
let empty_block = P(self.expr_block_empty(span));
self.arm(hir_vec![pending_pat], empty_block)
self.arm(pending_pat, empty_block)
};

let inner_match_stmt = {
Expand Down Expand Up @@ -645,7 +635,7 @@ impl LoweringContext<'_> {
});

// mut pinned => loop { ... }
let pinned_arm = self.arm(hir_vec![pinned_pat], loop_expr);
let pinned_arm = self.arm(pinned_pat, loop_expr);

// match <expr> {
// mut pinned => loop { .. }
Expand Down Expand Up @@ -1079,15 +1069,15 @@ impl LoweringContext<'_> {
ThinVec::new(),
));
let some_pat = self.pat_some(pat.span, val_pat);
self.arm(hir_vec![some_pat], assign)
self.arm(some_pat, assign)
};

// `::std::option::Option::None => break`
let break_arm = {
let break_expr =
self.with_loop_scope(e.id, |this| this.expr_break(e.span, ThinVec::new()));
let pat = self.pat_none(e.span);
self.arm(hir_vec![pat], break_expr)
self.arm(pat, break_expr)
};

// `mut iter`
Expand Down Expand Up @@ -1158,7 +1148,7 @@ impl LoweringContext<'_> {
});

// `mut iter => { ... }`
let iter_arm = self.arm(hir_vec![iter_pat], loop_expr);
let iter_arm = self.arm(iter_pat, loop_expr);

// `match ::std::iter::IntoIterator::into_iter(<head>) { ... }`
let into_iter_expr = {
Expand Down Expand Up @@ -1244,7 +1234,7 @@ impl LoweringContext<'_> {
ThinVec::from(attrs.clone()),
));
let ok_pat = self.pat_ok(span, val_pat);
self.arm(hir_vec![ok_pat], val_expr)
self.arm(ok_pat, val_expr)
};

// `Err(err) => #[allow(unreachable_code)]
Expand Down Expand Up @@ -1279,7 +1269,7 @@ impl LoweringContext<'_> {
};

let err_pat = self.pat_err(try_span, err_local);
self.arm(hir_vec![err_pat], ret_expr)
self.arm(err_pat, ret_expr)
};

hir::ExprKind::Match(
Expand Down Expand Up @@ -1474,14 +1464,11 @@ impl LoweringContext<'_> {
}
}

/// HACK(or_patterns; Centril | dlrobertson): For now we don't push down top level or-patterns
/// `p | q` into `hir::PatKind::Or(...)` as post-lowering bits of the compiler are not ready
/// to deal with it. This should by fixed by pushing it down to HIR and then HAIR.
fn arm(&mut self, pats: HirVec<P<hir::Pat>>, expr: P<hir::Expr>) -> hir::Arm {
fn arm(&mut self, pat: P<hir::Pat>, expr: P<hir::Expr>) -> hir::Arm {
hir::Arm {
hir_id: self.next_id(),
attrs: hir_vec![],
pats,
pat,
guard: None,
span: expr.span,
body: expr,
Expand Down
86 changes: 57 additions & 29 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,44 +882,61 @@ impl fmt::Debug for Pat {

impl Pat {
// FIXME(#19596) this is a workaround, but there should be a better way
fn walk_<G>(&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;
}

match self.node {
PatKind::Binding(.., Some(ref p)) => p.walk_(it),
PatKind::Struct(_, ref fields, _) => {
fields.iter().all(|field| field.pat.walk_(it))
}
PatKind::TupleStruct(_, ref s, _) | PatKind::Tuple(ref s, _) => {
s.iter().all(|p| p.walk_(it))
}
PatKind::Or(ref pats) => pats.iter().all(|p| p.walk_(it)),
PatKind::Box(ref s) | PatKind::Ref(ref s, _) => {
s.walk_(it)
}
PatKind::Slice(ref before, ref slice, ref after) => {
use PatKind::*;
match &self.node {
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<F>(&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)
}
}
Expand Down Expand Up @@ -1259,21 +1276,32 @@ pub struct Local {
}

/// Represents a single arm of a `match` expression, e.g.
/// `<pats> (if <guard>) => <body>`.
/// `<pat> (if <guard>) => <body>`.
#[derive(RustcEncodable, RustcDecodable, Debug, HashStable)]
pub struct Arm {
#[stable_hasher(ignore)]
pub hir_id: HirId,
pub span: Span,
pub attrs: HirVec<Attribute>,
/// Multiple patterns can be combined with `|`
pub pats: HirVec<P<Pat>>,
/// If this pattern and the optional guard matches, then `body` is evaluated.
pub pat: P<Pat>,
/// Optional guard clause.
pub guard: Option<Guard>,
/// The expression the arm evaluates to if this arm matches.
pub body: P<Expr>,
}

impl Arm {
// HACK(or_patterns; Centril | dlrobertson): Remove this and
// correctly handle each case in which this method is used.
pub fn top_pats_hack(&self) -> &[P<Pat>] {
match &self.pat.node {
PatKind::Or(pats) => pats,
_ => std::slice::from_ref(&self.pat),
}
}
}

#[derive(RustcEncodable, RustcDecodable, Debug, HashStable)]
pub enum Guard {
If(P<Expr>),
Expand Down
Loading

0 comments on commit da78127

Please sign in to comment.