Skip to content

Commit

Permalink
Auto merge of #39713 - estebank:issue-39698, r=jonathandturner
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Mar 8, 2017
2 parents b04ebef + 3ffa4b5 commit f91c3f6
Show file tree
Hide file tree
Showing 9 changed files with 175 additions and 50 deletions.
139 changes: 98 additions & 41 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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));
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/test/compile-fail/E0408.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
3 changes: 2 additions & 1 deletion src/test/compile-fail/issue-2848.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
2 changes: 1 addition & 1 deletion src/test/compile-fail/issue-2849.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
6 changes: 3 additions & 3 deletions src/test/compile-fail/resolve-inconsistent-binding-mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_) => {}
}
Expand All @@ -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(_) => {}
}
Expand All @@ -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(_) => {}
}
Expand Down
6 changes: 4 additions & 2 deletions src/test/compile-fail/resolve-inconsistent-names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
2 changes: 1 addition & 1 deletion src/test/ui/mismatched_types/E0409.stderr
Original file line number Diff line number Diff line change
@@ -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
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/span/issue-39698.rs
Original file line number Diff line number Diff line change
@@ -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); }
}
}
42 changes: 42 additions & 0 deletions src/test/ui/span/issue-39698.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit f91c3f6

Please sign in to comment.