Skip to content

Issue 4635 #4748

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

Closed
wants to merge 2 commits into from
Closed

Issue 4635 #4748

wants to merge 2 commits into from

Conversation

sanxiyn
Copy link
Member

@sanxiyn sanxiyn commented Feb 1, 2013

Fix for #4635.
check_match.rs needs to be updated, but I couldn't figure out how. Tests are xfailed.
Code generation seems to work. Added a test vec-matching-fold.rs.

First commit is pure rename, since tail is no more appropriate.

pat_vec(~[@pat], Option<@pat>) was changed to pat_vec(~[@pat], Option<uint>).

Before:
[a] parses to pat_vec(~[a], None)
[a, ..b] parses to pat_vec(~[a], Some(b))

After:
[a] parses to pat_vec(~[a], None)
[a, ..b] parses to pat_vec(~[a, b], Some(1))
[..a, b] parses to pat_vec(~[a, b], Some(0))
[a, ..b, c] parses to pat_vec(~[a, b, c], Some(1))

@brson
Copy link
Contributor

brson commented Feb 5, 2013

maybe @nikomatsakis can help. I am not familiar with this code.

@sanxiyn
Copy link
Member Author

sanxiyn commented Feb 12, 2013

Rebased.

@nikomatsakis
Copy link
Contributor

Note: r- on 2875079 pending more tests being added / tests being re-enabled.

Also, a question to @sanxiyn : Did you try setting up pat_vec like so:

pat_vec(before_pats, slice_pat, after_pats)

rather than using an index? That would have been my first guess. I'm not sure if the code would wind up cleaner, messier, or more of the same, but it seems potentially cleaner to me overall, since the role that these patterns play is quite different. But the current setup is also ok if you think it works out better this way.

@catamorphism
Copy link
Contributor

I'm closing this due to lack of reply to @nikomatsakis 's questions -- but, @sanxiyn , please open a new pull request when you have time to address these issues! Thanks!

bors added a commit that referenced this pull request Mar 11, 2013
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.

4 participants