Skip to content

Commit

Permalink
or-patterns: address review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
Centril committed Sep 5, 2019
1 parent d70b0c5 commit a7db1a4
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 9 deletions.
27 changes: 23 additions & 4 deletions src/librustc_resolve/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}

fn resolve_params(&mut self, params: &[Param]) {
let mut bindings = smallvec![(false, <_>::default())];
let mut bindings = smallvec![(false, Default::default())];
for Param { pat, ty, .. } in params {
self.resolve_pattern(pat, PatternSource::FnParam, &mut bindings);
self.visit_ty(ty);
Expand Down Expand Up @@ -1255,7 +1255,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {

/// Arising from `source`, resolve a top level pattern.
fn resolve_pattern_top(&mut self, pat: &Pat, pat_src: PatternSource) {
self.resolve_pattern(pat, pat_src, &mut smallvec![(false, <_>::default())]);
self.resolve_pattern(pat, pat_src, &mut smallvec![(false, Default::default())]);
}

fn resolve_pattern(
Expand All @@ -1270,6 +1270,25 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
visit::walk_pat(self, pat);
}

/// Resolve bindings in a pattern. This is a helper to `resolve_pattern`.
///
/// ### `bindings`
///
/// A stack of sets of bindings accumulated.
///
/// In each set, `false` denotes that a found binding in it should be interpreted as
/// re-binding an already bound binding. This results in an error. Meanwhile, `true`
/// denotes that a found binding in the set should result in reusing this binding
/// rather than creating a fresh one. In other words, `false` and `true` correspond
/// to product (e.g., `(a, b)`) and sum/or contexts (e.g., `p_0 | ... | p_i`) respectively.
///
/// When called at the top level, the stack should have a single element with `false`.
/// Otherwise, pushing to the stack happens as or-patterns are encountered and the
/// context needs to be switched to `true` and then `false` for each `p_i.
/// When each `p_i` has been dealt with, the top set is merged with its parent.
/// When a whole or-pattern has been dealt with, the thing happens.
///
/// See the implementation and `fresh_binding` for more details.
fn resolve_pattern_inner(
&mut self,
pat: &Pat,
Expand Down Expand Up @@ -1301,12 +1320,12 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
// Add a new set of bindings to the stack. `true` here records that when a
// binding already exists in this set, it should not result in an error because
// `V1(a) | V2(a)` must be allowed and are checked for consistency later.
bindings.push((true, <_>::default()));
bindings.push((true, Default::default()));
for p in ps {
// Now we need to switch back to a product context so that each
// part of the or-pattern internally rejects already bound names.
// For example, `V1(a) | V2(a, a)` and `V1(a, a) | V2(a)` are bad.
bindings.push((false, <_>::default()));
bindings.push((false, Default::default()));
self.resolve_pattern_inner(p, pat_src, bindings);
// Move up the non-overlapping bindings to the or-pattern.
// Existing bindings just get "merged".
Expand Down
5 changes: 1 addition & 4 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ impl Pat {

/// Walk top-down and call `it` in each place where a pattern occurs
/// starting with the root pattern `walk` is called on. If `it` returns
/// false then we will decend no further but siblings will be processed.
/// false then we will descend no further but siblings will be processed.
pub fn walk(&self, it: &mut impl FnMut(&Pat) -> bool) {
if !it(self) {
return;
Expand Down Expand Up @@ -1150,9 +1150,6 @@ pub enum ExprKind {
Type(P<Expr>, P<Ty>),
/// A `let pat = expr` expression that is only semantically allowed in the condition
/// of `if` / `while` expressions. (e.g., `if let 0 = x { .. }`).
///
/// The `Vec<P<Pat>>` is for or-patterns at the top level.
/// FIXME(54883): Change this to just `P<Pat>`.
Let(P<Pat>, P<Expr>),
/// An `if` block, with an optional `else` block.
///
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,7 @@ pub fn walk_param<'a, V: Visitor<'a>>(visitor: &mut V, param: &'a Param) {

pub fn walk_arm<'a, V: Visitor<'a>>(visitor: &mut V, arm: &'a Arm) {
visitor.visit_pat(&arm.pat);
// HACK(or_patterns; Centril | dlrobertson):
// NOTE(or_patterns; Centril | dlrobertson):
// If you change this, also change the hack in `lowering.rs`.
walk_list!(visitor, visit_expr, &arm.guard);
visitor.visit_expr(&arm.body);
Expand Down

0 comments on commit a7db1a4

Please sign in to comment.