Address review comments

2024-11-01 23:12:02 +00:00 · 2014-06-19 20:55:12 +02:00 · 2014-06-19 20:55:12 +02:00 · abce42afa3
commit abce42afa3
parent 76f7eeef52
10 changed files with 153 additions and 66 deletions
--- a/src/librustc/middle/check_match.rs
+++ b/src/librustc/middle/check_match.rs
@ -51,13 +51,13 @@ impl Usefulness {
 }

 fn def_to_path(tcx: &ty::ctxt, id: DefId) -> Path {
-    ty::with_path(tcx, id, |path| Path {
+    ty::with_path(tcx, id, |mut path| Path {
        global: false,
-        segments: path.map(|elem| PathSegment {
+        segments: path.last().map(|elem| PathSegment {
            identifier: Ident::new(elem.name()),
            lifetimes: vec!(),
            types: OwnedSlice::empty()
-        }).collect(),
+        }).move_iter().collect(),
        span: DUMMY_SP,
    })
 }
@ -100,10 +100,11 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) {
                                                arm.pats.as_slice());
            }

+            // Second, check for unreachable arms.
            check_arms(cx, arms.as_slice());
-            /* Check for exhaustiveness */
-             // Check for empty enum, because is_useful only works on inhabited
-             // types.
+
+            // Finally, check if the whole match expression is exhaustive.
+            // Check for empty enum, because is_useful only works on inhabited types.
            let pat_ty = node_id_to_type(cx.tcx, scrut.id);
            if (*arms).is_empty() {
               if !type_is_empty(cx.tcx, pat_ty) {
@ -180,11 +181,11 @@ fn check_exhaustive(cx: &MatchCheckCtxt, sp: Span, m: &Matrix) {
        }
        Useful(pats) => {
            let witness = match pats.as_slice() {
-                [ref witness] => witness.clone(),
+                [witness] => witness,
                [] => wild(),
                _ => unreachable!()
            };
-            let msg = format!("non-exhaustive patterns: {0} not covered", pat_to_str(&*witness));
+            let msg = format!("non-exhaustive patterns: `{0}` not covered", pat_to_str(&*witness));
            cx.tcx.sess.span_err(sp, msg.as_slice());
        }
    }
@ -193,7 +194,7 @@ fn check_exhaustive(cx: &MatchCheckCtxt, sp: Span, m: &Matrix) {
 #[deriving(Clone, PartialEq)]
 enum ctor {
    single,
-    variant(DefId),
+    variant(DefId /* variant */, bool /* is_structure */),
    val(const_val),
    range(const_val, const_val),
    vec(uint)
@ -215,23 +216,23 @@ fn construct_witness(cx: &MatchCheckCtxt, ctor: &ctor, pats: Vec<Gc<Pat>>, lty:
    let pat = match ty::get(lty).sty {
        ty::ty_tup(_) => PatTup(pats),

-        ty::ty_enum(_, _) => {
-            let vid = match ctor {
-                &variant(vid) => vid,
-                _ => unreachable!()
+        ty::ty_enum(cid, _) | ty::ty_struct(cid, _)  => {
+            let (vid, is_structure) = match ctor {
+                &variant(vid, is_structure) => (vid, is_structure),
+                _ => (cid, true)
            };
-            PatEnum(def_to_path(cx.tcx, vid), Some(pats))
-        },
-
-        ty::ty_struct(cid, _) => {
-            let fields = ty::lookup_struct_fields(cx.tcx, cid);
-            let field_pats = fields.move_iter()
-                .zip(pats.iter())
-                .map(|(field, pat)| FieldPat {
-                    ident: Ident::new(field.name),
-                    pat: pat.clone()
-                }).collect();
-            PatStruct(def_to_path(cx.tcx, cid), field_pats, false)
+            if is_structure {
+                let fields = ty::lookup_struct_fields(cx.tcx, vid);
+                let field_pats = fields.move_iter()
+                    .zip(pats.iter())
+                    .map(|(field, pat)| FieldPat {
+                        ident: Ident::new(field.name),
+                        pat: pat.clone()
+                    }).collect();
+                PatStruct(def_to_path(cx.tcx, vid), field_pats, false)
+            } else {
+                PatEnum(def_to_path(cx.tcx, vid), Some(pats))
+            }
        },

        ty::ty_rptr(_, ty::mt { ty: ty, .. }) => {
@ -307,7 +308,10 @@ fn all_constructors(cx: &MatchCheckCtxt, m: &Matrix, left_ty: ty::t) -> Vec<ctor
        },

        ty::ty_enum(eid, _) =>
-            ty::enum_variants(cx.tcx, eid).iter().map(|va| variant(va.id)).collect(),
+            ty::enum_variants(cx.tcx, eid)
+                .iter()
+                .map(|va| variant(va.id, va.arg_names.is_some()))
+                .collect(),

        ty::ty_vec(_, None) =>
            vec_constructors(m),
@ -389,8 +393,8 @@ fn is_useful(cx: &MatchCheckCtxt, m: &Matrix, v: &[Gc<Pat>],
            },

            Some(ctor) => {
-                let matrix = &m.iter().filter_map(|r| default(cx, r.as_slice())).collect();
-                match is_useful(cx, matrix, v.tail(), witness) {
+                let matrix = m.iter().filter_map(|r| default(cx, r.as_slice())).collect();
+                match is_useful(cx, &matrix, v.tail(), witness) {
                    Useful(pats) => Useful(match witness {
                        ConstructWitness => {
                            let arity = constructor_arity(cx, &ctor, left_ty);
@ -424,19 +428,28 @@ fn is_useful_specialized(cx: &MatchCheckCtxt, m: &Matrix, v: &[Gc<Pat>],
 fn pat_ctor_id(cx: &MatchCheckCtxt, left_ty: ty::t, p: Gc<Pat>) -> Option<ctor> {
    let pat = raw_pat(p);
    match pat.node {
-        PatIdent(..) | PatEnum(..) | PatStruct(..) =>
+        PatIdent(..) =>
            match cx.tcx.def_map.borrow().find(&pat.id) {
                Some(&DefStatic(did, false)) => {
                    let const_expr = lookup_const_by_id(cx.tcx, did).unwrap();
                    Some(val(eval_const_expr(cx.tcx, &*const_expr)))
                },
-                Some(&DefVariant(_, id, _)) =>
-                    Some(variant(id)),
-                _ => match pat.node {
-                    PatEnum(..) | PatStruct(..) => Some(single),
-                    PatIdent(..) => None,
-                    _ => unreachable!()
-                }
+                Some(&DefVariant(_, id, is_structure)) => Some(variant(id, is_structure)),
+                _ => None
+            },
+        PatEnum(..) =>
+            match cx.tcx.def_map.borrow().find(&pat.id) {
+                Some(&DefStatic(did, false)) => {
+                    let const_expr = lookup_const_by_id(cx.tcx, did).unwrap();
+                    Some(val(eval_const_expr(cx.tcx, &*const_expr)))
+                },
+                Some(&DefVariant(_, id, is_structure)) => Some(variant(id, is_structure)),
+                _ => Some(single)
+            },
+        PatStruct(..) =>
+            match cx.tcx.def_map.borrow().find(&pat.id) {
+                Some(&DefVariant(_, id, is_structure)) => Some(variant(id, is_structure)),
+                _ => Some(single)
            },
        PatLit(expr) =>
            Some(val(eval_const_expr(cx.tcx, &*expr))),
@ -485,7 +498,7 @@ fn constructor_arity(cx: &MatchCheckCtxt, ctor: &ctor, ty: ty::t) -> uint {
        },
        ty::ty_enum(eid, _) => {
            match *ctor {
-                variant(id) => enum_variant_with_id(cx.tcx, eid, id).args.len(),
+                variant(id, _) => enum_variant_with_id(cx.tcx, eid, id).args.len(),
                _ => unreachable!()
            }
        }
@ -532,13 +545,10 @@ fn specialize(cx: &MatchCheckCtxt, r: &[Gc<Pat>],
        &PatIdent(_, _, _) => {
            let opt_def = cx.tcx.def_map.borrow().find_copy(pat_id);
            match opt_def {
-                Some(DefVariant(_, id, _)) => {
-                    if variant(id) == *ctor_id {
-                        Some(vec!())
-                    } else {
-                        None
-                    }
-                }
+                Some(DefVariant(_, id, _)) => match *ctor_id {
+                    variant(vid, _) if vid == id => Some(vec!()),
+                    _ => None
+                },
                Some(DefStatic(did, _)) => {
                    let const_expr = lookup_const_by_id(cx.tcx, did).unwrap();
                    let e_v = eval_const_expr(cx.tcx, &*const_expr);
@ -571,7 +581,7 @@ fn specialize(cx: &MatchCheckCtxt, r: &[Gc<Pat>],
                        }
                    }
                }
-                DefVariant(_, id, _) if variant(id) != *ctor_id => None,
+                DefVariant(_, id, _) if variant(id, false) != *ctor_id => None,
                DefVariant(..) | DefFn(..) | DefStruct(..) => {
                    Some(match args {
                        &Some(ref args) => args.clone(),
@ -586,7 +596,7 @@ fn specialize(cx: &MatchCheckCtxt, r: &[Gc<Pat>],
            // Is this a struct or an enum variant?
            let def = cx.tcx.def_map.borrow().get_copy(pat_id);
            let class_id = match def {
-                DefVariant(_, variant_id, _) => if variant(variant_id) == *ctor_id {
+                DefVariant(_, variant_id, _) => if *ctor_id == variant(variant_id, true) {
                    Some(variant_id)
                } else {
                    None
@ -687,7 +697,7 @@ fn check_local(cx: &mut MatchCheckCtxt, loc: &Local) {
    match is_refutable(cx, loc.pat) {
        Some(pat) => {
            let msg = format!(
-                "refutable pattern in {} binding: {} not covered",
+                "refutable pattern in {} binding: `{}` not covered",
                name, pat_to_str(&*pat)
            );
            cx.tcx.sess.span_err(loc.pat.span, msg.as_slice());
@ -709,7 +719,7 @@ fn check_fn(cx: &mut MatchCheckCtxt,
        match is_refutable(cx, input.pat) {
            Some(pat) => {
                let msg = format!(
-                    "refutable pattern in function argument: {} not covered",
+                    "refutable pattern in function argument: `{}` not covered",
                    pat_to_str(&*pat)
                );
                cx.tcx.sess.span_err(input.pat.span, msg.as_slice());
--- a/src/librustc/middle/typeck/check/_match.rs
+++ b/src/librustc/middle/typeck/check/_match.rs
@ -701,10 +701,7 @@ pub fn check_pat(pcx: &pat_ctxt, pat: &ast::Pat, expected: ty::t) {
                Some(format!("a fixed vector pattern of size {}", min_len)),

            _ => None
-        }).and_then(|message| {
-            check_err(message);
-            Some(())
-        });
+        }).map(check_err);

        for elt in before.iter() {
            check_pat(pcx, &**elt, elt_type);
--- a/src/test/compile-fail/issue-2111.rs
+++ b/src/test/compile-fail/issue-2111.rs
@ -10,7 +10,7 @@

 fn foo(a: Option<uint>, b: Option<uint>) {
  match (a,b) {
-  //~^ ERROR: non-exhaustive patterns: (core::option::None, core::option::None) not covered
+  //~^ ERROR: non-exhaustive patterns: `(None, None)` not covered
    (Some(a), Some(b)) if a == b => { }
    (Some(_), None) |
    (None, Some(_)) => { }
--- a/src/test/compile-fail/issue-4321.rs
+++ b/src/test/compile-fail/issue-4321.rs
@ -10,7 +10,7 @@

 fn main() {
    let tup = (true, true);
-    println!("foo {:}", match tup { //~ ERROR non-exhaustive patterns: (true, false) not covered
+    println!("foo {:}", match tup { //~ ERROR non-exhaustive patterns: `(true, false)` not covered
        (false, false) => "foo",
        (false, true) => "bar",
        (true, true) => "baz"
--- a/src/test/compile-fail/non-exhaustive-match-nested.rs
+++ b/src/test/compile-fail/non-exhaustive-match-nested.rs
@ -13,7 +13,7 @@ enum u { c, d }

 fn main() {
  let x = a(c);
-  match x { //~ ERROR non-exhaustive patterns: a(c) not covered
+  match x { //~ ERROR non-exhaustive patterns: `a(c)` not covered
      a(d) => { fail!("hello"); }
      b => { fail!("goodbye"); }
    }
--- a/src/test/compile-fail/non-exhaustive-match.rs
+++ b/src/test/compile-fail/non-exhaustive-match.rs
@ -12,21 +12,21 @@ enum t { a, b, }

 fn main() {
    let x = a;
-    match x { b => { } } //~ ERROR non-exhaustive patterns: a not covered
-    match true { //~ ERROR non-exhaustive patterns: false not covered
+    match x { b => { } } //~ ERROR non-exhaustive patterns: `a` not covered
+    match true { //~ ERROR non-exhaustive patterns: `false` not covered
      true => {}
    }
-    match Some(10) { //~ ERROR non-exhaustive patterns: core::option::Some(_) not covered
+    match Some(10) { //~ ERROR non-exhaustive patterns: `Some(_)` not covered
      None => {}
    }
-    match (2, 3, 4) { //~ ERROR non-exhaustive patterns: (_, _, _) not covered
+    match (2, 3, 4) { //~ ERROR non-exhaustive patterns: `(_, _, _)` not covered
      (_, _, 4) => {}
    }
-    match (a, a) { //~ ERROR non-exhaustive patterns: (a, a) not covered
+    match (a, a) { //~ ERROR non-exhaustive patterns: `(a, a)` not covered
      (a, b) => {}
      (b, a) => {}
    }
-    match a { //~ ERROR non-exhaustive patterns: b not covered
+    match a { //~ ERROR non-exhaustive patterns: `b` not covered
      a => {}
    }
    // This is exhaustive, though the algorithm got it wrong at one point
@ -37,7 +37,7 @@ fn main() {
    }
    let vec = vec!(Some(42), None, Some(21));
    let vec: &[Option<int>] = vec.as_slice();
-    match vec { //~ ERROR non-exhaustive patterns: [] not covered
+    match vec { //~ ERROR non-exhaustive patterns: `[]` not covered
        [Some(..), None, ..tail] => {}
        [Some(..), Some(..), ..tail] => {}
        [None] => {}
@ -50,7 +50,7 @@ fn main() {
    }
    let vec = vec!(0.5);
    let vec: &[f32] = vec.as_slice();
-    match vec { //~ ERROR non-exhaustive patterns: [_, _, _, _] not covered
+    match vec { //~ ERROR non-exhaustive patterns: `[_, _, _, _]` not covered
        [0.1, 0.2, 0.3] => (),
        [0.1, 0.2] => (),
        [0.1] => (),
--- a/src/test/compile-fail/non-exhaustive-pattern-witness.rs
+++ b/src/test/compile-fail/non-exhaustive-pattern-witness.rs
@ -0,0 +1,74 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(struct_variant)]
+
+struct Foo {
+    first: bool,
+    second: Option<[uint, ..4]>
+}
+
+enum Color {
+    Red,
+    Green,
+    CustomRGBA { a: bool, r: u8, g: u8, b: u8 }
+}
+
+fn struct_with_a_nested_enum_and_vector() {
+    match Foo { first: true, second: None } {
+    //~^ ERROR non-exhaustive patterns: `Foo{first: false, second: Some([_, _, _, _])}` not covered
+        Foo { first: true, second: None } => (),
+        Foo { first: true, second: Some(_) } => (),
+        Foo { first: false, second: None } => (),
+        Foo { first: false, second: Some([1u, 2u, 3u, 4u]) } => ()
+    }
+}
+
+fn enum_with_multiple_missing_variants() {
+    match Red {
+    //~^ ERROR non-exhaustive patterns: `Red` not covered
+        CustomRGBA { .. } => ()
+    }
+}
+
+fn enum_struct_variant() {
+    match Red {
+    //~^ ERROR non-exhaustive patterns: `CustomRGBA{a: true, r: _, g: _, b: _}` not covered
+        Red => (),
+        Green => (),
+        CustomRGBA { a: false, r: _, g: _, b: 0 } => (),
+        CustomRGBA { a: false, r: _, g: _, b: _ } => ()
+    }
+}
+
+enum Enum {
+    First,
+    Second(bool)
+}
+
+fn vectors_with_nested_enums() {
+    let x: &'static [Enum] = [First, Second(false)];
+    match x {
+    //~^ ERROR non-exhaustive patterns: `[Second(true), Second(false)]` not covered
+        [] => (),
+        [_] => (),
+        [First, _] => (),
+        [Second(true), First] => (),
+        [Second(true), Second(true)] => (),
+        [Second(false), _] => (),
+        [_, _, ..tail, _] => ()
+    }
+}
+
+fn main() {
+    struct_with_a_nested_enum_and_vector();
+    enum_with_multiple_missing_variants();
+    enum_struct_variant();
+}
--- a/src/test/compile-fail/refutable-pattern-errors.rs
+++ b/src/test/compile-fail/refutable-pattern-errors.rs
@ -10,9 +10,9 @@


 fn func((1, (Some(1), 2..3)): (int, (Option<int>, int))) { }
-//~^ ERROR refutable pattern in function argument: (_, _) not covered
+//~^ ERROR refutable pattern in function argument: `(_, _)` not covered

 fn main() {
    let (1, (Some(1), 2..3)) = (1, (None, 2));
-    //~^ ERROR refutable pattern in local binding: (_, _) not covered
+    //~^ ERROR refutable pattern in local binding: `(_, _)` not covered
 }
--- a/src/test/compile-fail/refutable-pattern-in-fn-arg.rs
+++ b/src/test/compile-fail/refutable-pattern-in-fn-arg.rs
@ -10,6 +10,6 @@

 fn main() {
    let f = |3: int| println!("hello");
-    //~^ ERROR refutable pattern in function argument: _ not covered
+    //~^ ERROR refutable pattern in function argument: `_` not covered
    f(4);
 }
--- a/src/test/run-pass/issue-7784.rs
+++ b/src/test/run-pass/issue-7784.rs
@ -27,4 +27,10 @@ fn main() {
    let [a, _, _, d] = bar("baz", "foo");
    assert_eq!(a, "baz");
    assert_eq!(d, "baz");
+
+    let out = bar("baz", "foo");
+    let [a, ..xs, d] = out;
+    assert_eq!(a, "baz");
+    assert!(xs == ["foo", "foo"]);
+    assert_eq!(d, "baz");
 }