Auto merge of #39713 - estebank:issue-39698, r=jonathandturner

Clean up "pattern doesn't bind x" messages

Group "missing variable bind" spans in `or` matches and clarify wording
for the two possible cases: when a variable from the first pattern is
not in any of the subsequent patterns, and when a variable in any of the
other patterns is not in the first one.

Before:

```rust
error[E0408]: variable `a` from pattern #1 is not bound in pattern #2
  --> file.rs:10:23
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                       ^^^^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `b` from pattern #2 is not bound in pattern #1
  --> file.rs:10:32
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                ^ pattern doesn't bind `b`

error[E0408]: variable `a` from pattern #1 is not bound in pattern #3
  --> file.rs:10:37
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                     ^^^^^^^^ pattern doesn't bind `a`

error[E0408]: variable `d` from pattern #1 is not bound in pattern #3
  --> file.rs:10:37
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                     ^^^^^^^^ pattern doesn't bind `d`

error[E0408]: variable `c` from pattern #3 is not bound in pattern #1
  --> file.rs:10:43
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                           ^ pattern doesn't bind `c`

error[E0408]: variable `d` from pattern #1 is not bound in pattern #4
  --> file.rs:10:48
   |
10 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
   |                                                ^^^^^^^^ pattern doesn't bind `d`

error: aborting due to 6 previous errors
```

After:

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

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

error[E0408]: variable `b` is not bound in all patterns
  --> $DIR/issue-39698.rs:20:37
   |
20 |         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: aborting due to 4 previous errors
```

Fixes #39698.

src/librustc_resolve/lib.rs

@@ -69,6 +69,7 @@ use errors::DiagnosticBuilder;
 
 use std::cell::{Cell, RefCell};
 use std::cmp;
+use std::collections::BTreeSet;
 use std::fmt;
 use std::mem::replace;
 use std::rc::Rc;
@@ -97,6 +98,31 @@ enum AssocSuggestion {
     AssocItem,
 }
 
+#[derive(Eq)]
+struct BindingError {
+    name: Name,
+    origin: BTreeSet<Span>,
+    target: BTreeSet<Span>,
+}
+
+impl PartialOrd for BindingError {
+    fn partial_cmp(&self, other: &BindingError) -> Option<cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl PartialEq for BindingError {
+    fn eq(&self, other: &BindingError) -> bool {
+        self.name == other.name
+    }
+}
+
+impl Ord for BindingError {
+    fn cmp(&self, other: &BindingError) -> cmp::Ordering {
+        self.name.cmp(&other.name)
+    }
+}
+
 enum ResolutionError<'a> {
     /// error E0401: can't use type parameters from outer function
     TypeParametersFromOuterFunction,
@@ -110,10 +136,10 @@ enum ResolutionError<'a> {
     TypeNotMemberOfTrait(Name, &'a str),
     /// error E0438: const is not a member of trait
     ConstNotMemberOfTrait(Name, &'a str),
-    /// error E0408: variable `{}` from pattern #{} is not bound in pattern #{}
-    VariableNotBoundInPattern(Name, usize, usize),
-    /// error E0409: variable is bound with different mode in pattern #{} than in pattern #1
-    VariableBoundWithDifferentMode(Name, usize, Span),
+    /// error E0408: variable `{}` is not bound in all patterns
+    VariableNotBoundInPattern(&'a BindingError),
+    /// error E0409: variable `{}` is bound in inconsistent ways within the same match arm
+    VariableBoundWithDifferentMode(Name, Span),
     /// error E0415: identifier is bound more than once in this parameter list
     IdentifierBoundMoreThanOnceInParameterList(&'a str),
     /// error E0416: identifier is bound more than once in the same pattern
@@ -207,27 +233,28 @@ fn resolve_struct_error<'sess, 'a>(resolver: &'sess Resolver,
             err.span_label(span, &format!("not a member of trait `{}`", trait_));
             err
         }
-        ResolutionError::VariableNotBoundInPattern(variable_name, from, to) => {
-            let mut err = struct_span_err!(resolver.session,
-                             span,
-                             E0408,
-                             "variable `{}` from pattern #{} is not bound in pattern #{}",
-                             variable_name,
-                             from,
-                             to);
-            err.span_label(span, &format!("pattern doesn't bind `{}`", variable_name));
+        ResolutionError::VariableNotBoundInPattern(binding_error) => {
+            let target_sp = binding_error.target.iter().map(|x| *x).collect::<Vec<_>>();
+            let msp = MultiSpan::from_spans(target_sp.clone());
+            let msg = format!("variable `{}` is not bound in all patterns", binding_error.name);
+            let mut err = resolver.session.struct_span_err_with_code(msp, &msg, "E0408");
+            for sp in target_sp {
+                err.span_label(sp, &format!("pattern doesn't bind `{}`", binding_error.name));
+            }
+            let origin_sp = binding_error.origin.iter().map(|x| *x).collect::<Vec<_>>();
+            for sp in origin_sp {
+                err.span_label(sp, &"variable not in all patterns");
+            }
             err
         }
         ResolutionError::VariableBoundWithDifferentMode(variable_name,
-                                                        pattern_number,
                                                         first_binding_span) => {
             let mut err = struct_span_err!(resolver.session,
                              span,
                              E0409,
-                             "variable `{}` is bound with different mode in pattern #{} than in \
-                              pattern #1",
-                             variable_name,
-                             pattern_number);
+                             "variable `{}` is bound in inconsistent \
+                             ways within the same match arm",
+                             variable_name);
             err.span_label(span, &format!("bound in different ways"));
             err.span_label(first_binding_span, &format!("first binding"));
             err
@@ -335,7 +362,7 @@ fn resolve_struct_error<'sess, 'a>(resolver: &'sess Resolver,
     }
 }
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 struct BindingInfo {
     span: Span,
     binding_mode: BindingMode,
@@ -1904,36 +1931,66 @@ impl<'a> Resolver<'a> {
         if arm.pats.is_empty() {
             return;
         }
-        let map_0 = self.binding_mode_map(&arm.pats[0]);
+
+        let mut missing_vars = FxHashMap();
+        let mut inconsistent_vars = FxHashMap();
         for (i, p) in arm.pats.iter().enumerate() {
             let map_i = self.binding_mode_map(&p);
 
-            for (&key, &binding_0) in &map_0 {
-                match map_i.get(&key) {
-                    None => {
-                        let error = ResolutionError::VariableNotBoundInPattern(key.name, 1, i + 1);
-                        resolve_error(self, p.span, error);
+            for (j, q) in arm.pats.iter().enumerate() {
+                if i == j {
+                    continue;
+                }
+
+                let map_j = self.binding_mode_map(&q);
+                for (&key, &binding_i) in &map_i {
+                    if map_j.len() == 0 {                   // Account for missing bindings when
+                        let binding_error = missing_vars    // map_j has none.
+                            .entry(key.name)
+                            .or_insert(BindingError {
+                                name: key.name,
+                                origin: BTreeSet::new(),
+                                target: BTreeSet::new(),
+                            });
+                        binding_error.origin.insert(binding_i.span);
+                        binding_error.target.insert(q.span);
                     }
-                    Some(binding_i) => {
-                        if binding_0.binding_mode != binding_i.binding_mode {
-                            resolve_error(self,
-                                          binding_i.span,
-                                          ResolutionError::VariableBoundWithDifferentMode(
-                                              key.name,
-                                              i + 1,
-                                              binding_0.span));
+                    for (&key_j, &binding_j) in &map_j {
+                        match map_i.get(&key_j) {
+                            None => {  // missing binding
+                                let binding_error = missing_vars
+                                    .entry(key_j.name)
+                                    .or_insert(BindingError {
+                                        name: key_j.name,
+                                        origin: BTreeSet::new(),
+                                        target: BTreeSet::new(),
+                                    });
+                                binding_error.origin.insert(binding_j.span);
+                                binding_error.target.insert(p.span);
+                            }
+                            Some(binding_i) => {  // check consistent binding
+                                if binding_i.binding_mode != binding_j.binding_mode {
+                                    inconsistent_vars
+                                        .entry(key.name)
+                                        .or_insert((binding_j.span, binding_i.span));
+                                }
+                            }
                         }
                     }
                 }
             }
-
-            for (&key, &binding) in &map_i {
-                if !map_0.contains_key(&key) {
-                    resolve_error(self,
-                                  binding.span,
-                                  ResolutionError::VariableNotBoundInPattern(key.name, i + 1, 1));
-                }
-            }
+        }
+        let mut missing_vars = missing_vars.iter().collect::<Vec<_>>();
+        missing_vars.sort();
+        for (_, v) in missing_vars {
+            resolve_error(self,
+                          *v.origin.iter().next().unwrap(),
+                          ResolutionError::VariableNotBoundInPattern(v));
+        }
+        let mut inconsistent_vars = inconsistent_vars.iter().collect::<Vec<_>>();
+        inconsistent_vars.sort();
+        for (name, v) in inconsistent_vars {
+            resolve_error(self, v.0, ResolutionError::VariableBoundWithDifferentMode(*name, v.1));
         }
     }
 

src/test/compile-fail/E0408.rs

@@ -12,7 +12,8 @@ fn main() {
     let x = Some(0);
 
     match x {
-        Some(y) | None => {} //~  ERROR variable `y` from pattern #1 is not bound in pattern #2
+        Some(y) | None => {} //~  ERROR variable `y` is not bound in all patterns
         _ => ()              //~| NOTE pattern doesn't bind `y`
+                             //~| NOTE variable not in all patterns
     }
 }

src/test/compile-fail/issue-2848.rs

@@ -19,7 +19,8 @@ mod bar {
 fn main() {
     use bar::foo::{alpha, charlie};
     match alpha {
-      alpha | beta => {} //~  ERROR variable `beta` from pattern #2 is not bound in pattern #1
+      alpha | beta => {} //~  ERROR variable `beta` is not bound in all patterns
       charlie => {}      //~| NOTE pattern doesn't bind `beta`
+                         //~| NOTE variable not in all patterns
     }
 }

src/test/compile-fail/issue-2849.rs

@@ -13,6 +13,6 @@ enum foo { alpha, beta(isize) }
 fn main() {
     match foo::alpha {
       foo::alpha | foo::beta(i) => {}
-      //~^ ERROR variable `i` from pattern #2 is not bound in pattern #1
+      //~^ ERROR variable `i` is not bound in all patterns
     }
 }

src/test/compile-fail/resolve-inconsistent-binding-mode.rs

@@ -15,7 +15,7 @@ enum opts {
 fn matcher1(x: opts) {
     match x {
       opts::a(ref i) | opts::b(i) => {}
-      //~^ ERROR variable `i` is bound with different mode in pattern #2 than in pattern #1
+      //~^ ERROR variable `i` is bound in inconsistent ways within the same match arm
       //~^^ ERROR mismatched types
       opts::c(_) => {}
     }
@@ -24,7 +24,7 @@ fn matcher1(x: opts) {
 fn matcher2(x: opts) {
     match x {
       opts::a(ref i) | opts::b(i) => {}
-      //~^ ERROR variable `i` is bound with different mode in pattern #2 than in pattern #1
+      //~^ ERROR variable `i` is bound in inconsistent ways within the same match arm
       //~^^ ERROR mismatched types
       opts::c(_) => {}
     }
@@ -33,7 +33,7 @@ fn matcher2(x: opts) {
 fn matcher4(x: opts) {
     match x {
       opts::a(ref mut i) | opts::b(ref i) => {}
-      //~^ ERROR variable `i` is bound with different mode in pattern #2 than in pattern #1
+      //~^ ERROR variable `i` is bound in inconsistent ways within the same match arm
       //~^^ ERROR mismatched types
       opts::c(_) => {}
     }

src/test/compile-fail/resolve-inconsistent-names.rs

@@ -11,9 +11,11 @@
 fn main() {
     let y = 1;
     match y {
-       a | b => {} //~  ERROR variable `a` from pattern #1 is not bound in pattern #2
-                   //~^ ERROR variable `b` from pattern #2 is not bound in pattern #1
+       a | b => {} //~  ERROR variable `a` is not bound in all patterns
+                   //~^ ERROR variable `b` is not bound in all patterns
                    //~| NOTE pattern doesn't bind `a`
                    //~| NOTE pattern doesn't bind `b`
+                   //~| NOTE variable not in all patterns
+                   //~| NOTE variable not in all patterns
     }
 }

src/test/ui/mismatched_types/E0409.stderr

@@ -1,4 +1,4 @@
-error[E0409]: variable `y` is bound with different mode in pattern #2 than in pattern #1
+error[E0409]: variable `y` is bound in inconsistent ways within the same match arm
   --> $DIR/E0409.rs:15:23
    |
 15 |         (0, ref y) | (y, 0) => {} //~ ERROR E0409

src/test/ui/span/issue-39698.rs

@@ -0,0 +1,22 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+enum T {
+    T1(i32, i32),
+    T2(i32, i32),
+    T3(i32),
+    T4(i32),
+}
+
+fn main() {
+    match T::T1(123, 456) {
+        T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
+    }
+}

src/test/ui/span/issue-39698.stderr

@@ -0,0 +1,42 @@
+error[E0408]: variable `a` is not bound in all patterns
+  --> $DIR/issue-39698.rs:20:23
+   |
+20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
+   |               -       ^^^^^^^^^^^   ^^^^^^^^         - variable not in all patterns
+   |               |       |             |
+   |               |       |             pattern doesn't bind `a`
+   |               |       pattern doesn't bind `a`
+   |               variable not in all patterns
+
+error[E0408]: variable `d` is not bound in all patterns
+  --> $DIR/issue-39698.rs:20:37
+   |
+20 |         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 `b` is not bound in all patterns
+  --> $DIR/issue-39698.rs:20:9
+   |
+20 |         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:20:9
+   |
+20 |         T::T1(a, d) | T::T2(d, b) | T::T3(c) | T::T4(a) => { println!("{:?}", a); }
+   |         ^^^^^^^^^^^   ^^^^^^^^^^^         -    ^^^^^^^^ pattern doesn't bind `c`
+   |         |             |                   |
+   |         |             |                   variable not in all patterns
+   |         |             pattern doesn't bind `c`
+   |         pattern doesn't bind `c`
+
+error: aborting due to 4 previous errors
+