Skip to content
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

Add support for destructuring vectors in match expressions #4143

Closed
wants to merge 5 commits into from
Closed

Add support for destructuring vectors in match expressions #4143

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 8, 2012

Resubmitting #4091 as I cannot seem to be able to change the target branch in the original PR. I squashed the commits as there isn't much of a logical separation possible and some of them were in half-broken state.

It'd be great if we could get this in for 0.5 if it's not too late. I'm stil considering @erickt's [a, ..b, c] proposal and wonder what other people think? As we're rejecting that syntax now, we can still be forward compatible with just the [a, ..tail] variant, though, so this could be added later on.

@brson
Copy link
Contributor

brson commented Dec 11, 2012

This is a very nice looking patch.

@brson
Copy link
Contributor

brson commented Dec 11, 2012

Since this was written, the default binding mode changed from the old implicit by-ref to copy, so I hope all these test cases are still valid (if not they can be changed to ref. It would be nice to have test cases using all the binding types: ref, copy and move.

@ghost
Copy link
Author

ghost commented Dec 11, 2012

@brson The tests are still passing except for ./compile-fail/alt-vec-illegal-tail-loan.rs due to what seems to be #3243. It should probably be xfailed for now unless it's actually an oversight in this patch...

@graydon
Copy link
Contributor

graydon commented Dec 11, 2012

I'll merge this presently, just wanted to check .. this is substantial enough that we should probably have a contributor agreement. Have you filled one of those out?

@ghost
Copy link
Author

ghost commented Dec 12, 2012

@graydon No, I have not... Is this the one you have in mind: http://www.mozilla.org/hacking/committer/? I'll take a look.

@ghost
Copy link
Author

ghost commented Dec 12, 2012

@graydon I submitted the agreement via e-mail today.

@ghost
Copy link
Author

ghost commented Dec 15, 2012

I rebased and made some more changes to make the exhaustiveness/reachability checks cleaner.

The lifetime problems with destructured tails are clearly an oversight on my part, I hadn't looked into the memory categorization pass at all.

@graydon
Copy link
Contributor

graydon commented Dec 18, 2012

Excellent. Contributor forms received, landed this in 6c83fe4 , thanks so much!

@graydon graydon closed this Dec 18, 2012
@ghost ghost deleted the vector-destructuring-2 branch March 9, 2014 14:04
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 25, 2021
Rework use_self impl based on ty::Ty comparison rust-lang#3410 | Take 2

This builds on top of rust-lang#5531

I already reviewed and approved the commits by `@montrivo.` So only the review of my commits should be necessary.

I would also appreciate your review `@montrivo,` since you are familiar with the challenges here.

Fixes rust-lang#3410 and Fixes rust-lang#4143 (same problem)
Fixes rust-lang#2843
Fixes rust-lang#3859
Fixes rust-lang#4734 and fixes rust-lang#6221
Fixes rust-lang#4305
Fixes rust-lang#5078 (even at expression level now 🎉)
Fixes rust-lang#3881 and Fixes rust-lang#4887 (same problem)
Fixes rust-lang#3909

Not yet: rust-lang#4140 (test added)

All the credit for the fixes goes to `@montrivo.` I only refactored and copy and pasted his code.

changelog: rewrite [`use_self`] lint and fix multiple (8) FPs. One to go.
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.

None yet

2 participants