Add a range argument to vec.extract_if#133265
Conversation
This comment has been minimized.
This comment has been minimized.
| /// ``` | ||
| /// #![feature(extract_if)] | ||
| /// let mut items = vec![0, 0, 0, 0, 0, 0, 0, 1, 2, 1, 2, 1, 2]; | ||
| /// let ones = items.extract_if(7.., |x| *x == 1).collect::<Vec<_>>(); |
There was a problem hiding this comment.
Maybe change one of the first few 0s to 1 to indicate they don't get extracted (out of range)?
There was a problem hiding this comment.
Hrm, I wanted to show the "I have a prefix that I know doesn't contain those things, so I can skip them" use.
34ed582 to
0076504
Compare
This comment has been minimized.
This comment has been minimized.
0076504 to
d8ce784
Compare
This comment has been minimized.
This comment has been minimized.
5d0e0a7 to
aabed33
Compare
cuviper
left a comment
There was a problem hiding this comment.
LGTM.
The FCP finished -- do you want to go ahead and add the stabilization too?
|
r? @cuviper (sorry, I didn't realize this was assigned to me)
Since this is a pretty significant API and implementation change, maybe it would be good to hold off for a couple of weeks? In case anyone using this API unstably has feedback after the change (I doubt it, but doesn't hurt). |
|
Changing and stabilizing in the same PR doesn't seem ideal to me either, especially when the change happened parallel to the FCP. I think it should bake a few days at least. |
|
@bors r+ |
nk9
left a comment
There was a problem hiding this comment.
I hope this is the right place to raise this question about the sample code in this PR.
| /// ``` | ||
| /// # use std::cmp::min; | ||
| /// # let some_predicate = |x: &mut i32| { *x == 2 || *x == 3 || *x == 6 }; | ||
| /// # let mut vec = vec![1, 2, 3, 4, 5, 6]; |
There was a problem hiding this comment.
Apologies for the late comment, but I wanted to point out that this sample code doesn't behave the same way as the real extract_if(). Given the same predicate and range, if your vector were [1, 2, 3, 3, 3, 3, 3, 3, 4, 5, 6], then all of the 3s would be removed. i is only incremented when an element is dropped, but range.end is unchanged, so the items shift down. I got very confused when reading the docs and trying to square this sample code with the explanation of how the function works.
Fortunately, the real extract_if() does not have this problem. Proposed change to the sample code:
let some_predicate = |x: &mut i32| { *x == 2 || *x == 3 || *x == 5 };
let mut vec = vec![0, 1, 2, 3, 4, 5, 6];
let range = 1..5;
let mut i = range.start;
let mut end = range.end;
while i < min(vec.len(), end) {
if some_predicate(&mut vec[i]) {
let _val = vec.remove(i);
end -= 1;
// your code here
} else {
i += 1;
}
}
assert_eq!(vec, vec![0, 1, 4, 5, 6]);There was a problem hiding this comment.
If you want to propose a documentation improvement you can just open a PR.
tracking issue: #43244
This adds the range argument requested in #43244 (comment)