Skip to content

Commit 67cc89f

Browse files
marijnhmarijnh
committed
Rewrite exhaustiveness checker
Issue #352 Closes #1720 The old checker would happily accept things like 'alt x { @some(a) { a } }'. It now properly descends into patterns, checks exhaustiveness of booleans, and complains when number/string patterns aren't exhaustive.
1 parent 4b63826 commit 67cc89f

32 files changed

+192
-124
lines changed

‎src/comp/metadata/decoder.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ fn lookup_def(cnum: ast::crate_num, data: @[u8], did_: ast::def_id) ->
218218
let fam_ch = item_family(item);
219219
let did = {crate: cnum, node: did_.node};
220220
// We treat references to enums as references to types.
221-
alt fam_ch {
221+
alt check fam_ch {
222222
'c' { ast::def_const(did) }
223223
'u' { ast::def_fn(did, ast::unsafe_fn) }
224224
'f' { ast::def_fn(did, ast::impure_fn) }
@@ -336,7 +336,7 @@ fn get_iface_methods(cdata: cmd, id: ast::node_id, tcx: ty::ctxt)
336336
_ { tcx.sess.bug("get_iface_methods: id has non-function type");
337337
} };
338338
result += [{ident: name, tps: bounds, fty: fty,
339-
purity: alt item_family(mth) {
339+
purity: alt check item_family(mth) {
340340
'u' { ast::unsafe_fn }
341341
'f' { ast::impure_fn }
342342
'p' { ast::pure_fn }
@@ -346,7 +346,7 @@ fn get_iface_methods(cdata: cmd, id: ast::node_id, tcx: ty::ctxt)
346346
}
347347

348348
fn family_has_type_params(fam_ch: char) -> bool {
349-
alt fam_ch {
349+
alt check fam_ch {
350350
'c' | 'T' | 'm' | 'n' { false }
351351
'f' | 'u' | 'p' | 'F' | 'U' | 'P' | 'y' | 't' | 'v' | 'i' | 'I' { true }
352352
}
@@ -370,7 +370,7 @@ fn describe_def(items: ebml::doc, id: ast::def_id) -> str {
370370
}
371371

372372
fn item_family_to_str(fam: char) -> str {
373-
alt fam {
373+
alt check fam {
374374
'c' { ret "const"; }
375375
'f' { ret "fn"; }
376376
'u' { ret "unsafe fn"; }

‎src/comp/metadata/tydecode.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,15 +177,15 @@ fn parse_proto(c: char) -> ast::proto {
177177
}
178178

179179
fn parse_ty(st: @pstate, conv: conv_did) -> ty::t {
180-
alt next(st) {
180+
alt check next(st) {
181181
'n' { ret ty::mk_nil(st.tcx); }
182182
'z' { ret ty::mk_bot(st.tcx); }
183183
'b' { ret ty::mk_bool(st.tcx); }
184184
'i' { ret ty::mk_int(st.tcx); }
185185
'u' { ret ty::mk_uint(st.tcx); }
186186
'l' { ret ty::mk_float(st.tcx); }
187187
'M' {
188-
alt next(st) {
188+
alt check next(st) {
189189
'b' { ret ty::mk_mach_uint(st.tcx, ast::ty_u8); }
190190
'w' { ret ty::mk_mach_uint(st.tcx, ast::ty_u16); }
191191
'l' { ret ty::mk_mach_uint(st.tcx, ast::ty_u32); }
@@ -269,7 +269,7 @@ fn parse_ty(st: @pstate, conv: conv_did) -> ty::t {
269269
'Y' { ret ty::mk_type(st.tcx); }
270270
'y' { ret ty::mk_send_type(st.tcx); }
271271
'C' {
272-
let ck = alt next(st) {
272+
let ck = alt check next(st) {
273273
'&' { ty::ck_block }
274274
'@' { ty::ck_box }
275275
'~' { ty::ck_uniq }
@@ -355,7 +355,7 @@ fn parse_ty_fn(st: @pstate, conv: conv_did) -> ty::fn_ty {
355355
assert (next(st) == '[');
356356
let inputs: [ty::arg] = [];
357357
while peek(st) != ']' {
358-
let mode = alt peek(st) {
358+
let mode = alt check peek(st) {
359359
'&' { ast::by_mut_ref }
360360
'-' { ast::by_move }
361361
'+' { ast::by_copy }
@@ -405,7 +405,7 @@ fn parse_bounds_data(data: @[u8], start: uint,
405405
fn parse_bounds(st: @pstate, conv: conv_did) -> @[ty::param_bound] {
406406
let bounds = [];
407407
while true {
408-
bounds += [alt next(st) {
408+
bounds += [alt check next(st) {
409409
'S' { ty::bound_send }
410410
'C' { ty::bound_copy }
411411
'I' { ty::bound_iface(parse_ty(st, conv)) }

‎src/comp/middle/check_alt.rs

Lines changed: 118 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ fn check_expr(tcx: ty::ctxt, ex: @expr, &&s: (), v: visit::vt<()>) {
2727
/* Check for exhaustiveness */
2828
if mode == alt_exhaustive {
2929
let arms = vec::concat(vec::filter_map(arms, unguarded_pat));
30-
check_exhaustive(tcx, ex.span, expr_ty(tcx, scrut), arms);
30+
check_exhaustive(tcx, ex.span, arms);
3131
}
3232
}
3333
_ { }
@@ -59,89 +59,136 @@ fn check_arms(tcx: ty::ctxt, arms: [arm]) {
5959
}
6060
}
6161

62+
fn raw_pat(p: @pat) -> @pat {
63+
alt p.node {
64+
pat_ident(_, some(s)) { raw_pat(s) }
65+
_ { p }
66+
}
67+
}
68+
6269
// Precondition: patterns have been normalized
6370
// (not checked statically yet)
64-
fn check_exhaustive(tcx: ty::ctxt, sp:span, scrut_ty:ty::t, pats:[@pat]) {
65-
let represented : [def_id] = [];
66-
/* Determine the type of the scrutinee */
67-
/* If it's not an enum, exit (bailing out on checking non-enum alts
68-
for now) */
69-
/* Otherwise, get the list of variants and make sure each one is
70-
represented. Then recurse on the columns. */
71-
72-
let ty_def_id = alt ty::get(scrut_ty).struct {
73-
ty_enum(id, _) { id }
74-
_ { ret; } };
71+
fn check_exhaustive(tcx: ty::ctxt, sp: span, pats: [@pat]) {
72+
if pats.len() == 0u {
73+
tcx.sess.span_err(sp, "non-exhaustive patterns");
74+
ret;
75+
}
76+
// If there a non-refutable pattern in the set, we're okay.
77+
for pat in pats { if !is_refutable(tcx, pat) { ret; } }
7578

76-
let variants = *enum_variants(tcx, ty_def_id);
77-
for pat in pats {
78-
if !is_refutable(tcx, pat) {
79-
/* automatically makes this alt complete */ ret;
79+
alt ty::get(ty::node_id_to_type(tcx, pats[0].id)).struct {
80+
ty::ty_enum(id, _) {
81+
check_exhaustive_enum(tcx, id, sp, pats);
82+
}
83+
ty::ty_box(_) {
84+
check_exhaustive(tcx, sp, vec::filter_map(pats, {|p|
85+
alt raw_pat(p).node { pat_box(sub) { some(sub) } _ { none } }
86+
}));
87+
}
88+
ty::ty_uniq(_) {
89+
check_exhaustive(tcx, sp, vec::filter_map(pats, {|p|
90+
alt raw_pat(p).node { pat_uniq(sub) { some(sub) } _ { none } }
91+
}));
92+
}
93+
ty::ty_tup(ts) {
94+
let cols = vec::init_elt_mut(ts.len(), []);
95+
for p in pats {
96+
alt raw_pat(p).node {
97+
pat_tup(sub) {
98+
vec::iteri(sub) {|i, sp| cols[i] += [sp];}
99+
}
100+
_ {}
101+
}
80102
}
81-
alt pat.node {
82-
// want the def_id for the constructor
83-
pat_enum(id,_) {
84-
alt tcx.def_map.find(pat.id) {
85-
some(def_variant(_, variant_def_id)) {
86-
represented += [variant_def_id];
103+
vec::iter(cols) {|col| check_exhaustive(tcx, sp, col); }
104+
}
105+
ty::ty_rec(fs) {
106+
let cols = vec::init_elt(fs.len(), {mutable wild: false,
107+
mutable pats: []});
108+
for p in pats {
109+
alt raw_pat(p).node {
110+
pat_rec(sub, _) {
111+
vec::iteri(fs) {|i, field|
112+
alt vec::find(sub, {|pf| pf.ident == field.ident }) {
113+
some(pf) { cols[i].pats += [pf.pat]; }
114+
none { cols[i].wild = true; }
87115
}
88-
_ { tcx.sess.span_bug(pat.span, "check_exhaustive:
89-
pat_tag not bound to a variant"); }
90116
}
117+
}
118+
_ {}
119+
}
120+
}
121+
vec::iter(cols) {|col|
122+
if !col.wild { check_exhaustive(tcx, sp, copy col.pats); }
123+
}
124+
}
125+
ty::ty_bool {
126+
let saw_true = false, saw_false = false;
127+
for p in pats {
128+
alt raw_pat(p).node {
129+
pat_lit(@{node: expr_lit(@{node: lit_bool(b), _}), _}) {
130+
if b { saw_true = true; }
131+
else { saw_false = true; }
132+
}
133+
_ {}
91134
}
92-
_ { tcx.sess.span_bug(pat.span, "check_exhaustive: ill-typed \
93-
pattern"); // we know this has enum type,
94-
} // so anything else should be impossible
95-
}
96-
}
97-
fn not_represented(v: [def_id], &&vinfo: variant_info) -> bool {
98-
!vec::contains(v, vinfo.id)
99-
}
100-
// Could be more efficient (bitvectors?)
101-
alt vec::find(variants, bind not_represented(represented,_)) {
102-
some(bad) {
103-
// complain
104-
// TODO: give examples of cases that aren't covered
105-
tcx.sess.note("Patterns not covered include:");
106-
tcx.sess.note(bad.name);
107-
tcx.sess.span_err(sp, "Non-exhaustive pattern");
108135
}
109-
_ {}
136+
if !saw_true { tcx.sess.span_err(
137+
sp, "non-exhaustive bool patterns: true not covered"); }
138+
if !saw_false { tcx.sess.span_err(
139+
sp, "non-exhaustive bool patterns: false not covered"); }
140+
}
141+
ty::ty_nil {
142+
let seen = vec::any(pats, {|p|
143+
alt raw_pat(p).node {
144+
pat_lit(@{node: expr_lit(@{node: lit_nil, _}), _}) { true }
145+
_ { false }
146+
}
147+
});
148+
if !seen { tcx.sess.span_err(sp, "non-exhaustive patterns"); }
149+
}
150+
// Literal patterns are always considered non-exhaustive
151+
_ {
152+
tcx.sess.span_err(sp, "non-exhaustive literal patterns");
153+
}
110154
}
111-
// Otherwise, check subpatterns
112-
// inefficient
113-
for variant in variants {
114-
// rows consists of the argument list for each pat that's an enum
115-
let rows : [[@pat]] = [];
116-
for pat in pats {
155+
}
156+
157+
fn check_exhaustive_enum(tcx: ty::ctxt, enum_id: def_id, sp: span,
158+
pats: [@pat]) {
159+
let variants = enum_variants(tcx, enum_id);
160+
let columns_by_variant = vec::map(*variants, {|v|
161+
{mutable seen: false,
162+
cols: vec::init_elt_mut(v.args.len(), [])}
163+
});
164+
165+
for pat in pats {
166+
let pat = raw_pat(pat);
167+
alt tcx.def_map.get(pat.id) {
168+
def_variant(_, id) {
169+
let variant_idx =
170+
option::get(vec::position(*variants, {|v| v.id == id}));
171+
columns_by_variant[variant_idx].seen = true;
117172
alt pat.node {
118-
pat_enum(id, args) {
119-
alt tcx.def_map.find(pat.id) {
120-
some(def_variant(_,variant_id))
121-
if variant_id == variant.id { rows += [args]; }
122-
_ { }
123-
}
124-
}
125-
_ {}
173+
pat_enum(_, args) {
174+
vec::iteri(args) {|i, p|
175+
columns_by_variant[variant_idx].cols[i] += [p];
176+
}
177+
}
178+
_ {}
126179
}
180+
}
181+
_ {}
127182
}
128-
if check vec::is_not_empty(rows) {
129-
let i = 0u;
130-
for it in rows[0] {
131-
let column = [it];
132-
// Annoying -- see comment in
133-
// tstate::states::find_pre_post_state_loop
134-
check vec::is_not_empty(rows);
135-
for row in vec::tail(rows) {
136-
column += [row[i]];
137-
}
138-
check_exhaustive(tcx, sp, pat_ty(tcx, it), column);
139-
i += 1u;
140-
}
183+
}
184+
185+
vec::iteri(columns_by_variant) {|i, cv|
186+
if !cv.seen {
187+
tcx.sess.span_err(sp, "non-exhaustive patterns: variant `" +
188+
variants[i].name + "` not covered");
189+
} else {
190+
vec::iter(cv.cols) {|col| check_exhaustive(tcx, sp, col); }
141191
}
142-
// This shouldn't actually happen, since there were no
143-
// irrefutable patterns if we got here.
144-
else { cont; }
145192
}
146193
}
147194

‎src/comp/middle/lint.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ fn time(do_it: bool, what: str, thunk: fn()) {
4040
fn merge_opts(attrs: [ast::attribute], cmd_opts: [(option, bool)]) ->
4141
[(option, bool)] {
4242
fn str_to_option(name: str) -> (option, bool) {
43-
ret alt name {
43+
ret alt check name {
4444
"ctypes" { (ctypes, true) }
4545
"no_ctypes" { (ctypes, false) }
4646
}

‎src/comp/syntax/print/pprust.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1023,7 +1023,7 @@ fn print_expr(s: ps, &&expr: @ast::expr) {
10231023
}
10241024
ast::expr_be(result) { word_nbsp(s, "be"); print_expr(s, result); }
10251025
ast::expr_log(lvl, lexp, expr) {
1026-
alt lvl {
1026+
alt check lvl {
10271027
1 { word_nbsp(s, "log"); print_expr(s, expr); }
10281028
0 { word_nbsp(s, "log_err"); print_expr(s, expr); }
10291029
2 {

‎src/compiletest/procsrv.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ fn run(lib_path: str, prog: str, args: [str],
6868
let count = 2;
6969
while count > 0 {
7070
let stream = comm::recv(p);
71-
alt stream {
71+
alt check stream {
7272
(1, s) {
7373
outs = s;
7474
}

‎src/libcore/bool.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,10 @@ pure fn is_false(v: t) -> bool { !v }
6060
brief = "Parse logic value from `s`"
6161
)]
6262
pure fn from_str(s: str) -> t {
63-
alt s {
63+
alt check s {
6464
"true" { true }
6565
"false" { false }
66+
_ { fail "'" + s + "' is not a valid boolean string"; }
6667
}
6768
}
6869

‎src/libcore/str.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2140,7 +2140,7 @@ mod tests {
21402140
fn test_chars_iter() {
21412141
let i = 0;
21422142
chars_iter("x\u03c0y") {|ch|
2143-
alt i {
2143+
alt check i {
21442144
0 { assert ch == 'x'; }
21452145
1 { assert ch == '\u03c0'; }
21462146
2 { assert ch == 'y'; }
@@ -2156,7 +2156,7 @@ mod tests {
21562156
let i = 0;
21572157

21582158
bytes_iter("xyz") {|bb|
2159-
alt i {
2159+
alt check i {
21602160
0 { assert bb == 'x' as u8; }
21612161
1 { assert bb == 'y' as u8; }
21622162
2 { assert bb == 'z' as u8; }

‎src/libstd/four.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ Function: from_str
166166
Parse logic value from `s`
167167
*/
168168
pure fn from_str(s: str) -> t {
169-
alt s {
169+
alt check s {
170170
"none" { none }
171171
"false" { four::false }
172172
"true" { four::true }
@@ -181,7 +181,7 @@ Convert `v` into a string
181181
*/
182182
pure fn to_str(v: t) -> str {
183183
// FIXME replace with consts as soon as that works
184-
alt v {
184+
alt check v {
185185
0u8 { "none" }
186186
1u8 { "true" }
187187
2u8 { "false" }
@@ -265,7 +265,7 @@ mod tests {
265265
}
266266

267267
fn to_tup(v: four::t) -> (bool, bool) {
268-
alt v {
268+
alt check v {
269269
0u8 { (false, false) }
270270
1u8 { (false, true) }
271271
2u8 { (true, false) }

‎src/libstd/serialization.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ fn test_option_int() {
241241
fn deserialize_0<S: deserializer>(s: S) -> option<int> {
242242
s.read_enum("option") {||
243243
s.read_enum_variant {|i|
244-
alt i {
244+
alt check i {
245245
0u { none }
246246
1u {
247247
let v0 = s.read_enum_variant_arg(0u) {||

0 commit comments

Comments
 (0)