From 30db4ebdc225521853889b50cc5164646bbe66ed Mon Sep 17 00:00:00 2001 From: Jakub Adam Wieczorek Date: Fri, 9 Aug 2019 20:21:45 +0000 Subject: [PATCH] Apply suggestions from code review Co-Authored-By: Mazdak Farrokhzad --- src/librustc_resolve/diagnostics.rs | 14 +++--- src/librustc_resolve/late.rs | 48 +++++++++---------- src/librustc_resolve/lib.rs | 2 +- .../resolve/resolve-inconsistent-names.stderr | 6 +-- 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/librustc_resolve/diagnostics.rs b/src/librustc_resolve/diagnostics.rs index b3f6252e4aab..9827ce6ef20f 100644 --- a/src/librustc_resolve/diagnostics.rs +++ b/src/librustc_resolve/diagnostics.rs @@ -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::>(); - let origin_sp = origin.iter().cloned().collect::>(); + let target_sp = target.iter().copied().collect::>(); + let origin_sp = origin.iter().copied().collect::>(); let msp = MultiSpan::from_spans(target_sp.clone()); let msg = format!("variable `{}` is not bound in all patterns", name); @@ -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 diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 13bae25a69af..358eaae11e71 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -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]) { - 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)); } } } @@ -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::>(); inconsistent_vars.sort(); for (name, v) in inconsistent_vars { @@ -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) { diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 1cacc6b7d606..98782dfbc7a5 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -135,7 +135,7 @@ struct BindingError { name: Name, origin: BTreeSet, target: BTreeSet, - could_be_variant: bool + could_be_path: bool } impl PartialOrd for BindingError { diff --git a/src/test/ui/resolve/resolve-inconsistent-names.stderr b/src/test/ui/resolve/resolve-inconsistent-names.stderr index 5a6ae31411ee..f02867a0024b 100644 --- a/src/test/ui/resolve/resolve-inconsistent-names.stderr +++ b/src/test/ui/resolve/resolve-inconsistent-names.stderr @@ -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) => () @@ -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) => () @@ -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) => ()