Skip to content

Commit 0c48c57

Browse files
author
Jakub Wieczorek
committed
Never expand specialized columns that only contain wild patterns in them
Doing so would incur deeply nested expansion of the tree with no useful side effects. This is problematic for "wide" data types such as structs with dozens of fields but where only a few are actually being matched or bound. Most notably, matching a fixed slice would use a number of stack frames that grows with the number of elements in the slice. Fixes #17877.
1 parent 86509d8 commit 0c48c57

File tree

4 files changed

+107
-62
lines changed

4 files changed

+107
-62
lines changed

src/librustc/middle/check_match.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -638,15 +638,15 @@ fn pat_constructors(cx: &MatchCheckCtxt, p: &Pat,
638638
PatEnum(..) =>
639639
match cx.tcx.def_map.borrow().find(&pat.id) {
640640
Some(&DefConst(..)) =>
641-
cx.tcx.sess.span_bug(pat.span, "static pattern should've \
641+
cx.tcx.sess.span_bug(pat.span, "const pattern should've \
642642
been rewritten"),
643643
Some(&DefVariant(_, id, _)) => vec!(Variant(id)),
644644
_ => vec!(Single)
645645
},
646646
PatStruct(..) =>
647647
match cx.tcx.def_map.borrow().find(&pat.id) {
648648
Some(&DefConst(..)) =>
649-
cx.tcx.sess.span_bug(pat.span, "static pattern should've \
649+
cx.tcx.sess.span_bug(pat.span, "const pattern should've \
650650
been rewritten"),
651651
Some(&DefVariant(_, id, _)) => vec!(Variant(id)),
652652
_ => vec!(Single)

src/librustc/middle/pat_util.rs

+15-1
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,25 @@ pub type PatIdMap = HashMap<Ident, NodeId>;
2525
pub fn pat_id_map(dm: &resolve::DefMap, pat: &Pat) -> PatIdMap {
2626
let mut map = HashMap::new();
2727
pat_bindings(dm, pat, |_bm, p_id, _s, path1| {
28-
map.insert(path1.node, p_id);
28+
map.insert(path1.node, p_id);
2929
});
3030
map
3131
}
3232

33+
pub fn pat_is_refutable(dm: &resolve::DefMap, pat: &Pat) -> bool {
34+
match pat.node {
35+
PatLit(_) | PatRange(_, _) => true,
36+
PatEnum(_, _) | PatIdent(_, _, None) | PatStruct(..) => {
37+
match dm.borrow().find(&pat.id) {
38+
Some(&DefVariant(..)) => true,
39+
_ => false
40+
}
41+
}
42+
PatVec(_, _, _) => true,
43+
_ => false
44+
}
45+
}
46+
3347
pub fn pat_is_variant_or_struct(dm: &resolve::DefMap, pat: &Pat) -> bool {
3448
match pat.node {
3549
PatEnum(_, _) | PatIdent(_, _, None) | PatStruct(..) => {

src/librustc/middle/trans/_match.rs

+69-59
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ use util::ppaux::{Repr, vec_map_to_string};
218218

219219
use std;
220220
use std::collections::HashMap;
221+
use std::iter::AdditiveIterator;
221222
use std::rc::Rc;
222223
use syntax::ast;
223224
use syntax::ast::{DUMMY_NODE_ID, Ident};
@@ -754,33 +755,41 @@ impl FailureHandler {
754755
}
755756
}
756757

757-
fn pick_col(m: &[Match]) -> uint {
758-
fn score(p: &ast::Pat) -> uint {
759-
match p.node {
760-
ast::PatLit(_) | ast::PatEnum(_, _) | ast::PatRange(_, _) => 1u,
761-
ast::PatIdent(_, _, Some(ref p)) => score(&**p),
762-
_ => 0u
758+
fn pick_column_to_specialize(def_map: &DefMap, m: &[Match]) -> Option<uint> {
759+
fn pat_score(def_map: &DefMap, pat: &ast::Pat) -> uint {
760+
match pat.node {
761+
ast::PatIdent(_, _, Some(ref inner)) => pat_score(def_map, &**inner),
762+
_ if pat_is_refutable(def_map, pat) => 1u,
763+
_ => 0u
763764
}
764765
}
765-
let mut scores = Vec::from_elem(m[0].pats.len(), 0u);
766-
for br in m.iter() {
767-
for (i, ref p) in br.pats.iter().enumerate() {
768-
*scores.get_mut(i) += score(&***p);
766+
767+
let column_score: |&[Match], uint| -> uint = |m, col| {
768+
let total_score = m.iter()
769+
.map(|row| row.pats[col])
770+
.map(|pat| pat_score(def_map, pat))
771+
.sum();
772+
773+
// Irrefutable columns always go first, they'd only be duplicated in the branches.
774+
if total_score == 0 {
775+
std::uint::MAX
776+
} else {
777+
total_score
769778
}
770-
}
771-
let mut max_score = 0u;
772-
let mut best_col = 0u;
773-
for (i, score) in scores.iter().enumerate() {
774-
let score = *score;
775-
776-
// Irrefutable columns always go first, they'd only be duplicated in
777-
// the branches.
778-
if score == 0u { return i; }
779-
// If no irrefutable ones are found, we pick the one with the biggest
780-
// branching factor.
781-
if score > max_score { max_score = score; best_col = i; }
782-
}
783-
return best_col;
779+
};
780+
781+
let column_contains_any_nonwild_patterns: |&uint| -> bool = |&col| {
782+
m.iter().any(|row| match row.pats[col].node {
783+
ast::PatWild(_) => false,
784+
_ => true
785+
})
786+
};
787+
788+
range(0, m[0].pats.len())
789+
.filter(column_contains_any_nonwild_patterns)
790+
.map(|col| (col, column_score(m, col)))
791+
.max_by(|&(_, score)| score)
792+
.map(|(col, _)| col)
784793
}
785794

786795
// Compiles a comparison between two things.
@@ -951,44 +960,45 @@ fn compile_submatch<'a, 'p, 'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
951960
return;
952961
}
953962

954-
let col_count = m[0].pats.len();
955-
if col_count == 0u {
956-
let data = &m[0].data;
957-
for &(ref ident, ref value_ptr) in m[0].bound_ptrs.iter() {
958-
let llmatch = data.bindings_map.get(ident).llmatch;
959-
call_lifetime_start(bcx, llmatch);
960-
Store(bcx, *value_ptr, llmatch);
963+
let tcx = bcx.tcx();
964+
let def_map = &tcx.def_map;
965+
match pick_column_to_specialize(def_map, m) {
966+
Some(col) => {
967+
let val = vals[col];
968+
if has_nested_bindings(m, col) {
969+
let expanded = expand_nested_bindings(bcx, m, col, val);
970+
compile_submatch_continue(bcx,
971+
expanded.as_slice(),
972+
vals,
973+
chk,
974+
col,
975+
val,
976+
has_genuine_default)
977+
} else {
978+
compile_submatch_continue(bcx, m, vals, chk, col, val, has_genuine_default)
979+
}
961980
}
962-
match data.arm.guard {
963-
Some(ref guard_expr) => {
964-
bcx = compile_guard(bcx,
965-
&**guard_expr,
966-
m[0].data,
967-
m[1..m.len()],
968-
vals,
969-
chk,
970-
has_genuine_default);
981+
None => {
982+
let data = &m[0].data;
983+
for &(ref ident, ref value_ptr) in m[0].bound_ptrs.iter() {
984+
let llmatch = data.bindings_map.get(ident).llmatch;
985+
call_lifetime_start(bcx, llmatch);
986+
Store(bcx, *value_ptr, llmatch);
971987
}
972-
_ => ()
988+
match data.arm.guard {
989+
Some(ref guard_expr) => {
990+
bcx = compile_guard(bcx,
991+
&**guard_expr,
992+
m[0].data,
993+
m[1..m.len()],
994+
vals,
995+
chk,
996+
has_genuine_default);
997+
}
998+
_ => ()
999+
}
1000+
Br(bcx, data.bodycx.llbb);
9731001
}
974-
Br(bcx, data.bodycx.llbb);
975-
return;
976-
}
977-
978-
let col = pick_col(m);
979-
let val = vals[col];
980-
981-
if has_nested_bindings(m, col) {
982-
let expanded = expand_nested_bindings(bcx, m, col, val);
983-
compile_submatch_continue(bcx,
984-
expanded.as_slice(),
985-
vals,
986-
chk,
987-
col,
988-
val,
989-
has_genuine_default)
990-
} else {
991-
compile_submatch_continue(bcx, m, vals, chk, col, val, has_genuine_default)
9921002
}
9931003
}
9941004

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

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
fn main() {
12+
assert_eq!(match [0u8, ..1024] {
13+
_ => 42u,
14+
}, 42u);
15+
16+
assert_eq!(match [0u8, ..1024] {
17+
[1, _..] => 0u,
18+
[0, _..] => 1u,
19+
_ => 2u
20+
}, 1u);
21+
}

0 commit comments

Comments
 (0)