Skip to content

Commit

Permalink
check_match: refactor + improve non-exhaustive diag for default bindi…
Browse files Browse the repository at this point in the history 
…ng modes.
  • Loading branch information
@Centril
Centril committed Sep 9, 2019
1 parent 824383d commit 50a0ec9
Show file tree
Hide file tree
Showing 5 changed files with 339 additions and 102 deletions.
18 changes: 18 additions & 0 deletions src/librustc/ty/util.rs
View file
Expand Up @@ -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 {
Expand Down
204 changes: 102 additions & 102 deletions src/librustc_mir/hair/pattern/check_match.rs
View file
@@ -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::*;

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand All @@ -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();
});
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}
_ => {}
}
}
}
Expand Down Expand Up @@ -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
}
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/consts/match_ice.stderr
View file
Expand Up @@ -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
|
Expand Down
65 changes: 65 additions & 0 deletions src/test/ui/match/non-exhaustive-defined-here.rs
View file
@@ -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() {}

0 comments on commit 50a0ec9

Please sign in to comment.