Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-Authored-By: Mazdak Farrokhzad <twingoow@gmail.com>
  • Loading branch information
jakubadamw and Centril committed Aug 10, 2019
1 parent 53a6304 commit 30db4eb
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 35 deletions.
14 changes: 8 additions & 6 deletions src/librustc_resolve/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,10 @@ impl<'a> Resolver<'a> {
err
}
ResolutionError::VariableNotBoundInPattern(binding_error) => {
let BindingError { name, target, origin, could_be_variant } = binding_error;
let BindingError { name, target, origin, could_be_path } = binding_error;

let target_sp = target.iter().cloned().collect::<Vec<_>>();
let origin_sp = origin.iter().cloned().collect::<Vec<_>>();
let target_sp = target.iter().copied().collect::<Vec<_>>();
let origin_sp = origin.iter().copied().collect::<Vec<_>>();

let msp = MultiSpan::from_spans(target_sp.clone());
let msg = format!("variable `{}` is not bound in all patterns", name);
Expand All @@ -225,10 +225,12 @@ impl<'a> Resolver<'a> {
for sp in origin_sp {
err.span_label(sp, "variable not in all patterns");
}
if *could_be_variant {
if *could_be_path {
let help_msg = format!(
"if you meant to match on a variant or a const, consider \
making the path in the pattern qualified: `?::{}`", name);
"if you meant to match on a variant or a `const` item, consider \
making the path in the pattern qualified: `?::{}`",
name,
);
err.span_help(span, &help_msg);
}
err
Expand Down
48 changes: 23 additions & 25 deletions src/librustc_resolve/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1136,40 +1136,35 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
// 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>]) {
if pats.len() <= 1 {
return;
}

let mut missing_vars = FxHashMap::default();
let mut inconsistent_vars = FxHashMap::default();
for p in pats.iter() {
let map_i = self.binding_mode_map(&p);
for q in pats.iter() {
if p.id == q.id {
continue;
}

let map_j = self.binding_mode_map(&q);
for (&key_j, &binding_j) in map_j.iter() {
match map_i.get(&key_j) {
for pat_outer in pats.iter() {
let map_outer = self.binding_mode_map(&pat_outer);

for pat_inner in pats.iter().filter(|pat| pat.id != pat_outer.id) {
let map_inner = self.binding_mode_map(&pat_inner);

for (&key_inner, &binding_inner) in map_inner.iter() {
match map_outer.get(&key_inner) {
None => { // missing binding
let binding_error = missing_vars
.entry(key_j.name)
.entry(key_inner.name)
.or_insert(BindingError {
name: key_j.name,
name: key_inner.name,
origin: BTreeSet::new(),
target: BTreeSet::new(),
could_be_variant:
key_j.name.as_str().starts_with(char::is_uppercase)
could_be_path:
key_inner.name.as_str().starts_with(char::is_uppercase)
});
binding_error.origin.insert(binding_j.span);
binding_error.target.insert(p.span);
binding_error.origin.insert(binding_inner.span);
binding_error.target.insert(pat_outer.span);
}
Some(binding_i) => { // check consistent binding
if binding_i.binding_mode != binding_j.binding_mode {
Some(binding_outer) => { // check consistent binding
if binding_outer.binding_mode != binding_inner.binding_mode {
inconsistent_vars
.entry(key_j.name)
.or_insert((binding_j.span, binding_i.span));
.entry(key_inner.name)
.or_insert((binding_inner.span, binding_outer.span));
}
}
}
Expand All @@ -1181,12 +1176,13 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
missing_vars.sort();
for (name, mut v) in missing_vars {
if inconsistent_vars.contains_key(name) {
v.could_be_variant = false;
v.could_be_path = false;
}
self.r.report_error(
*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 {
Expand Down Expand Up @@ -1214,7 +1210,9 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
self.resolve_pattern(pat, source, &mut bindings_list);
}
// This has to happen *after* we determine which pat_idents are variants
self.check_consistent_bindings(pats);
if pats.len() > 1 {
self.check_consistent_bindings(pats);
}
}

fn resolve_block(&mut self, block: &Block) {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ struct BindingError {
name: Name,
origin: BTreeSet<Span>,
target: BTreeSet<Span>,
could_be_variant: bool
could_be_path: bool
}

impl PartialOrd for BindingError {
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/resolve/resolve-inconsistent-names.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ LL | (A, B) | (ref B, c) | (c, A) => ()
| | pattern doesn't bind `A`
| variable not in all patterns
|
help: if you meant to match on a variant or a const, consider making the path in the pattern qualified: `?::A`
help: if you meant to match on a variant or a `const` item, consider making the path in the pattern qualified: `?::A`
--> $DIR/resolve-inconsistent-names.rs:19:10
|
LL | (A, B) | (ref B, c) | (c, A) => ()
Expand Down Expand Up @@ -63,7 +63,7 @@ LL | (CONST1, _) | (_, Const2) => ()
| |
| variable not in all patterns
|
help: if you meant to match on a variant or a const, consider making the path in the pattern qualified: `?::CONST1`
help: if you meant to match on a variant or a `const` item, consider making the path in the pattern qualified: `?::CONST1`
--> $DIR/resolve-inconsistent-names.rs:30:10
|
LL | (CONST1, _) | (_, Const2) => ()
Expand All @@ -77,7 +77,7 @@ LL | (CONST1, _) | (_, Const2) => ()
| |
| pattern doesn't bind `Const2`
|
help: if you meant to match on a variant or a const, consider making the path in the pattern qualified: `?::Const2`
help: if you meant to match on a variant or a `const` item, consider making the path in the pattern qualified: `?::Const2`
--> $DIR/resolve-inconsistent-names.rs:30:27
|
LL | (CONST1, _) | (_, Const2) => ()
Expand Down

0 comments on commit 30db4eb

Please sign in to comment.