remove drain-on-drop behavior from vec::DrainFilter and add #[must_use]

Author: the8472

compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs

@@ -753,20 +753,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             return;
         }
 
-        errors.drain_filter(|error| {
+        errors.retain(|error| {
             let Error::Invalid(
                 provided_idx,
                 expected_idx,
                 Compatibility::Incompatible(Some(e)),
-            ) = error else { return false };
+            ) = error else { return true };
             let (provided_ty, provided_span) = provided_arg_tys[*provided_idx];
             let trace =
                 mk_trace(provided_span, formal_and_expected_inputs[*expected_idx], provided_ty);
             if !matches!(trace.cause.as_failure_code(*e), FailureCode::Error0308) {
                 self.err_ctxt().report_and_explain_type_error(trace, *e).emit();
-                return true;
+                return false;
             }
-            false
+            true
         });
 
         // We're done if we found errors, but we already emitted them.

compiler/rustc_trait_selection/src/traits/project.rs

@@ -1170,11 +1170,11 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>(
             };
 
             let mut deduped: SsoHashSet<_> = Default::default();
-            result.obligations.drain_filter(|projected_obligation| {
+            result.obligations.retain(|projected_obligation| {
                 if !deduped.insert(projected_obligation.clone()) {
-                    return true;
+                    return false;
                 }
-                false
+                true
             });
 
             if use_cache {

library/alloc/src/vec/drain_filter.rs

@@ -1,5 +1,4 @@
 use crate::alloc::{Allocator, Global};
-use core::mem::{ManuallyDrop, SizedTypeProperties};
 use core::ptr;
 use core::slice;
 
@@ -20,6 +19,7 @@ use super::Vec;
 /// ```
 #[unstable(feature = "drain_filter", reason = "recently added", issue = "43244")]
 #[derive(Debug)]
+#[must_use = "iterators are lazy and do nothing unless consumed"]
 pub struct DrainFilter<
     'a,
     T,
@@ -55,59 +55,6 @@ where
     pub fn allocator(&self) -> &A {
         self.vec.allocator()
     }
-
-    /// Keep unyielded elements in the source `Vec`.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// #![feature(drain_filter)]
-    /// #![feature(drain_keep_rest)]
-    ///
-    /// let mut vec = vec!['a', 'b', 'c'];
-    /// let mut drain = vec.drain_filter(|_| true);
-    ///
-    /// assert_eq!(drain.next().unwrap(), 'a');
-    ///
-    /// // This call keeps 'b' and 'c' in the vec.
-    /// drain.keep_rest();
-    ///
-    /// // If we wouldn't call `keep_rest()`,
-    /// // `vec` would be empty.
-    /// assert_eq!(vec, ['b', 'c']);
-    /// ```
-    #[unstable(feature = "drain_keep_rest", issue = "101122")]
-    pub fn keep_rest(self) {
-        // At this moment layout looks like this:
-        //
-        //  _____________________/-- old_len
-        // /                     \
-        // [kept] [yielded] [tail]
-        //        \_______/ ^-- idx
-        //                \-- del
-        //
-        // Normally `Drop` impl would drop [tail] (via .for_each(drop), ie still calling `pred`)
-        //
-        // 1. Move [tail] after [kept]
-        // 2. Update length of the original vec to `old_len - del`
-        //    a. In case of ZST, this is the only thing we want to do
-        // 3. Do *not* drop self, as everything is put in a consistent state already, there is nothing to do
-        let mut this = ManuallyDrop::new(self);
-
-        unsafe {
-            // ZSTs have no identity, so we don't need to move them around.
-            if !T::IS_ZST && this.idx < this.old_len && this.del > 0 {
-                let ptr = this.vec.as_mut_ptr();
-                let src = ptr.add(this.idx);
-                let dst = src.sub(this.del);
-                let tail_len = this.old_len - this.idx;
-                src.copy_to(dst, tail_len);
-            }
-
-            let new_len = this.old_len - this.del;
-            this.vec.set_len(new_len);
-        }
-    }
 }
 
 #[unstable(feature = "drain_filter", reason = "recently added", issue = "43244")]
@@ -154,44 +101,21 @@ where
     F: FnMut(&mut T) -> bool,
 {
     fn drop(&mut self) {
-        struct BackshiftOnDrop<'a, 'b, T, F, A: Allocator>
-        where
-            F: FnMut(&mut T) -> bool,
-        {
-            drain: &'b mut DrainFilter<'a, T, F, A>,
-        }
-
-        impl<'a, 'b, T, F, A: Allocator> Drop for BackshiftOnDrop<'a, 'b, T, F, A>
-        where
-            F: FnMut(&mut T) -> bool,
-        {
-            fn drop(&mut self) {
-                unsafe {
-                    if self.drain.idx < self.drain.old_len && self.drain.del > 0 {
-                        // This is a pretty messed up state, and there isn't really an
-                        // obviously right thing to do. We don't want to keep trying
-                        // to execute `pred`, so we just backshift all the unprocessed
-                        // elements and tell the vec that they still exist. The backshift
-                        // is required to prevent a double-drop of the last successfully
-                        // drained item prior to a panic in the predicate.
-                        let ptr = self.drain.vec.as_mut_ptr();
-                        let src = ptr.add(self.drain.idx);
-                        let dst = src.sub(self.drain.del);
-                        let tail_len = self.drain.old_len - self.drain.idx;
-                        src.copy_to(dst, tail_len);
-                    }
-                    self.drain.vec.set_len(self.drain.old_len - self.drain.del);
-                }
+        unsafe {
+            if self.idx < self.old_len && self.del > 0 {
+                // This is a pretty messed up state, and there isn't really an
+                // obviously right thing to do. We don't want to keep trying
+                // to execute `pred`, so we just backshift all the unprocessed
+                // elements and tell the vec that they still exist. The backshift
+                // is required to prevent a double-drop of the last successfully
+                // drained item prior to a panic in the predicate.
+                let ptr = self.vec.as_mut_ptr();
+                let src = ptr.add(self.idx);
+                let dst = src.sub(self.del);
+                let tail_len = self.old_len - self.idx;
+                src.copy_to(dst, tail_len);
             }
-        }
-
-        let backshift = BackshiftOnDrop { drain: self };
-
-        // Attempt to consume any remaining elements if the filter predicate
-        // has not yet panicked. We'll backshift any remaining elements
-        // whether we've already panicked or if the consumption here panics.
-        if !backshift.drain.panic_flag {
-            backshift.drain.for_each(drop);
+            self.vec.set_len(self.old_len - self.del);
         }
     }
 }

library/alloc/src/vec/mod.rs

@@ -2892,6 +2892,12 @@ impl<T, A: Allocator> Vec<T, A> {
     /// If the closure returns false, the element will remain in the vector and will not be yielded
     /// by the iterator.
     ///
+    /// If the returned `DrainFilter` is not exhausted, e.g. because it is dropped without iterating
+    /// or the iteration short-circuits, then the remaining elements will be retained.
+    /// Use [`retain`] with a negated predicate if you do not need the returned iterator.
+    ///
+    /// [`retain`]: Vec::retain
+    ///
     /// Using this method is equivalent to the following code:
     ///
     /// ```

library/alloc/tests/vec.rs

@@ -1608,36 +1608,7 @@ fn drain_filter_unconsumed() {
     let mut vec = vec![1, 2, 3, 4];
     let drain = vec.drain_filter(|&mut x| x % 2 != 0);
     drop(drain);
-    assert_eq!(vec, [2, 4]);
-}
-
-#[test]
-fn test_drain_filter_keep_rest() {
-    let mut v = vec![0, 1, 2, 3, 4, 5, 6];
-    let mut drain = v.drain_filter(|&mut x| x % 2 == 0);
-    assert_eq!(drain.next(), Some(0));
-    assert_eq!(drain.next(), Some(2));
-
-    drain.keep_rest();
-    assert_eq!(v, &[1, 3, 4, 5, 6]);
-}
-
-#[test]
-fn test_drain_filter_keep_rest_all() {
-    let mut v = vec![0, 1, 2, 3, 4, 5, 6];
-    v.drain_filter(|_| true).keep_rest();
-    assert_eq!(v, &[0, 1, 2, 3, 4, 5, 6]);
-}
-
-#[test]
-fn test_drain_filter_keep_rest_none() {
-    let mut v = vec![0, 1, 2, 3, 4, 5, 6];
-    let mut drain = v.drain_filter(|_| true);
-
-    drain.by_ref().for_each(drop);
-
-    drain.keep_rest();
-    assert_eq!(v, &[]);
+    assert_eq!(vec, [1, 2, 3, 4]);
 }
 
 #[test]

src/librustdoc/clean/inline.rs

@@ -158,13 +158,13 @@ pub(crate) fn try_inline_glob(
                 .filter_map(|child| child.res.opt_def_id())
                 .collect();
             let mut items = build_module_items(cx, did, visited, inlined_names, Some(&reexports));
-            items.drain_filter(|item| {
+            items.retain(|item| {
                 if let Some(name) = item.name {
                     // If an item with the same type and name already exists,
                     // it takes priority over the inlined stuff.
-                    !inlined_names.insert((item.type_(), name))
+                    inlined_names.insert((item.type_(), name))
                 } else {
-                    false
+                    true
                 }
             });
             Some(items)