From 6ca07235a600bfc20b5fda2d5d6e985ca81d124f Mon Sep 17 00:00:00 2001
From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com>
Date: Fri, 4 Aug 2023 11:30:09 +0200
Subject: [PATCH] Replace `HashMap` with `IndexMap` in pattern binding resolve

It will be iterated over, so we should avoid using `HashMap`.
---
 compiler/rustc_resolve/src/late.rs            | 38 ++++-----
 tests/ui/or-patterns/missing-bindings.stderr  | 84 +++++++++----------
 .../resolve/resolve-inconsistent-names.stderr | 42 +++++-----
 tests/ui/span/issue-39698.stderr              | 34 ++++----
 4 files changed, 98 insertions(+), 100 deletions(-)

diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
index 01f9f06059426..5df6e405e4eb7 100644
--- a/compiler/rustc_resolve/src/late.rs
+++ b/compiler/rustc_resolve/src/late.rs
@@ -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,
 };
@@ -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 {
@@ -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 {
@@ -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;
             }
@@ -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.
diff --git a/tests/ui/or-patterns/missing-bindings.stderr b/tests/ui/or-patterns/missing-bindings.stderr
index 4457b7893d5d2..7f182a857871d 100644
--- a/tests/ui/or-patterns/missing-bindings.stderr
+++ b/tests/ui/or-patterns/missing-bindings.stderr
@@ -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
    |
@@ -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
    |
@@ -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
    |
@@ -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
    |
@@ -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
    |
@@ -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`.
diff --git a/tests/ui/resolve/resolve-inconsistent-names.stderr b/tests/ui/resolve/resolve-inconsistent-names.stderr
index 42b7281d7b060..d6240fb8f872c 100644
--- a/tests/ui/resolve/resolve-inconsistent-names.stderr
+++ b/tests/ui/resolve/resolve-inconsistent-names.stderr
@@ -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
    |
@@ -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
    |
@@ -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
    |
@@ -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
    |
diff --git a/tests/ui/span/issue-39698.stderr b/tests/ui/span/issue-39698.stderr
index 81211b20a0135..500080832111b 100644
--- a/tests/ui/span/issue-39698.stderr
+++ b/tests/ui/span/issue-39698.stderr
@@ -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
    |
@@ -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
    |
@@ -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