check_match: refactor + improve non-exhaustive diag for default binding modes.

Author: Centril

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 {

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
     }

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
    |

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() {}