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.

Sign up for GitHub

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

Jump to bottom

Replace HashMap with IndexMap in pattern binding resolve #114454

Merged
merged 1 commit into from
Oct 2, 2023
Merged

Replace HashMap with IndexMap in pattern binding resolve #114454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 18 additions & 20 deletions compiler/rustc_resolve/src/late.rs
View file
Open in desktop
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
84 changes: 42 additions & 42 deletions tests/ui/or-patterns/missing-bindings.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,22 +103,6 @@ LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
| |
| variable not in all patterns

error[E0408]: variable `c` 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 `c`
| |
| variable not in all patterns

error[E0408]: variable `d` 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 `d`
| |
| variable not in all patterns

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

error[E0408]: variable `c` 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 `c`
| |
| variable not in all patterns

error[E0408]: variable `d` 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 `d`
| |
| variable not in all patterns

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

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

error[E0408]: variable `b` is not bound in all patterns
--> $DIR/missing-bindings.rs:68:21
|
Expand All @@ -167,6 +159,14 @@ LL | A(_, a) |
LL | B(b),
| - variable not in all patterns

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

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

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[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
Open in desktop
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
Open in desktop
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
Loading