Fix #12285

Unit-like struct patterns are irrefutable, no need for a branch.

Author: Jakub Wieczorek

src/librustc/middle/check_match.rs

@@ -93,9 +93,10 @@ pub enum Constructor {
     Slice(uint)
 }
 
-#[deriving(Clone)]
+#[deriving(Clone, PartialEq)]
 enum Usefulness {
-    Useful(Vec<Gc<Pat>>),
+    Useful,
+    UsefulWithWitness(Vec<Gc<Pat>>),
     NotUseful
 }
 
@@ -104,15 +105,6 @@ enum WitnessPreference {
     LeaveOutWitness
 }
 
-impl Usefulness {
-    fn useful(self) -> Option<Vec<Gc<Pat>>> {
-        match self {
-            Useful(pats) => Some(pats),
-            _ => None
-        }
-    }
-}
-
 impl<'a> Visitor<()> for MatchCheckCtxt<'a> {
     fn visit_expr(&mut self, ex: &Expr, _: ()) {
         check_expr(self, ex);
@@ -203,7 +195,8 @@ fn check_arms(cx: &MatchCheckCtxt, arms: &[Arm]) {
             let v = vec!(*pat);
             match is_useful(cx, &seen, v.as_slice(), LeaveOutWitness) {
                 NotUseful => cx.tcx.sess.span_err(pat.span, "unreachable pattern"),
-                _ => ()
+                Useful => (),
+                UsefulWithWitness(_) => unreachable!()
             }
             if arm.guard.is_none() {
                 let Matrix(mut rows) = seen;
@@ -223,7 +216,7 @@ fn raw_pat(p: Gc<Pat>) -> Gc<Pat> {
 
 fn check_exhaustive(cx: &MatchCheckCtxt, sp: Span, m: &Matrix) {
     match is_useful(cx, m, [wild()], ConstructWitness) {
-        Useful(pats) => {
+        UsefulWithWitness(pats) => {
             let witness = match pats.as_slice() {
                 [witness] => witness,
                 [] => wild(),
@@ -234,7 +227,8 @@ fn check_exhaustive(cx: &MatchCheckCtxt, sp: Span, m: &Matrix) {
         }
         NotUseful => {
             // This is good, wildcard pattern isn't reachable
-        }
+        },
+        _ => unreachable!()
     }
 }
 
@@ -404,11 +398,14 @@ fn all_constructors(cx: &MatchCheckCtxt, left_ty: ty::t,
 
 // Note: is_useful doesn't work on empty types, as the paper notes.
 // So it assumes that v is non-empty.
-fn is_useful(cx: &MatchCheckCtxt, m @ &Matrix(ref rows): &Matrix,
+fn is_useful(cx: &MatchCheckCtxt, matrix @ &Matrix(ref rows): &Matrix,
              v: &[Gc<Pat>], witness: WitnessPreference) -> Usefulness {
-    debug!("{:}", m);
+    debug!("{:}", matrix);
     if rows.len() == 0u {
-        return Useful(vec!());
+        return match witness {
+            ConstructWitness => UsefulWithWitness(vec!()),
+            LeaveOutWitness => Useful
+        };
     }
     if rows.get(0).len() == 0u {
         return NotUseful;
@@ -438,53 +435,46 @@ fn is_useful(cx: &MatchCheckCtxt, m @ &Matrix(ref rows): &Matrix,
 
     let constructors = pat_constructors(cx, v[0], left_ty, max_slice_length);
     if constructors.is_empty() {
-        match missing_constructor(cx, m, left_ty, max_slice_length) {
+        match missing_constructor(cx, matrix, left_ty, max_slice_length) {
             None => {
-                all_constructors(cx, left_ty, max_slice_length).move_iter().filter_map(|c| {
-                    is_useful_specialized(cx, m, v, c.clone(),
-                                          left_ty, witness).useful().map(|pats| {
-                        Useful(match witness {
-                            ConstructWitness => {
-                                let arity = constructor_arity(cx, &c, left_ty);
-                                let subpats = {
-                                    let pat_slice = pats.as_slice();
-                                    Vec::from_fn(arity, |i| {
-                                        pat_slice.get(i).map(|p| p.clone())
-                                            .unwrap_or_else(|| wild())
-                                    })
-                                };
-                                let mut result = vec!(construct_witness(cx, &c, subpats, left_ty));
-                                result.extend(pats.move_iter().skip(arity));
-                                result
-                            }
-                            LeaveOutWitness => vec!()
-                        })
-                    })
-                }).nth(0).unwrap_or(NotUseful)
+                all_constructors(cx, left_ty, max_slice_length).move_iter().map(|c| {
+                    match is_useful_specialized(cx, matrix, v, c.clone(), left_ty, witness) {
+                        UsefulWithWitness(pats) => UsefulWithWitness({
+                            let arity = constructor_arity(cx, &c, left_ty);
+                            let subpats = {
+                                let pat_slice = pats.as_slice();
+                                Vec::from_fn(arity, |i| {
+                                    pat_slice.get(i).map(|p| p.clone())
+                                        .unwrap_or_else(|| wild())
+                                })
+                            };
+                            let mut result = vec!(construct_witness(cx, &c, subpats, left_ty));
+                            result.extend(pats.move_iter().skip(arity));
+                            result
+                        }),
+                        result => result
+                    }
+                }).find(|result| result != &NotUseful).unwrap_or(NotUseful)
             },
 
             Some(constructor) => {
                 let matrix = Matrix(rows.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, &constructor, left_ty);
-                            let wild_pats = Vec::from_elem(arity, wild());
-                            let enum_pat = construct_witness(cx, &constructor, wild_pats, left_ty);
-                            (vec!(enum_pat)).append(pats.as_slice())
-                        }
-                        LeaveOutWitness => vec!()
-                    }),
+                    UsefulWithWitness(pats) => {
+                        let arity = constructor_arity(cx, &constructor, left_ty);
+                        let wild_pats = Vec::from_elem(arity, wild());
+                        let enum_pat = construct_witness(cx, &constructor, wild_pats, left_ty);
+                        UsefulWithWitness(vec!(enum_pat).append(pats.as_slice()))
+                    },
                     result => result
                 }
             }
         }
     } else {
-        constructors.move_iter().filter_map(|c| {
-            is_useful_specialized(cx, m, v, c.clone(), left_ty, witness)
-                .useful().map(|pats| Useful(pats))
-        }).nth(0).unwrap_or(NotUseful)
+        constructors.move_iter().map(|c|
+            is_useful_specialized(cx, matrix, v, c.clone(), left_ty, witness)
+        ).find(|result| result != &NotUseful).unwrap_or(NotUseful)
     }
 }
 
@@ -519,6 +509,7 @@ fn pat_constructors(cx: &MatchCheckCtxt, p: Gc<Pat>,
                     let const_expr = lookup_const_by_id(cx.tcx, did).unwrap();
                     vec!(ConstantValue(eval_const_expr(cx.tcx, &*const_expr)))
                 },
+                Some(&DefStruct(_)) => vec!(Single),
                 Some(&DefVariant(_, id, _)) => vec!(Variant(id)),
                 _ => vec!()
             },
@@ -560,21 +551,6 @@ fn pat_constructors(cx: &MatchCheckCtxt, p: Gc<Pat>,
     }
 }
 
-fn is_wild(cx: &MatchCheckCtxt, p: Gc<Pat>) -> bool {
-    let pat = raw_pat(p);
-    match pat.node {
-        PatWild | PatWildMulti => true,
-        PatIdent(_, _, _) =>
-            match cx.tcx.def_map.borrow().find(&pat.id) {
-                Some(&DefVariant(_, _, _)) | Some(&DefStatic(..)) => false,
-                _ => true
-            },
-        PatVec(ref before, Some(_), ref after) =>
-            before.is_empty() && after.is_empty(),
-        _ => false
-    }
-}
-
 /// This computes the arity of a constructor. The arity of a constructor
 /// is how many subpattern patterns of that constructor should be expanded to.
 ///
@@ -780,7 +756,7 @@ pub fn specialize(cx: &MatchCheckCtxt, r: &[Gc<Pat>],
 }
 
 fn default(cx: &MatchCheckCtxt, r: &[Gc<Pat>]) -> Option<Vec<Gc<Pat>>> {
-    if is_wild(cx, r[0]) {
+    if pat_is_binding_or_wild(&cx.tcx.def_map, &*raw_pat(r[0])) {
         Some(Vec::from_slice(r.tail()))
     } else {
         None
@@ -833,12 +809,14 @@ fn check_fn(cx: &mut MatchCheckCtxt,
 
 fn is_refutable(cx: &MatchCheckCtxt, pat: Gc<Pat>) -> Option<Gc<Pat>> {
     let pats = Matrix(vec!(vec!(pat)));
-    is_useful(cx, &pats, [wild()], ConstructWitness)
-        .useful()
-        .map(|pats| {
+    match is_useful(cx, &pats, [wild()], ConstructWitness) {
+        UsefulWithWitness(pats) => {
             assert_eq!(pats.len(), 1);
-            pats.get(0).clone()
-        })
+            Some(pats.get(0).clone())
+        },
+        NotUseful => None,
+        Useful => unreachable!()
+    }
 }
 
 // Legality of move bindings checking

src/librustc/middle/trans/_match.rs

@@ -212,7 +212,6 @@ use middle::trans::cleanup::CleanupMethods;
 use middle::trans::common::*;
 use middle::trans::consts;
 use middle::trans::controlflow;
-use middle::trans::datum;
 use middle::trans::datum::*;
 use middle::trans::expr::Dest;
 use middle::trans::expr;
@@ -236,10 +235,8 @@ use syntax::ast_util;
 use syntax::codemap::Span;
 use syntax::parse::token::InternedString;
 
-// An option identifying a literal: either a unit-like struct or an
-// expression.
+// An option identifying a literal: either an expression or a DefId of a static expression.
 enum Lit {
-    UnitLikeStructLit(ast::NodeId),    // the node ID of the pattern
     ExprLit(Gc<ast::Expr>),
     ConstLit(ast::DefId),              // the def ID of the constant
 }
@@ -262,14 +259,12 @@ enum Opt {
 fn lit_to_expr(tcx: &ty::ctxt, a: &Lit) -> Gc<ast::Expr> {
     match *a {
         ExprLit(existing_a_expr) => existing_a_expr,
-        ConstLit(a_const) => const_eval::lookup_const_by_id(tcx, a_const).unwrap(),
-        UnitLikeStructLit(_) => fail!("lit_to_expr: unexpected struct lit"),
+        ConstLit(a_const) => const_eval::lookup_const_by_id(tcx, a_const).unwrap()
     }
 }
 
 fn opt_eq(tcx: &ty::ctxt, a: &Opt, b: &Opt) -> bool {
     match (a, b) {
-        (&lit(UnitLikeStructLit(a)), &lit(UnitLikeStructLit(b))) => a == b,
         (&lit(a), &lit(b)) => {
             let a_expr = lit_to_expr(tcx, &a);
             let b_expr = lit_to_expr(tcx, &b);
@@ -310,11 +305,6 @@ fn trans_opt<'a>(bcx: &'a Block<'a>, o: &Opt) -> opt_result<'a> {
             let lit_datum = unpack_datum!(bcx, lit_datum.to_appropriate_datum(bcx));
             return single_result(Result::new(bcx, lit_datum.val));
         }
-        lit(UnitLikeStructLit(pat_id)) => {
-            let struct_ty = ty::node_id_to_type(bcx.tcx(), pat_id);
-            let datum = datum::rvalue_scratch_datum(bcx, struct_ty, "");
-            return single_result(Result::new(bcx, datum.val));
-        }
         lit(l @ ConstLit(ref def_id)) => {
             let lit_ty = ty::node_id_to_type(bcx.tcx(), lit_to_expr(bcx.tcx(), &l).id);
             let (llval, _) = consts::get_const_val(bcx.ccx(), *def_id);
@@ -347,9 +337,6 @@ fn variant_opt(bcx: &Block, pat_id: ast::NodeId) -> Opt {
             let variant = ty::enum_variant_with_id(ccx.tcx(), enum_id, var_id);
             var(variant.disr_val, adt::represent_node(bcx, pat_id), var_id)
         }
-        def::DefFn(..) | def::DefStruct(_) => {
-            lit(UnitLikeStructLit(pat_id))
-        }
         _ => {
             ccx.sess().bug("non-variant or struct in variant_opt()");
         }
@@ -567,7 +554,6 @@ fn enter_opt<'a, 'b>(
     let _indenter = indenter();
 
     let ctor = match opt {
-        &lit(UnitLikeStructLit(_)) => check_match::Single,
         &lit(x) => check_match::ConstantValue(const_eval::eval_const_expr(
             bcx.tcx(), lit_to_expr(bcx.tcx(), &x))),
         &range(ref lo, ref hi) => check_match::ConstantRange(
@@ -672,11 +658,10 @@ fn get_options(bcx: &Block, m: &[Match], col: uint) -> Vec<Opt> {
                 add_to_set(ccx.tcx(), &mut found, lit(ExprLit(l)));
             }
             ast::PatIdent(..) => {
-                // This is one of: an enum variant, a unit-like struct, or a
-                // variable binding.
+                // This is either an enum variant or a variable binding.
                 let opt_def = ccx.tcx.def_map.borrow().find_copy(&cur.id);
                 match opt_def {
-                    Some(def::DefVariant(..)) | Some(def::DefStruct(..)) => {
+                    Some(def::DefVariant(..)) => {
                         add_to_set(ccx.tcx(), &mut found,
                                    variant_opt(bcx, cur.id));
                     }
@@ -827,7 +812,7 @@ fn any_irrefutable_adt_pat(bcx: &Block, m: &[Match], col: uint) -> bool {
         let pat = *br.pats.get(col);
         match pat.node {
             ast::PatTup(_) => true,
-            ast::PatStruct(_, _, _) | ast::PatEnum(_, _) =>
+            ast::PatEnum(..) | ast::PatIdent(_, _, None) | ast::PatStruct(..) =>
                 match bcx.tcx().def_map.borrow().find(&pat.id) {
                     Some(&def::DefFn(..)) |
                     Some(&def::DefStruct(..)) => true,

src/test/run-pass/issue-12285.rs

@@ -0,0 +1,22 @@
+// 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.
+
+struct S;
+
+fn main() {
+    match Some(&S) {
+        Some(&S) => {},
+        _x => unreachable!()
+    }
+    match Some(&S) {
+        Some(&S) => {},
+        None => unreachable!()
+    }
+}