Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

or-patterns: Uniformly use PatKind::Or in AST & Fix/Cleanup resolve #64111

Merged
merged 27 commits into from
Sep 6, 2019
Merged
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
af06bfb
resolve: extract `resolve_params`.
Centril Aug 28, 2019
f8835ee
resolve: cleanup using `with_rib`, etc.
Centril Aug 28, 2019
6d537d4
resolve: simplify `resolve_arm`.
Centril Aug 28, 2019
219ddde
resolve: extract `try_resolve_as_non_binding`.
Centril Aug 28, 2019
f343e84
resolve: add `type IdentMap<T>` alias.
Centril Aug 28, 2019
fd3b441
resolve: move `fresh_binding`.
Centril Aug 28, 2019
dc91e22
resolve: move `resolve_block` to a better place.
Centril Aug 28, 2019
70cae78
resolve: already-bound-check: account for or-patterns.
Centril Aug 28, 2019
166a558
resolve: revamp already-bound check -- fix some bugs.
Centril Sep 1, 2019
498ec59
resolve: add tests for already-bound check.
Centril Sep 1, 2019
896a1c7
resolve: account for general or-patterns in consistency checking.
Centril Sep 1, 2019
33317c7
resolve: add test for missing bindings in or-patterns.
Centril Sep 1, 2019
aa7a02b
resolve: test binding mode consistency for or-patterns.
Centril Sep 1, 2019
dbe6873
resolve: test consistent or-patterns being allowed.
Centril Sep 3, 2019
ca968a1
or-patterns: syntax: simplify `Arm.pats` and `ExprKind::Let.0`.
Centril Aug 27, 2019
998060b
or-patterns: syntax: adjust `visit` and `mut_visit`.
Centril Aug 27, 2019
ad3db72
or-patterns: syntax: adjust parser removing a hack.
Centril Aug 27, 2019
424492a
or-patterns: syntax: adjust pretty printing.
Centril Aug 27, 2019
76625eb
or-patterns: syntax: adjust derive, format, and building.
Centril Aug 27, 2019
3fccbac
or-patterns: adjust librustc_lint.
Centril Aug 27, 2019
d8ef907
or-patterns: adjust lowering of `ast::Arm` & `ast::ExprKind::Let`.
Centril Aug 28, 2019
a867c5f
resolve: merge `resolve_pats` and `resolve_pattern_top`.
Centril Sep 1, 2019
be95dee
or-patterns: adjust save_analysis wrt. `process_var_decl{_multi}`.
Centril Sep 2, 2019
d70b0c5
or-patterns: fix pprust-expr-roundtrip due to AST change.
Centril Sep 3, 2019
a7db1a4
or-patterns: address review comments.
Centril Sep 4, 2019
0341b78
resolve: bool -> enum PatBoundCtx
Centril Sep 4, 2019
16ba502
or-patterns: fix fallout from #664128.
Centril Sep 5, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
resolve: bool -> enum PatBoundCtx
  • Loading branch information
Centril committed Sep 5, 2019
commit 0341b78044a51b4883477234d2133c904b69ae68
47 changes: 29 additions & 18 deletions src/librustc_resolve/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ impl PatternSource {
}
}

/// Denotes whether the context for the set of already bound bindings is a `Product`
/// or `Or` context. This is used in e.g., `fresh_binding` and `resolve_pattern_inner`.
/// See those functions for more information.
enum PatBoundCtx {
/// A product pattern context, e.g., `Variant(a, b)`.
Product,
/// An or-pattern context, e.g., `p_0 | ... | p_n`.
Or,
}

/// The rib kind restricts certain accesses,
/// e.g. to a `Res::Local` of an outer item.
#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -1109,7 +1119,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
}

fn resolve_params(&mut self, params: &[Param]) {
let mut bindings = smallvec![(false, Default::default())];
let mut bindings = smallvec![(PatBoundCtx::Product, 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,14 +1265,15 @@ 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::default())]);
let mut bindings = smallvec![(PatBoundCtx::Product, Default::default())];
self.resolve_pattern(pat, pat_src, &mut bindings);
}

fn resolve_pattern(
&mut self,
pat: &Pat,
pat_src: PatternSource,
bindings: &mut SmallVec<[(bool, FxHashSet<Ident>); 1]>,
bindings: &mut SmallVec<[(PatBoundCtx, FxHashSet<Ident>); 1]>,
) {
self.resolve_pattern_inner(pat, pat_src, bindings);
// This has to happen *after* we determine which pat_idents are variants:
Expand All @@ -1276,15 +1287,15 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
///
/// 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.
/// In each set, `PatBoundCtx::Product` denotes that a found binding in it should
/// be interpreted as re-binding an already bound binding. This results in an error.
/// Meanwhile, `PatBound::Or` denotes that a found binding in the set should result
/// in reusing this binding rather than creating a fresh one.
///
/// 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 called at the top level, the stack must have a single element
/// with `PatBound::Product`. Otherwise, pushing to the stack happens as
/// or-patterns (`p_0 | ... | p_n`) are encountered and the context needs
/// to be switched to `PatBoundCtx::Or` and then `PatBoundCtx::Product` 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.
///
Expand All @@ -1293,7 +1304,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
&mut self,
pat: &Pat,
pat_src: PatternSource,
bindings: &mut SmallVec<[(bool, FxHashSet<Ident>); 1]>,
bindings: &mut SmallVec<[(PatBoundCtx, FxHashSet<Ident>); 1]>,
) {
// Visit all direct subpatterns of this pattern.
pat.walk(&mut |pat| {
Expand All @@ -1317,15 +1328,15 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
self.smart_resolve_path(pat.id, None, path, PathSource::Struct);
}
PatKind::Or(ref ps) => {
// Add a new set of bindings to the stack. `true` here records that when a
// Add a new set of bindings to the stack. `Or` 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::default()));
bindings.push((PatBoundCtx::Or, 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::default()));
bindings.push((PatBoundCtx::Product, 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 All @@ -1352,7 +1363,7 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
ident: Ident,
pat_id: NodeId,
pat_src: PatternSource,
bindings: &mut SmallVec<[(bool, FxHashSet<Ident>); 1]>,
bindings: &mut SmallVec<[(PatBoundCtx, FxHashSet<Ident>); 1]>,
) -> Res {
// Add the binding to the local ribs, if it doesn't already exist in the bindings map.
// (We must not add it if it's in the bindings map because that breaks the assumptions
Expand All @@ -1366,10 +1377,10 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
for (is_sum, set) in bindings.iter_mut().rev() {
match (is_sum, set.get(&ident).cloned()) {
// Already bound in a product pattern, e.g. `(a, a)` which is not allowed.
(false, Some(..)) => already_bound_and = true,
(PatBoundCtx::Product, Some(..)) => already_bound_and = true,
// Already bound in an or-pattern, e.g. `V1(a) | V2(a)`.
// This is *required* for consistency which is checked later.
(true, Some(..)) => already_bound_or = true,
(PatBoundCtx::Or, Some(..)) => already_bound_or = true,
// Not already bound here.
_ => {}
}
Expand Down