Rollup merge of rust-lang#114454 - Nilstrieb:no-evil-sorting, r=cjgillot

Replace `HashMap` with `IndexMap` in pattern binding resolve fixes rust-lang#114332 (comment)
GuillaumeGomez · Sep 24, 2023 · a87b9da · a87b9da
2 parents 44bd31c + a654b4e
commit a87b9da
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 91 deletions.
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
@@ -3201,8 +3201,8 @@ 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();
+        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<_>>();
@@ -3244,10 +3244,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 +3255,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
@@ -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
    |
@@ -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
 
@@ -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
    |
@@ -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
    |
@@ -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
    |
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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