diff options
| author | Mazdak Farrokhzad <twingoow@gmail.com> | 2019-09-08 00:55:38 +0200 |
|---|---|---|
| committer | Mazdak Farrokhzad <twingoow@gmail.com> | 2019-09-09 16:44:23 +0200 |
| commit | 50a0ec91e066237acd8cc416cd1c0457c8240b96 (patch) | |
| tree | 8f05e8d65a130a261f8acc23a81d461710c6d1a8 | |
| parent | 824383d4ab66abd32abc6e19b68d78ecfddcb7d4 (diff) | |
| download | rust-50a0ec91e066237acd8cc416cd1c0457c8240b96.tar.gz rust-50a0ec91e066237acd8cc416cd1c0457c8240b96.zip | |
check_match: refactor + improve non-exhaustive diag for default binding modes.
| -rw-r--r-- | src/librustc/ty/util.rs | 18 | ||||
| -rw-r--r-- | src/librustc_mir/hair/pattern/check_match.rs | 204 | ||||
| -rw-r--r-- | src/test/ui/consts/match_ice.stderr | 3 | ||||
| -rw-r--r-- | src/test/ui/match/non-exhaustive-defined-here.rs | 65 | ||||
| -rw-r--r-- | src/test/ui/match/non-exhaustive-defined-here.stderr | 151 |
5 files changed, 339 insertions, 102 deletions
diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index a08c82a0ae8..78d94df4fa0 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -996,6 +996,24 @@ impl<'tcx> ty::TyS<'tcx> { debug!("is_type_representable: {:?} is {:?}", self, r); r } + + /// Peel off all reference types in this type until there are none left. + /// + /// This method is idempotent, i.e. `ty.peel_refs().peel_refs() == ty.peel_refs()`. + /// + /// # Examples + /// + /// - `u8` -> `u8` + /// - `&'a mut u8` -> `u8` + /// - `&'a &'b u8` -> `u8` + /// - `&'a *const &'b u8 -> *const &'b u8` + pub fn peel_refs(&'tcx self) -> Ty<'tcx> { + let mut ty = self; + while let Ref(_, inner_ty, _) = ty.sty { + ty = inner_ty; + } + ty + } } fn is_copy_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 5352888006c..c58f5d747e0 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -1,4 +1,4 @@ -use super::_match::{MatchCheckCtxt, Matrix, expand_pattern, is_useful}; +use super::_match::{MatchCheckCtxt, Matrix, Witness, expand_pattern, is_useful}; use super::_match::Usefulness::*; use super::_match::WitnessPreference::*; @@ -61,7 +61,7 @@ struct MatchVisitor<'a, 'tcx> { signalled_error: SignalledError, } -impl<'a, 'tcx> Visitor<'tcx> for MatchVisitor<'a, 'tcx> { +impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> { fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { NestedVisitorMap::None } @@ -98,8 +98,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MatchVisitor<'a, 'tcx> { } } - -impl<'a, 'tcx> PatternContext<'a, 'tcx> { +impl PatternContext<'_, '_> { fn report_inlining_errors(&self, pat_span: Span) { for error in &self.errors { match *error { @@ -131,7 +130,7 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> { } } -impl<'a, 'tcx> MatchVisitor<'a, 'tcx> { +impl<'tcx> MatchVisitor<'_, 'tcx> { fn check_patterns(&mut self, has_guard: bool, pats: &[P<Pat>]) { check_legality_of_move_bindings(self, has_guard, pats); for pat in pats { @@ -277,15 +276,9 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> { expand_pattern(cx, pattern) ]].into_iter().collect(); - let wild_pattern = Pattern { - ty: pattern_ty, - span: DUMMY_SP, - kind: box PatternKind::Wild, - }; - let witness = match is_useful(cx, &pats, &[&wild_pattern], ConstructWitness) { - UsefulWithWitness(witness) => witness, - NotUseful => return, - Useful => bug!() + let witness = match check_not_useful(cx, pattern_ty, &pats) { + Ok(_) => return, + Err((witness, _)) => witness, }; let pattern_string = witness[0].single_pattern().to_string(); @@ -294,20 +287,15 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> { "refutable pattern in {}: `{}` not covered", origin, pattern_string ); - let label_msg = match pat.node { + err.span_label(pat.span, match pat.node { PatKind::Path(hir::QPath::Resolved(None, ref path)) if path.segments.len() == 1 && path.segments[0].args.is_none() => { format!("interpreted as {} {} pattern, not new variable", path.res.article(), path.res.descr()) } _ => format!("pattern `{}` not covered", pattern_string), - }; - err.span_label(pat.span, label_msg); - if let ty::Adt(def, _) = pattern_ty.sty { - if let Some(sp) = self.tcx.hir().span_if_local(def.did){ - err.span_label(sp, format!("`{}` defined here", pattern_ty)); - } - } + }); + adt_defined_here(cx, pattern_ty.peel_refs(), &mut err); err.emit(); }); } @@ -362,9 +350,9 @@ fn pat_is_catchall(pat: &Pat) -> bool { } // Check for unreachable patterns -fn check_arms<'a, 'tcx>( - cx: &mut MatchCheckCtxt<'a, 'tcx>, - arms: &[(Vec<(&'a Pattern<'tcx>, &hir::Pat)>, Option<&hir::Expr>)], +fn check_arms<'tcx>( + cx: &mut MatchCheckCtxt<'_, 'tcx>, + arms: &[(Vec<(&Pattern<'tcx>, &hir::Pat)>, Option<&hir::Expr>)], source: hir::MatchSource, ) { let mut seen = Matrix::empty(); @@ -445,104 +433,116 @@ fn check_arms<'a, 'tcx>( } } -fn check_exhaustive<'p, 'a, 'tcx>( - cx: &mut MatchCheckCtxt<'a, 'tcx>, +fn check_not_useful( + cx: &mut MatchCheckCtxt<'_, 'tcx>, + ty: Ty<'tcx>, + matrix: &Matrix<'_, 'tcx>, +) -> Result<(), (Vec<Witness<'tcx>>, Pattern<'tcx>)> { + let wild_pattern = Pattern { ty, span: DUMMY_SP, kind: box PatternKind::Wild }; + match is_useful(cx, matrix, &[&wild_pattern], ConstructWitness) { + NotUseful => Ok(()), // This is good, wildcard pattern isn't reachable. + UsefulWithWitness(pats) => Err((pats, wild_pattern)), + Useful => bug!(), + } +} + +fn check_exhaustive<'tcx>( + cx: &mut MatchCheckCtxt<'_, 'tcx>, scrut_ty: Ty<'tcx>, sp: Span, - matrix: &Matrix<'p, 'tcx>, + matrix: &Matrix<'_, 'tcx>, ) { - let wild_pattern = Pattern { - ty: scrut_ty, - span: DUMMY_SP, - kind: box PatternKind::Wild, + let (pats, wild_pattern) = match check_not_useful(cx, scrut_ty, matrix) { + Ok(_) => return, + Err(err) => err, }; - match is_useful(cx, matrix, &[&wild_pattern], ConstructWitness) { - UsefulWithWitness(pats) => { - let witnesses = if pats.is_empty() { - vec![&wild_pattern] - } else { - pats.iter().map(|w| w.single_pattern()).collect() - }; - const LIMIT: usize = 3; - let joined_patterns = match witnesses.len() { - 0 => bug!(), - 1 => format!("`{}`", witnesses[0]), - 2..=LIMIT => { - let (tail, head) = witnesses.split_last().unwrap(); - let head: Vec<_> = head.iter().map(|w| w.to_string()).collect(); - format!("`{}` and `{}`", head.join("`, `"), tail) - } - _ => { - let (head, tail) = witnesses.split_at(LIMIT); - let head: Vec<_> = head.iter().map(|w| w.to_string()).collect(); - format!("`{}` and {} more", head.join("`, `"), tail.len()) - } - }; + let witnesses = if pats.is_empty() { + vec![&wild_pattern] + } else { + pats.iter().map(|w| w.single_pattern()).collect() + }; - let label_text = match witnesses.len() { - 1 => format!("pattern {} not covered", joined_patterns), - _ => format!("patterns {} not covered", joined_patterns), - }; - let mut err = create_e0004(cx.tcx.sess, sp, format!( - "non-exhaustive patterns: {} not covered", - joined_patterns, - )); - err.span_label(sp, label_text); - // point at the definition of non-covered enum variants - if let ty::Adt(def, _) = scrut_ty.sty { - if let Some(sp) = cx.tcx.hir().span_if_local(def.did){ - err.span_label(sp, format!("`{}` defined here", scrut_ty)); - } - } - let patterns = witnesses.iter().map(|p| (**p).clone()).collect::<Vec<Pattern<'_>>>(); - if patterns.len() < 4 { - for sp in maybe_point_at_variant(cx, scrut_ty, patterns.as_slice()) { - err.span_label(sp, "not covered"); - } - } - err.help("ensure that all possible cases are being handled, \ - possibly by adding wildcards or more match arms"); - err.emit(); + const LIMIT: usize = 3; + let joined_patterns = match witnesses.len() { + 0 => bug!(), + 1 => format!("`{}`", witnesses[0]), + 2..=LIMIT => { + let (tail, head) = witnesses.split_last().unwrap(); + let head: Vec<_> = head.iter().map(|w| w.to_string()).collect(); + format!("`{}` and `{}`", head.join("`, `"), tail) } - NotUseful => { - // This is good, wildcard pattern isn't reachable + _ => { + let (head, tail) = witnesses.split_at(LIMIT); + let head: Vec<_> = head.iter().map(|w| w.to_string()).collect(); + format!("`{}` and {} more", head.join("`, `"), tail.len()) + } + }; + + let mut err = create_e0004(cx.tcx.sess, sp, format!( + "non-exhaustive patterns: {} not covered", + joined_patterns, + )); + err.span_label(sp, match witnesses.len() { + 1 => format!("pattern {} not covered", joined_patterns), + _ => format!("patterns {} not covered", joined_patterns), + }); + // point at the definition of non-covered enum variants + let scrut_ty = scrut_ty.peel_refs(); + adt_defined_here(cx, scrut_ty, &mut err); + let patterns = witnesses.iter().map(|p| (**p).clone()).collect::<Vec<Pattern<'_>>>(); + if patterns.len() < 4 { + for sp in maybe_point_at_variant(scrut_ty, &patterns) { + err.span_label(sp, "not covered"); } - _ => bug!() } + err.help("ensure that all possible cases are being handled, \ + possibly by adding wildcards or more match arms"); + err.emit(); } -fn maybe_point_at_variant( - cx: &mut MatchCheckCtxt<'a, 'tcx>, - ty: Ty<'tcx>, - patterns: &[Pattern<'_>], -) -> Vec<Span> { +fn adt_defined_here(cx: &mut MatchCheckCtxt<'_, '_>, ty: Ty<'_>, err: &mut DiagnosticBuilder<'_>) { + if let ty::Adt(def, _) = ty.sty { + if let Some(sp) = cx.tcx.hir().span_if_local(def.did) { + err.span_label(sp, format!("`{}` defined here", ty)); + } + } +} + +fn maybe_point_at_variant(ty: Ty<'_>, patterns: &[Pattern<'_>]) -> Vec<Span> { let mut covered = vec![]; if let ty::Adt(def, _) = ty.sty { // Don't point at variants that have already been covered due to other patterns to avoid - // visual clutter + // visual clutter. for pattern in patterns { - let pk: &PatternKind<'_> = &pattern.kind; - if let PatternKind::Variant { adt_def, variant_index, subpatterns, .. } = pk { - if adt_def.did == def.did { + use PatternKind::{AscribeUserType, Deref, Variant, Or, Leaf}; + match &*pattern.kind { + AscribeUserType { subpattern, .. } | Deref { subpattern } => { + covered.extend(maybe_point_at_variant(ty, slice::from_ref(&subpattern))); + } + Variant { adt_def, variant_index, subpatterns, .. } if adt_def.did == def.did => { let sp = def.variants[*variant_index].ident.span; if covered.contains(&sp) { continue; } covered.push(sp); - let subpatterns = subpatterns.iter() + + let pats = subpatterns.iter() .map(|field_pattern| field_pattern.pattern.clone()) - .collect::<Vec<_>>(); - covered.extend( - maybe_point_at_variant(cx, ty, subpatterns.as_slice()), - ); + .collect::<Box<[_]>>(); + covered.extend(maybe_point_at_variant(ty, &pats)); } - } - if let PatternKind::Leaf { subpatterns } = pk { - let subpatterns = subpatterns.iter() - .map(|field_pattern| field_pattern.pattern.clone()) - .collect::<Vec<_>>(); - covered.extend(maybe_point_at_variant(cx, ty, subpatterns.as_slice())); + Leaf { subpatterns } => { + let pats = subpatterns.iter() + .map(|field_pattern| field_pattern.pattern.clone()) + .collect::<Box<[_]>>(); + covered.extend(maybe_point_at_variant(ty, &pats)); + } + Or { pats } => { + let pats = pats.iter().cloned().collect::<Box<[_]>>(); + covered.extend(maybe_point_at_variant(ty, &pats)); + } + _ => {} } } } @@ -709,7 +709,7 @@ struct AtBindingPatternVisitor<'a, 'b, 'tcx> { bindings_allowed: bool } -impl<'a, 'b, 'tcx, 'v> Visitor<'v> for AtBindingPatternVisitor<'a, 'b, 'tcx> { +impl<'v> Visitor<'v> for AtBindingPatternVisitor<'_, '_, '_> { fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'v> { NestedVisitorMap::None } diff --git a/src/test/ui/consts/match_ice.stderr b/src/test/ui/consts/match_ice.stderr index 158581fcb15..bf0bd3aca97 100644 --- a/src/test/ui/consts/match_ice.stderr +++ b/src/test/ui/consts/match_ice.stderr @@ -7,6 +7,9 @@ LL | C => {} error[E0004]: non-exhaustive patterns: `&T` not covered --> $DIR/match_ice.rs:15:11 | +LL | struct T; + | --------- `T` defined here +... LL | match K { | ^ pattern `&T` not covered | diff --git a/src/test/ui/match/non-exhaustive-defined-here.rs b/src/test/ui/match/non-exhaustive-defined-here.rs new file mode 100644 index 00000000000..1ba7c2a66ba --- /dev/null +++ b/src/test/ui/match/non-exhaustive-defined-here.rs @@ -0,0 +1,65 @@ +// Test the "defined here" and "not covered" diagnostic hints. +// We also make sure that references are peeled off from the scrutinee type +// so that the diagnostics work better with default binding modes. + +#[derive(Clone)] +enum E { +//~^ `E` defined here +//~| `E` defined here +//~| `E` defined here +//~| `E` defined here +//~| `E` defined here +//~| `E` defined here + A, + B, + //~^ not covered + //~| not covered + //~| not covered + C + //~^ not covered + //~| not covered + //~| not covered +} + +fn by_val(e: E) { + let e1 = e.clone(); + match e1 { //~ ERROR non-exhaustive patterns: `B` and `C` not covered + E::A => {} + } + + let E::A = e; //~ ERROR refutable pattern in local binding: `B` not covered +} + +fn by_ref_once(e: &E) { + match e { //~ ERROR non-exhaustive patterns: `&B` and `&C` not covered + E::A => {} + } + + let E::A = e; //~ ERROR refutable pattern in local binding: `&B` not covered +} + +fn by_ref_thrice(e: & &mut &E) { + match e { //~ ERROR non-exhaustive patterns: `&&mut &B` and `&&mut &C` not covered + E::A => {} + } + + let E::A = e; //~ ERROR refutable pattern in local binding: `&&mut &B` not covered +} + +enum Opt { +//~^ `Opt` defined here +//~| `Opt` defined here + Some(u8), + None, + //~^ not covered +} + +fn ref_pat(e: Opt) { + match e {//~ ERROR non-exhaustive patterns: `None` not covered + Opt::Some(ref _x) => {} + } + + let Opt::Some(ref _x) = e; //~ ERROR refutable pattern in local binding: `None` not covered +} + +fn main() {} diff --git a/src/test/ui/match/non-exhaustive-defined-here.stderr b/src/test/ui/match/non-exhaustive-defined-here.stderr new file mode 100644 index 00000000000..b0dccc975ab --- /dev/null +++ b/src/test/ui/match/non-exhaustive-defined-here.stderr @@ -0,0 +1,151 @@ +error[E0004]: non-exhaustive patterns: `B` and `C` not covered + --> $DIR/non-exhaustive-defined-here.rs:26:11 + | +LL | / enum E { +LL | | +LL | | +LL | | +... | +LL | | B, + | | - not covered +... | +LL | | C + | | - not covered +... | +LL | | +LL | | } + | |_- `E` defined here +... +LL | match e1 { + | ^^ patterns `B` and `C` not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + +error[E0005]: refutable pattern in local binding: `B` not covered + --> $DIR/non-exhaustive-defined-here.rs:30:9 + | +LL | / enum E { +LL | | +LL | | +LL | | +... | +LL | | +LL | | } + | |_- `E` defined here +... +LL | let E::A = e; + | ^^^^ pattern `B` not covered + +error[E0004]: non-exhaustive patterns: `&B` and `&C` not covered + --> $DIR/non-exhaustive-defined-here.rs:34:11 + | +LL | / enum E { +LL | | +LL | | +LL | | +... | +LL | | B, + | | - not covered +... | +LL | | C + | | - not covered +... | +LL | | +LL | | } + | |_- `E` defined here +... +LL | match e { + | ^ patterns `&B` and `&C` not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + +error[E0005]: refutable pattern in local binding: `&B` not covered + --> $DIR/non-exhaustive-defined-here.rs:38:9 + | +LL | / enum E { +LL | | +LL | | +LL | | +... | +LL | | +LL | | } + | |_- `E` defined here +... +LL | let E::A = e; + | ^^^^ pattern `&B` not covered + +error[E0004]: non-exhaustive patterns: `&&mut &B` and `&&mut &C` not covered + --> $DIR/non-exhaustive-defined-here.rs:42:11 + | +LL | / enum E { +LL | | +LL | | +LL | | +... | +LL | | B, + | | - not covered +... | +LL | | C + | | - not covered +... | +LL | | +LL | | } + | |_- `E` defined here +... +LL | match e { + | ^ patterns `&&mut &B` and `&&mut &C` not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + +error[E0005]: refutable pattern in local binding: `&&mut &B` not covered + --> $DIR/non-exhaustive-defined-here.rs:46:9 + | +LL | / enum E { +LL | | +LL | | +LL | | +... | +LL | | +LL | | } + | |_- `E` defined here +... +LL | let E::A = e; + | ^^^^ pattern `&&mut &B` not covered + +error[E0004]: non-exhaustive patterns: `None` not covered + --> $DIR/non-exhaustive-defined-here.rs:58:11 + | +LL | / enum Opt { +LL | | +LL | | +LL | | Some(u8), +LL | | None, + | | ---- not covered +LL | | +LL | | } + | |_- `Opt` defined here +... +LL | match e { + | ^ pattern `None` not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + +error[E0005]: refutable pattern in local binding: `None` not covered + --> $DIR/non-exhaustive-defined-here.rs:62:9 + | +LL | / enum Opt { +LL | | +LL | | +LL | | Some(u8), +LL | | None, +LL | | +LL | | } + | |_- `Opt` defined here +... +LL | let Opt::Some(ref _x) = e; + | ^^^^^^^^^^^^^^^^^ pattern `None` not covered + +error: aborting due to 8 previous errors + +Some errors have detailed explanations: E0004, E0005. +For more information about an error, try `rustc --explain E0004`. |
