Avoid use of unsafe in pop_if

Author: avandesa

Diff for: library/alloc/src/vec/mod.rs

@@ -2079,19 +2079,8 @@ impl<T, A: Allocator> Vec<T, A> {
     where
         F: FnOnce(&mut T) -> bool,
     {
-        if self.len == 0 {
-            return None;
-        }
-
-        // SAFETY: the vector is not empty
-        let mut last = unsafe { ptr::read(self.as_ptr().add(self.len() - 1)) };
-
-        if f(&mut last) {
-            self.len -= 1;
-            Some(last)
-        } else {
-            None
-        }
+        let last = self.last_mut()?;
+        if f(last) { self.pop() } else { None }
     }
 
     /// Moves all the elements of `other` into `self`, leaving `other` empty.

Diff for: library/alloc/tests/vec.rs

@@ -2663,6 +2663,17 @@ fn test_pop_if_empty() {
     assert!(v.is_empty());
 }
 
+#[test]
+fn test_pop_if_mutates() {
+    let mut v = vec![1];
+    let pred = |x: &mut i32| {
+        *x += 1;
+        false
+    };
+    assert_eq!(v.pop_if(pred), None);
+    assert_eq!(v, [2]);
+}
+
 /// This assortment of tests, in combination with miri, verifies we handle UB on fishy arguments
 /// in the stdlib. Draining and extending the allocation are fairly well-tested earlier, but
 /// `vec.insert(usize::MAX, val)` once slipped by!