Skip to content

Commit

Permalink
Replace HashMap with IndexMap in pattern binding resolve
Browse files Browse the repository at this point in the history 
It will be iterated over, so we should avoid using `HashMap`.
  • Loading branch information
@Nilstrieb
Nilstrieb committed Sep 25, 2023
1 parent 551c718 commit b6d602e
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 102 deletions.
38 changes: 18 additions & 20 deletions compiler/rustc_resolve/src/late.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,6 @@ type Res = def::Res<NodeId>;

type IdentMap<T> = FxHashMap<Ident, T>;

/// Map from the name in a pattern to its binding mode.
type BindingMap = IdentMap<BindingInfo>;

use diagnostics::{
ElisionFnParameter, LifetimeElisionCandidate, MissingLifetime, MissingLifetimeKind,
};
Expand Down Expand Up @@ -3164,8 +3161,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
/// this is done hygienically. This could arise for a macro
/// that expands into an or-pattern where one 'x' was from the
/// user and one 'x' came from the macro.
fn binding_mode_map(&mut self, pat: &Pat) -> BindingMap {
let mut binding_map = FxHashMap::default();
fn binding_mode_map(&mut self, pat: &Pat) -> FxIndexMap<Ident, BindingInfo> {
let mut binding_map = FxIndexMap::default();

pat.walk(&mut |pat| {
match pat.kind {
Expand Down Expand Up @@ -3200,22 +3197,28 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {

/// Checks that all of the arms in an or-pattern have exactly the
/// same set of bindings, with the same binding modes for each.
fn check_consistent_bindings(&mut self, pats: &[P<Pat>]) -> Vec<BindingMap> {
let mut missing_vars = FxHashMap::default();
let mut inconsistent_vars = FxHashMap::default();
fn check_consistent_bindings(
&mut self,
pats: &[P<Pat>],
) -> Vec<FxIndexMap<Ident, BindingInfo>> {
// pats is consistent.
let mut missing_vars = FxIndexMap::default();
let mut inconsistent_vars = FxIndexMap::default();

// 1) Compute the binding maps of all arms.
let maps = pats.iter().map(|pat| self.binding_mode_map(pat)).collect::<Vec<_>>();

// 2) Record any missing bindings or binding mode inconsistencies.
for (map_outer, pat_outer) in pats.iter().enumerate().map(|(idx, pat)| (&maps[idx], pat)) {
for (map_outer, pat_outer) in maps.iter().zip(pats.iter()) {
// Check against all arms except for the same pattern which is always self-consistent.
let inners = pats
let inners = maps
.iter()
.enumerate()
.zip(pats.iter())
.filter(|(_, pat)| pat.id != pat_outer.id)
.flat_map(|(idx, _)| maps[idx].iter())
.map(|(key, binding)| (key.name, map_outer.get(&key), binding));
.flat_map(|(map, _)| map)
.map(|(key, binding)| (key.name, map_outer.get(key), binding));

let inners = inners.collect::<Vec<_>>();

for (name, info, &binding_inner) in inners {
match info {
Expand Down Expand Up @@ -3244,10 +3247,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
}

// 3) Report all missing variables we found.
let mut missing_vars = missing_vars.into_iter().collect::<Vec<_>>();
missing_vars.sort_by_key(|&(sym, ref _err)| sym);

for (name, mut v) in missing_vars.into_iter() {
for (name, mut v) in missing_vars {
if inconsistent_vars.contains_key(&name) {
v.could_be_path = false;
}
Expand All @@ -3258,10 +3258,8 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
}

// 4) Report all inconsistencies in binding modes we found.
let mut inconsistent_vars = inconsistent_vars.iter().collect::<Vec<_>>();
inconsistent_vars.sort();
for (name, v) in inconsistent_vars {
self.report_error(v.0, ResolutionError::VariableBoundWithDifferentMode(*name, v.1));
self.report_error(v.0, ResolutionError::VariableBoundWithDifferentMode(name, v.1));
}

// 5) Finally bubble up all the binding maps.
Expand Down
88 changes: 44 additions & 44 deletions tests/ui/or-patterns/missing-bindings.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,14 @@ LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| |
| pattern doesn't bind `c`

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/missing-bindings.rs:45:22
|
LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| - ^^^^ pattern doesn't bind `b`
| |
| variable not in all patterns

error[E0408]: variable `a` is not bound in all patterns
--> $DIR/missing-bindings.rs:45:22
|
Expand All @@ -95,11 +103,19 @@ LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| |
| variable not in all patterns

error[E0408]: variable `e` is not bound in all patterns
--> $DIR/missing-bindings.rs:45:10
|
LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| ^^^^^^^^^^^^^^^^^^^^ - variable not in all patterns
| |
| pattern doesn't bind `e`

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/missing-bindings.rs:45:22
--> $DIR/missing-bindings.rs:45:33
|
LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| - ^^^^ pattern doesn't bind `b`
| - ^^^^ pattern doesn't bind `b`
| |
| variable not in all patterns

Expand All @@ -119,14 +135,6 @@ LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| |
| variable not in all patterns

error[E0408]: variable `e` is not bound in all patterns
--> $DIR/missing-bindings.rs:45:10
|
LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| ^^^^^^^^^^^^^^^^^^^^ - variable not in all patterns
| |
| pattern doesn't bind `e`

error[E0408]: variable `a` is not bound in all patterns
--> $DIR/missing-bindings.rs:45:33
|
Expand All @@ -135,14 +143,6 @@ LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| |
| variable not in all patterns

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/missing-bindings.rs:45:33
|
LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| - ^^^^ pattern doesn't bind `b`
| |
| variable not in all patterns

error[E0408]: variable `a` is not bound in all patterns
--> $DIR/missing-bindings.rs:61:29
|
Expand All @@ -151,6 +151,14 @@ LL | Ok(a) | Err(_),
| |
| variable not in all patterns

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/missing-bindings.rs:68:21
|
LL | A(_, a) |
| ^^^^^^^ pattern doesn't bind `b`
LL | B(b),
| - variable not in all patterns

error[E0408]: variable `a` is not bound in all patterns
--> $DIR/missing-bindings.rs:69:21
|
Expand All @@ -160,12 +168,13 @@ LL | B(b),
| ^^^^ pattern doesn't bind `a`

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/missing-bindings.rs:68:21
--> $DIR/missing-bindings.rs:72:17
|
LL | A(_, a) |
| ^^^^^^^ pattern doesn't bind `b`
LL | B(b),
| - variable not in all patterns
...
LL | B(_)
| ^^^^ pattern doesn't bind `b`

error[E0408]: variable `a` is not bound in all patterns
--> $DIR/missing-bindings.rs:72:17
Expand All @@ -177,13 +186,22 @@ LL | B(_)
| ^^^^ pattern doesn't bind `a`

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/missing-bindings.rs:72:17
--> $DIR/missing-bindings.rs:57:13
|
LL | B(b),
| - variable not in all patterns
LL | / V1(
LL | |
LL | |
LL | | A(
... |
LL | | B(Ok(a) | Err(a))
LL | | ) |
| |_____________^ pattern doesn't bind `b`
...
LL | B(_)
| ^^^^ pattern doesn't bind `b`
LL | B(b),
| - variable not in all patterns
...
LL | V3(c),
| ^^^^^ pattern doesn't bind `b`

error[E0408]: variable `c` is not bound in all patterns
--> $DIR/missing-bindings.rs:57:13
Expand Down Expand Up @@ -219,24 +237,6 @@ LL | A(_, a) |
LL | V3(c),
| ^^^^^ pattern doesn't bind `a`

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/missing-bindings.rs:57:13
|
LL | / V1(
LL | |
LL | |
LL | | A(
... |
LL | | B(Ok(a) | Err(a))
LL | | ) |
| |_____________^ pattern doesn't bind `b`
...
LL | B(b),
| - variable not in all patterns
...
LL | V3(c),
| ^^^^^ pattern doesn't bind `b`

error: aborting due to 26 previous errors

For more information about this error, try `rustc --explain E0408`.
42 changes: 21 additions & 21 deletions tests/ui/resolve/resolve-inconsistent-names.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
error[E0408]: variable `a` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:13:12
|
LL | a | b => {}
| - ^ pattern doesn't bind `a`
| |
| variable not in all patterns

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:13:8
|
Expand All @@ -14,6 +6,14 @@ LL | a | b => {}
| |
| pattern doesn't bind `b`

error[E0408]: variable `a` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:13:12
|
LL | a | b => {}
| - ^ pattern doesn't bind `a`
| |
| variable not in all patterns

error[E0408]: variable `c` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:19:9
|
Expand Down Expand Up @@ -54,6 +54,19 @@ LL | (A, B) | (ref B, c) | (c, A) => ()
| |
| first binding

error[E0408]: variable `Const2` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:31:9
|
LL | (CONST1, _) | (_, Const2) => ()
| ^^^^^^^^^^^ ------ variable not in all patterns
| |
| pattern doesn't bind `Const2`
|
help: if you meant to match on constant `m::Const2`, use the full path in the pattern
|
LL | (CONST1, _) | (_, m::Const2) => ()
| ~~~~~~~~~

error[E0408]: variable `CONST1` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:31:23
|
Expand All @@ -68,19 +81,6 @@ note: you might have meant to match on constant `m::CONST1`, which exists but is
LL | const CONST1: usize = 10;
| ^^^^^^^^^^^^^^^^^^^^^^^^^ not accessible

error[E0408]: variable `Const2` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:31:9
|
LL | (CONST1, _) | (_, Const2) => ()
| ^^^^^^^^^^^ ------ variable not in all patterns
| |
| pattern doesn't bind `Const2`
|
help: if you meant to match on constant `m::Const2`, use the full path in the pattern
|
LL | (CONST1, _) | (_, m::Const2) => ()
| ~~~~~~~~~

error[E0308]: mismatched types
--> $DIR/resolve-inconsistent-names.rs:19:19
|
Expand Down
34 changes: 17 additions & 17 deletions tests/ui/span/issue-39698.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
error[E0408]: variable `b` is not bound in all patterns
--> $DIR/issue-39698.rs:10:9
|
LL | T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
| ^^^^^^^^^^^ - ^^^^^^^^ ^^^^^^^^ pattern doesn't bind `b`
| | | |
| | | pattern doesn't bind `b`
| | variable not in all patterns
| pattern doesn't bind `b`

error[E0408]: variable `c` is not bound in all patterns
--> $DIR/issue-39698.rs:10:9
|
Expand All @@ -8,16 +18,6 @@ LL | T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}
| | pattern doesn't bind `c`
| pattern doesn't bind `c`

error[E0408]: variable `d` is not bound in all patterns
--> $DIR/issue-39698.rs:10:37
|
LL | T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
| - - ^^^^^^^^ ^^^^^^^^ pattern doesn't bind `d`
| | | |
| | | pattern doesn't bind `d`
| | variable not in all patterns
| variable not in all patterns

error[E0408]: variable `a` is not bound in all patterns
--> $DIR/issue-39698.rs:10:23
|
Expand All @@ -28,15 +28,15 @@ LL | T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}
| | pattern doesn't bind `a`
| variable not in all patterns

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/issue-39698.rs:10:9
error[E0408]: variable `d` is not bound in all patterns
--> $DIR/issue-39698.rs:10:37
|
LL | T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
| ^^^^^^^^^^^ - ^^^^^^^^ ^^^^^^^^ pattern doesn't bind `b`
| | | |
| | | pattern doesn't bind `b`
| | variable not in all patterns
| pattern doesn't bind `b`
| - - ^^^^^^^^ ^^^^^^^^ pattern doesn't bind `d`
| | | |
| | | pattern doesn't bind `d`
| | variable not in all patterns
| variable not in all patterns

error: aborting due to 4 previous errors

Expand Down

0 comments on commit b6d602e

Please sign in to comment.