Skip to content

Commit

Permalink
Fix bug in matching struct patterns
Browse files Browse the repository at this point in the history
Code that collects fields in struct-like patterns used to ignore
wildcard patterns like `Foo{_}`. But `enter_defaults` considered
struct-like patterns as default in order to overcome this
(accoring to my understanding of situation).

However such behaviour caused code like this:
```
enum E {
    Foo{f: int},
    Bar
}
let e = Bar;
match e {
    Foo{f: _f} => { /* do something (1) */ }
    _ => { /* do something (2) */ }
}
```
consider pattern `Foo{f: _f}` as default. That caused inproper behaviour
and even segfaults while trying to destruct `Bar` as `Foo{f: _f}`.
Issues: #5625 , #5530.

This patch fixes `collect_record_or_struct_fields` to split cases of
single wildcard struct-like pattern and no struct-like pattern at all.
Former case resolved with `enter_rec_or_struct` (and not with
`enter_defaults`).

Closes #5625.
Closes #5530.
  • Loading branch information
dim-an committed Aug 6, 2013
1 parent 1710125 commit 0fadfc5
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 24 deletions.
55 changes: 35 additions & 20 deletions src/librustc/middle/trans/_match.rs
Expand Up @@ -485,7 +485,7 @@ fn enter_default<'r>(bcx: @mut Block,

do enter_match(bcx, dm, m, col, val) |p| {
match p.node {
ast::pat_wild | ast::pat_tup(_) | ast::pat_struct(*) => Some(~[]),
ast::pat_wild | ast::pat_tup(_) => Some(~[]),
ast::pat_ident(_, _, None) if pat_is_binding(dm, p) => Some(~[]),
_ => None
}
Expand Down Expand Up @@ -947,24 +947,37 @@ fn extract_vec_elems(bcx: @mut Block,
ExtractedBlock { vals: elems, bcx: bcx }
}

// NB: This function does not collect fields from struct-like enum variants.
/// Checks every pattern in `m` at `col` column.
/// If there are a struct pattern among them function
/// returns list of all fields that are matched in these patterns.
/// Function returns None if there is no struct pattern.
/// Function doesn't collect fields from struct-like enum variants.
/// Function can return empty list if there is only wildcard struct pattern.
fn collect_record_or_struct_fields(bcx: @mut Block,
m: &[Match],
col: uint)
-> ~[ast::ident] {
-> Option<~[ast::ident]> {
let mut fields: ~[ast::ident] = ~[];
let mut found = false;
for br in m.iter() {
match br.pats[col].node {
ast::pat_struct(_, ref fs, _) => {
match ty::get(node_id_type(bcx, br.pats[col].id)).sty {
ty::ty_struct(*) => extend(&mut fields, *fs),
ty::ty_struct(*) => {
extend(&mut fields, *fs);
found = true;
}
_ => ()
}
}
_ => ()
}
}
return fields;
if found {
return Some(fields);
} else {
return None;
}

fn extend(idents: &mut ~[ast::ident], field_pats: &[ast::field_pat]) {
for field_pat in field_pats.iter() {
Expand Down Expand Up @@ -1336,22 +1349,24 @@ fn compile_submatch_continue(mut bcx: @mut Block,
// required to root any values.
assert!(any_box_pat(m, col) || !pats_require_rooting(bcx, m, col));

let rec_fields = collect_record_or_struct_fields(bcx, m, col);
if rec_fields.len() > 0 {
let pat_ty = node_id_type(bcx, pat_id);
let pat_repr = adt::represent_type(bcx.ccx(), pat_ty);
do expr::with_field_tys(tcx, pat_ty, None) |discr, field_tys| {
let rec_vals = rec_fields.map(|field_name| {
let ix = ty::field_idx_strict(tcx, *field_name, field_tys);
adt::trans_field_ptr(bcx, pat_repr, val, discr, ix)
});
compile_submatch(
bcx,
enter_rec_or_struct(bcx, dm, m, col, rec_fields, val),
vec::append(rec_vals, vals_left),
chk);
match collect_record_or_struct_fields(bcx, m, col) {
Some(ref rec_fields) => {
let pat_ty = node_id_type(bcx, pat_id);
let pat_repr = adt::represent_type(bcx.ccx(), pat_ty);
do expr::with_field_tys(tcx, pat_ty, None) |discr, field_tys| {
let rec_vals = rec_fields.map(|field_name| {
let ix = ty::field_idx_strict(tcx, *field_name, field_tys);
adt::trans_field_ptr(bcx, pat_repr, val, discr, ix)
});
compile_submatch(
bcx,
enter_rec_or_struct(bcx, dm, m, col, *rec_fields, val),
vec::append(rec_vals, vals_left),
chk);
}
return;
}
return;
None => {}
}

if any_tup_pat(m, col) {
Expand Down
2 changes: 0 additions & 2 deletions src/test/run-pass/issue-5530.rs
Expand Up @@ -8,8 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// xfail-test

enum Enum {
Foo { foo: uint },
Bar { bar: uint }
Expand Down
2 changes: 0 additions & 2 deletions src/test/run-pass/match-enum-struct-0.rs
Expand Up @@ -8,8 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// xfail-test

// regression test for issue #5625

enum E {
Expand Down

5 comments on commit 0fadfc5

@bors
Copy link
Contributor

@bors bors commented on 0fadfc5 Aug 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 0fadfc5 Aug 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging dim-an/rust/fix-struct-match = 0fadfc5 into auto

@bors
Copy link
Contributor

@bors bors commented on 0fadfc5 Aug 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dim-an/rust/fix-struct-match = 0fadfc5 merged ok, testing candidate = a931e04

@bors
Copy link
Contributor

@bors bors commented on 0fadfc5 Aug 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = a931e04

Please sign in to comment.