Implement retain for VecDeque #24879
Conversation
|
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
| /// buf.retain(|&x| x%2 == 0); | ||
| /// assert_eq!(buf, [2, 4]); | ||
| /// ``` | ||
| #[stable(feature = "rust1", since = "1.0.0")] |
There was a problem hiding this comment.
At this point anything to be included in 1.0 needs to be cherry-picked backwards, and this isn't necessarily a serious enough change to be cherry-picked backwards, so if we're going to insta-stabilize it this should be since = "1.1.0" with a unique feature name like vec_deque_retain.
| #[test] | ||
| fn test_retain() { | ||
| let mut buf = VecDeque::new(); | ||
| buf.extend(1..4); |
There was a problem hiding this comment.
I think this test will also need to be updated for 1..5 instead of 1..4
There was a problem hiding this comment.
That's bad. I ran the tests (make -j4 check) and they passed...
There was a problem hiding this comment.
An unrelated test was failing causing the tests to abort early. However, the abort was lost well past my scrollback buffer and the old test logs were still there so I figured that my test passed...
|
@alexcrichton I don't have a strong feeling re: stabilization, but given that there's not a lot of urgency here, I would lean more toward landing as unstable and then including in a later stabilization pass, per standard policy. |
|
I thought this was delayed/blocked on proper predicate support? |
|
|
@gankro I think we ended up abandoning generic predicates due to coherence problems, but I don't know the precise details. @Stebalien could you alter this to be introduced as |
|
Done. I switched it to the collections feature to avoid adding a new one. As for predicates, I'm guessing that the problem was that impl<T, V> Predicate<V> for T where T: FnMut { .. }
impl<V> Predicate<V> for SomeMatcher<V> { .. } // What if `SomeMatcher<V>` implements `FnMut`?(Although you could cheat and auto-implement Predicate for all lambdas.) |
|
Can you actually use a more descriptive feature name instead? We're trying to move away from large blanket feature names. |
|
Something like |
|
Done (forgot that GitHub doesn't ping on commits). |
|
⌛ Testing commit e8957aa with merge b129686... |
|
💔 Test failed - auto-mac-64-opt |
|
Sorry about that. It should work now (I think). |
|
Could you also squash the commits? |
|
Done. |
|
⌛ Testing commit 292193b with merge 1636d82... |
|
💔 Test failed - auto-mac-64-opt |
|
Now it should actually be fixed. Is there any way to get rust to not stop testing on failure? The debug tests are broken on my system due to grsecurity and |
|
@bors r=alexcrichton I can ask bors to run the tests on the |
|
📌 Commit decf395 has been approved by |
|
⌛ Testing commit decf395 with merge 465fc8f... |
|
💔 Test failed - auto-linux-64-nopt-t |
Nevermind, I figured out how to disable the faulty test (unset CFG_GDB in config.mk). However, the latest failure is unrelated to these changes (tcp rebind failure). Sorry for wasting so much of your time on a simple patch. |
|
@bors: retry No worries! (we should fix these tests...) |
According to rust-lang/rfcs#235, `VecDeque` should have this method (`VecDeque` was called `RingBuf` at the time) but it was never implemented. I marked this stable since "1.0.0" because it's stable in `Vec`.
According to rust-lang/rfcs#235,
VecDequeshould have this method (VecDequewas calledRingBufat the time) but it was never implemented.I marked this stable since "1.0.0" because it's stable in
Vec.