2229: Mark insignificant dtor in stdlib

compiler/rustc_ty_utils/src/needs_drop.rs

@@ -3,6 +3,7 @@
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::def_id::DefId;
 use rustc_middle::ty::subst::Subst;
+use rustc_middle::ty::subst::SubstsRef;
 use rustc_middle::ty::util::{needs_drop_components, AlwaysRequiresDrop};
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_session::Limit;
@@ -12,7 +13,7 @@ type NeedsDropResult<T> = Result<T, AlwaysRequiresDrop>;
 
 fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
     let adt_components =
-        move |adt_def: &ty::AdtDef| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter());
+        move |adt_def: &ty::AdtDef, _| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter());
 
     // If we don't know a type doesn't need drop, for example if it's a type
     // parameter without a `Copy` bound, then we conservatively return that it
@@ -28,8 +29,9 @@ fn has_significant_drop_raw<'tcx>(
     tcx: TyCtxt<'tcx>,
     query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
 ) -> bool {
-    let significant_drop_fields =
-        move |adt_def: &ty::AdtDef| tcx.adt_significant_drop_tys(adt_def.did).map(|tys| tys.iter());
+    let significant_drop_fields = move |adt_def: &ty::AdtDef, _| {
+        tcx.adt_significant_drop_tys(adt_def.did).map(|tys| tys.iter())
+    };
     let res = NeedsDropTypes::new(tcx, query.param_env, query.value, significant_drop_fields)
         .next()
         .is_some();
@@ -74,7 +76,7 @@ impl<'tcx, F> NeedsDropTypes<'tcx, F> {
 
 impl<'tcx, F, I> Iterator for NeedsDropTypes<'tcx, F>
 where
-    F: Fn(&ty::AdtDef) -> NeedsDropResult<I>,
+    F: Fn(&ty::AdtDef, SubstsRef<'tcx>) -> NeedsDropResult<I>,
     I: Iterator<Item = Ty<'tcx>>,
 {
     type Item = NeedsDropResult<Ty<'tcx>>;
@@ -138,7 +140,7 @@ where
                     // `ManuallyDrop`. If it's a struct or enum without a `Drop`
                     // impl then check whether the field types need `Drop`.
                     ty::Adt(adt_def, substs) => {
-                        let tys = match (self.adt_components)(adt_def) {
+                        let tys = match (self.adt_components)(adt_def, substs) {
                             Err(e) => return Some(Err(e)),
                             Ok(tys) => tys,
                         };
@@ -171,22 +173,44 @@ where
     }
 }
 
+enum DtorType {
+    /// Type has a `Drop` but it is considered insignificant.
+    /// Check the query `adt_significant_drop_tys` for understanding
+    /// "significant" / "insignificant".
+    Insignificant,
+
+    /// Type has a `Drop` implentation.
+    Significant,
+}
+
 // This is a helper function for `adt_drop_tys` and `adt_significant_drop_tys`.
 // Depending on the implentation of `adt_has_dtor`, it is used to check if the
 // ADT has a destructor or if the ADT only has a significant destructor. For
 // understanding significant destructor look at `adt_significant_drop_tys`.
-fn adt_drop_tys_helper(
-    tcx: TyCtxt<'_>,
+fn adt_drop_tys_helper<'tcx>(
+    tcx: TyCtxt<'tcx>,
     def_id: DefId,
-    adt_has_dtor: impl Fn(&ty::AdtDef) -> bool,
-) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
-    let adt_components = move |adt_def: &ty::AdtDef| {
+    adt_has_dtor: impl Fn(&ty::AdtDef) -> Option<DtorType>,
+) -> Result<&ty::List<Ty<'tcx>>, AlwaysRequiresDrop> {
+    let adt_components = move |adt_def: &ty::AdtDef, substs: SubstsRef<'tcx>| {
         if adt_def.is_manually_drop() {
             debug!("adt_drop_tys: `{:?}` is manually drop", adt_def);
             return Ok(Vec::new().into_iter());
-        } else if adt_has_dtor(adt_def) {
-            debug!("adt_drop_tys: `{:?}` implements `Drop`", adt_def);
-            return Err(AlwaysRequiresDrop);
+        } else if let Some(dtor_info) = adt_has_dtor(adt_def) {
+            match dtor_info {
+                DtorType::Significant => {
+                    debug!("adt_drop_tys: `{:?}` implements `Drop`", adt_def);
+                    return Err(AlwaysRequiresDrop);
+                }
+                DtorType::Insignificant => {
+                    debug!("adt_drop_tys: `{:?}` drop is insignificant", adt_def);
+
+                    // Since the destructor is insignificant, we just want to make sure all of
+                    // the passed in type parameters are also insignificant.
+                    // Eg: Vec<T> dtor is insignificant when T=i32 but significant when T=Mutex.
+                    return Ok(substs.types().collect::<Vec<Ty<'_>>>().into_iter());
+                }
+            }
         } else if adt_def.is_union() {
             debug!("adt_drop_tys: `{:?}` is a union", adt_def);
             return Ok(Vec::new().into_iter());
@@ -204,7 +228,10 @@ fn adt_drop_tys_helper(
 }
 
 fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
-    let adt_has_dtor = |adt_def: &ty::AdtDef| adt_def.destructor(tcx).is_some();
+    // This is for the "needs_drop" query, that considers all `Drop` impls, therefore all dtors are
+    // significant.
+    let adt_has_dtor =
+        |adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
     adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
 }
 
@@ -213,10 +240,13 @@ fn adt_significant_drop_tys(
     def_id: DefId,
 ) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
     let adt_has_dtor = |adt_def: &ty::AdtDef| {
-        adt_def
-            .destructor(tcx)
-            .map(|dtor| !tcx.has_attr(dtor.did, sym::rustc_insignificant_dtor))
-            .unwrap_or(false)
+        adt_def.destructor(tcx).map(|dtor| {
+            if tcx.has_attr(dtor.did, sym::rustc_insignificant_dtor) {
+                DtorType::Insignificant
+            } else {
+                DtorType::Significant
+            }
+        })
     };
     adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
 }

library/alloc/src/collections/btree/map.rs

@@ -162,6 +162,7 @@ pub struct BTreeMap<K, V> {
 
 #[stable(feature = "btree_drop", since = "1.7.0")]
 unsafe impl<#[may_dangle] K, #[may_dangle] V> Drop for BTreeMap<K, V> {
+    #[rustc_insignificant_dtor]
     fn drop(&mut self) {
         drop(unsafe { ptr::read(self) }.into_iter())
     }
@@ -1459,6 +1460,7 @@ impl<K, V> IntoIterator for BTreeMap<K, V> {
 
 #[stable(feature = "btree_drop", since = "1.7.0")]
 impl<K, V> Drop for IntoIter<K, V> {
+    #[rustc_insignificant_dtor]
     fn drop(&mut self) {
         struct DropGuard<'a, K, V>(&'a mut IntoIter<K, V>);
 

library/alloc/src/collections/linked_list.rs

@@ -975,6 +975,7 @@ impl<T> LinkedList<T> {
 
 #[stable(feature = "rust1", since = "1.0.0")]
 unsafe impl<#[may_dangle] T> Drop for LinkedList<T> {
+    #[rustc_insignificant_dtor]
     fn drop(&mut self) {
         struct DropGuard<'a, T>(&'a mut LinkedList<T>);
 

library/alloc/src/collections/vec_deque/mod.rs

@@ -130,6 +130,7 @@ impl<T: Clone, A: Allocator + Clone> Clone for VecDeque<T, A> {
 
 #[stable(feature = "rust1", since = "1.0.0")]
 unsafe impl<#[may_dangle] T, A: Allocator> Drop for VecDeque<T, A> {
+    #[rustc_insignificant_dtor]
     fn drop(&mut self) {
         /// Runs the destructor for all items in the slice when it gets dropped (normally or
         /// during unwinding).

library/alloc/src/rc.rs

@@ -1441,6 +1441,7 @@ unsafe impl<#[may_dangle] T: ?Sized> Drop for Rc<T> {
     /// drop(foo);    // Doesn't print anything
     /// drop(foo2);   // Prints "dropped!"
     /// ```
+    #[rustc_insignificant_dtor]
     fn drop(&mut self) {
         unsafe {
             self.inner().dec_strong();

library/alloc/src/vec/into_iter.rs

@@ -246,6 +246,7 @@ impl<T: Clone, A: Allocator + Clone> Clone for IntoIter<T, A> {
 
 #[stable(feature = "rust1", since = "1.0.0")]
 unsafe impl<#[may_dangle] T, A: Allocator> Drop for IntoIter<T, A> {
+    #[rustc_insignificant_dtor]
     fn drop(&mut self) {
         struct DropGuard<'a, T, A: Allocator>(&'a mut IntoIter<T, A>);
 

library/alloc/src/vec/mod.rs

@@ -2746,6 +2746,7 @@ impl<T: Ord, A: Allocator> Ord for Vec<T, A> {
 
 #[stable(feature = "rust1", since = "1.0.0")]
 unsafe impl<#[may_dangle] T, A: Allocator> Drop for Vec<T, A> {
+    #[rustc_insignificant_dtor]
     fn drop(&mut self) {
         unsafe {
             // use drop for [T]

library/core/src/array/iter.rs

@@ -180,6 +180,7 @@ impl<T, const N: usize> DoubleEndedIterator for IntoIter<T, N> {
 
 #[stable(feature = "array_value_iter_impls", since = "1.40.0")]
 impl<T, const N: usize> Drop for IntoIter<T, N> {
+    #[rustc_insignificant_dtor]
     fn drop(&mut self) {
         // SAFETY: This is safe: `as_mut_slice` returns exactly the sub-slice
         // of elements that have not been moved out yet and that remain

library/std/src/lazy.rs

@@ -492,6 +492,7 @@ impl<T> SyncOnceCell<T> {
 }
 
 unsafe impl<#[may_dangle] T> Drop for SyncOnceCell<T> {
+    #[rustc_insignificant_dtor]
     fn drop(&mut self) {
         if self.is_initialized() {
             // SAFETY: The cell is initialized and being dropped, so it can't

src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.fixed

@@ -4,6 +4,14 @@
 
 use std::thread;
 
+#[derive(Debug)]
+struct Foo(i32);
+impl Drop for Foo {
+    fn drop(&mut self) {
+        println!("{:?} dropped", self.0);
+    }
+}
+
 /* Test Send Trait Migration */
 struct SendPointer(*mut i32);
 unsafe impl Send for SendPointer {}
@@ -42,19 +50,19 @@ fn test_sync_trait() {
 }
 
 /* Test Clone Trait Migration */
-struct S(String);
+struct S(Foo);
 struct T(i32);
 
 struct U(S, T);
 
 impl Clone for U {
     fn clone(&self) -> Self {
-        U(S(String::from("Hello World")), T(0))
+        U(S(Foo(0)), T(0))
     }
 }
 
 fn test_clone_trait() {
-    let f = U(S(String::from("Hello World")), T(0));
+    let f = U(S(Foo(0)), T(0));
     let c = || {
         let _ = &f;
         //~^ ERROR: `Clone` trait implementation for closure and drop order

src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.rs

@@ -4,6 +4,14 @@
 
 use std::thread;
 
+#[derive(Debug)]
+struct Foo(i32);
+impl Drop for Foo {
+    fn drop(&mut self) {
+        println!("{:?} dropped", self.0);
+    }
+}
+
 /* Test Send Trait Migration */
 struct SendPointer(*mut i32);
 unsafe impl Send for SendPointer {}
@@ -42,19 +50,19 @@ fn test_sync_trait() {
 }
 
 /* Test Clone Trait Migration */
-struct S(String);
+struct S(Foo);
 struct T(i32);
 
 struct U(S, T);
 
 impl Clone for U {
     fn clone(&self) -> Self {
-        U(S(String::from("Hello World")), T(0))
+        U(S(Foo(0)), T(0))
     }
 }
 
 fn test_clone_trait() {
-    let f = U(S(String::from("Hello World")), T(0));
+    let f = U(S(Foo(0)), T(0));
     let c = || {
         //~^ ERROR: `Clone` trait implementation for closure and drop order
         //~| NOTE: in Rust 2018, this closure implements `Clone` as `f` implements `Clone`, but in Rust 2021, this closure will no longer implement `Clone` as `f.1` does not implement `Clone`

src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr

@@ -1,5 +1,5 @@
 error: changes to closure capture in Rust 2021 will affect `Send` trait implementation for closure
-  --> $DIR/auto_traits.rs:14:19
+  --> $DIR/auto_traits.rs:22:19
    |
 LL |     thread::spawn(move || unsafe {
    |                   ^^^^^^^^^^^^^^ in Rust 2018, this closure implements `Send` as `fptr` implements `Send`, but in Rust 2021, this closure will no longer implement `Send` as `fptr.0` does not implement `Send`
@@ -24,7 +24,7 @@ LL |         *fptr.0 = 20;
  ...
 
 error: changes to closure capture in Rust 2021 will affect `Sync`, `Send` trait implementation for closure
-  --> $DIR/auto_traits.rs:34:19
+  --> $DIR/auto_traits.rs:42:19
    |
 LL |     thread::spawn(move || unsafe {
    |                   ^^^^^^^^^^^^^^ in Rust 2018, this closure implements `Sync`, `Send` as `fptr` implements `Sync`, `Send`, but in Rust 2021, this closure will no longer implement `Sync`, `Send` as `fptr.0.0` does not implement `Sync`, `Send`
@@ -44,7 +44,7 @@ LL |         *fptr.0.0 = 20;
  ...
 
 error: changes to closure capture in Rust 2021 will affect `Clone` trait implementation for closure and drop order
-  --> $DIR/auto_traits.rs:58:13
+  --> $DIR/auto_traits.rs:66:13
    |
 LL |     let c = || {
    |             ^^ in Rust 2018, this closure implements `Clone` as `f` implements `Clone`, but in Rust 2021, this closure will no longer implement `Clone` as `f.1` does not implement `Clone`

src/test/ui/closures/2229_closure_analysis/migrations/closure-body-macro-fragment.fixed

@@ -3,6 +3,14 @@
 // check-pass
 #![warn(rust_2021_compatibility)]
 
+#[derive(Debug)]
+struct Foo(i32);
+impl Drop for Foo {
+    fn drop(&mut self) {
+        println!("{:?} dropped", self.0);
+    }
+}
+
 macro_rules! m {
     (@ $body:expr) => {{
         let f = || $body;
@@ -15,11 +23,11 @@ macro_rules! m {
 }
 
 fn main() {
-    let a = (1.to_string(), 2.to_string());
+    let a = (Foo(0), Foo(1));
     m!({
         let _ = &a;
         //~^ HELP: add a dummy
         let x = a.0;
-        println!("{}", x);
+        println!("{:?}", x);
     });
 }

src/test/ui/closures/2229_closure_analysis/migrations/closure-body-macro-fragment.rs

@@ -3,6 +3,14 @@
 // check-pass
 #![warn(rust_2021_compatibility)]
 
+#[derive(Debug)]
+struct Foo(i32);
+impl Drop for Foo {
+    fn drop(&mut self) {
+        println!("{:?} dropped", self.0);
+    }
+}
+
 macro_rules! m {
     (@ $body:expr) => {{
         let f = || $body;
@@ -15,10 +23,10 @@ macro_rules! m {
 }
 
 fn main() {
-    let a = (1.to_string(), 2.to_string());
+    let a = (Foo(0), Foo(1));
     m!({
         //~^ HELP: add a dummy
         let x = a.0;
-        println!("{}", x);
+        println!("{:?}", x);
     });
 }

src/test/ui/closures/2229_closure_analysis/migrations/closure-body-macro-fragment.stderr

@@ -1,5 +1,5 @@
 warning: changes to closure capture in Rust 2021 will affect drop order
-  --> $DIR/closure-body-macro-fragment.rs:8:17
+  --> $DIR/closure-body-macro-fragment.rs:16:17
    |
 LL |           let f = || $body;
    |  _________________^
@@ -15,7 +15,7 @@ LL | /     m!({
 LL | |
 LL | |         let x = a.0;
    | |                 --- in Rust 2018, this closure captures all of `a`, but in Rust 2021, it will only capture `a.0`
-LL | |         println!("{}", x);
+LL | |         println!("{:?}", x);
 LL | |     });
    | |_______- in this macro invocation
    |