Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check_match: refactor + improve non-exhaustive diagnostics for default binding modes #64271

Merged
merged 7 commits into from
Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
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> {
Centril marked this conversation as resolved.
Show resolved Hide resolved
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
4 changes: 2 additions & 2 deletions src/librustc_mir/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,9 +517,9 @@ struct PatternContext<'tcx> {
pub struct Witness<'tcx>(Vec<Pattern<'tcx>>);

impl<'tcx> Witness<'tcx> {
pub fn single_pattern(&self) -> &Pattern<'tcx> {
pub fn single_pattern(self) -> Pattern<'tcx> {
assert_eq!(self.0.len(), 1);
&self.0[0]
self.0.into_iter().next().unwrap()
}

fn push_wild_constructor<'a>(
Expand Down
222 changes: 115 additions & 107 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
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,37 +276,26 @@ 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 witnesses = match check_not_useful(cx, pattern_ty, &pats) {
Ok(_) => return,
Err(err) => err,
};

let pattern_string = witness[0].single_pattern().to_string();
let joined_patterns = joined_uncovered_patterns(&witnesses);
let mut err = struct_span_err!(
self.tcx.sess, pat.span, E0005,
"refutable pattern in {}: `{}` not covered",
origin, pattern_string
"refutable pattern in {}: {} not covered",
origin, joined_patterns
);
let label_msg = match pat.node {
PatKind::Path(hir::QPath::Resolved(None, ref path))
if path.segments.len() == 1 && path.segments[0].args.is_none() => {
err.span_label(pat.span, match &pat.node {
PatKind::Path(hir::QPath::Resolved(None, 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));
}
}
_ => pattern_not_convered_label(&witnesses, &joined_patterns),
});
adt_defined_here(cx, &mut err, pattern_ty, &witnesses);
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,124 @@ 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<Pattern<'tcx>>> {
let wild_pattern = Pattern { ty, span: DUMMY_SP, kind: box PatternKind::Wild };
Centril marked this conversation as resolved.
Show resolved Hide resolved
match is_useful(cx, matrix, &[&wild_pattern], ConstructWitness) {
NotUseful => Ok(()), // This is good, wildcard pattern isn't reachable.
UsefulWithWitness(pats) => Err(if pats.is_empty() {
vec![wild_pattern]
} else {
pats.into_iter().map(|w| w.single_pattern()).collect()
}),
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 witnesses = 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 joined_patterns = joined_uncovered_patterns(&witnesses);
let mut err = create_e0004(
cx.tcx.sess, sp,
format!("non-exhaustive patterns: {} not covered", joined_patterns),
);
err.span_label(sp, pattern_not_convered_label(&witnesses, &joined_patterns));
adt_defined_here(cx, &mut err, scrut_ty, &witnesses);
err.help(
"ensure that all possible cases are being handled, \
possibly by adding wildcards or more match arms"
)
.emit();
}

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();
fn joined_uncovered_patterns(witnesses: &[Pattern<'_>]) -> String {
const LIMIT: usize = 3;
match witnesses {
[] => bug!(),
[witness] => format!("`{}`", witness),
[head @ .., tail] if head.len() < LIMIT => {
let head: Vec<_> = head.iter().map(<_>::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(<_>::to_string).collect();
format!("`{}` and {} more", head.join("`, `"), tail.len())
}
_ => bug!()
}
}

fn maybe_point_at_variant(
cx: &mut MatchCheckCtxt<'a, 'tcx>,
ty: Ty<'tcx>,
patterns: &[Pattern<'_>],
) -> Vec<Span> {
fn pattern_not_convered_label(witnesses: &[Pattern<'_>], joined_patterns: &str) -> String {
format!("pattern{} {} not covered", rustc_errors::pluralise!(witnesses.len()), joined_patterns)
}

/// Point at the definition of non-covered `enum` variants.
fn adt_defined_here(
cx: &MatchCheckCtxt<'_, '_>,
err: &mut DiagnosticBuilder<'_>,
ty: Ty<'_>,
witnesses: &[Pattern<'_>],
) {
let ty = ty.peel_refs();
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));
}

if witnesses.len() < 4 {
for sp in maybe_point_at_variant(ty, &witnesses) {
err.span_label(sp, "not covered");
}
}
}
}

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 +717,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
27 changes: 10 additions & 17 deletions src/librustc_typeck/check/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
op.node.as_str(), lhs_ty),
);
let mut suggested_deref = false;
if let Ref(_, mut rty, _) = lhs_ty.sty {
if let Ref(_, rty, _) = lhs_ty.sty {
if {
self.infcx.type_is_copy_modulo_regions(self.param_env,
rty,
Expand All @@ -279,13 +279,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.is_ok()
} {
if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) {
while let Ref(_, rty_inner, _) = rty.sty {
rty = rty_inner;
}
let msg = &format!(
"`{}=` can be used on '{}', you can dereference `{}`",
op.node.as_str(),
rty,
rty.peel_refs(),
lstring,
);
err.span_suggestion(
Expand Down Expand Up @@ -361,7 +358,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}

let mut suggested_deref = false;
if let Ref(_, mut rty, _) = lhs_ty.sty {
if let Ref(_, rty, _) = lhs_ty.sty {
if {
self.infcx.type_is_copy_modulo_regions(self.param_env,
rty,
Expand All @@ -372,17 +369,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.is_ok()
} {
if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) {
while let Ref(_, rty_inner, _) = rty.sty {
rty = rty_inner;
}
let msg = &format!(
"`{}` can be used on '{}', you can \
dereference `{2}`: `*{2}`",
op.node.as_str(),
rty,
lstring
);
err.help(msg);
err.help(&format!(
"`{}` can be used on '{}', you can \
dereference `{2}`: `*{2}`",
op.node.as_str(),
rty.peel_refs(),
lstring
));
suggested_deref = true;
}
}
Expand Down
Loading