Skip to content

Stop the expansion of decision trees for fixed slice patterns as soon as possible #17944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from Oct 13, 2014
Merged

Conversation

ghost
Copy link

@ghost ghost commented Oct 11, 2014

Fixes #17877.

@ghost ghost changed the title Stop the expansion of decision trees for slice patterns as soon as possible Stop the expansion of decision trees for fixed slice patterns as soon as possible Oct 11, 2014
@ghost
Copy link
Author

ghost commented Oct 11, 2014

cc @chris-morgan @pnkfelix

@chris-morgan
Copy link
Member

So something that actually did use a long array match pattern would still crash? I wouldn’t call this a complete fix if that’s still the case, though it’ll fix every serious case. But being able to cause stack overflow in the compiler at all from that is bad.

@zwarich
Copy link

zwarich commented Oct 11, 2014

Since Rust has bounded stacks, you're not going to be able to eliminate all stack overflows from the compiler unless the compiler never uses recursion bounded by its input.

@chris-morgan
Copy link
Member

This is not the sort of thing that I at least would expect to be recursive. I can understand nested things becoming recursion, but an array? Not so much.

@ghost
Copy link
Author

ghost commented Oct 11, 2014

@chris-morgan Well, what you're describing is not really specific to slices. Tuples, structs and variants also incur recursion of depth that grows with the number of elements/fields and fixed-length slices are really just a homogenous kind of tuples.

match expressions get compiled to a tree and recursion is the most natural way of building said trees. There's certainly room for optimisation but I think the recursive nature of trans/_match.rs is most likely to stay.

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.
@ghost
Copy link
Author

ghost commented Oct 12, 2014

@alexcrichton I pushed a more general fix which will improve stack usage for all scenarios where you match a "wide" data type (a struct with several fields or a n-ary tuple with a high n) but only compare/bind a small subset of them.

bors added a commit that referenced this pull request Oct 12, 2014
@bors bors closed this Oct 13, 2014
@bors bors merged commit 0c48c57 into rust-lang:master Oct 13, 2014
@ghost ghost deleted the issue-17877 branch October 16, 2014 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiling a match on [T, ..N] uses O(N) stack space in rustc, leading to stack overflow
3 participants