Auto merge of rust-lang#89144 - sexxi-goose:insig_stdlib, r=nikomatsakis

2229: Mark insignificant dtor in stdlib

I looked at all public [stdlib Drop implementations](https://doc.rust-lang.org/stable/std/ops/trait.Drop.html#implementors) and categorized them into Insigificant/Maybe/Significant Drop.

Reasons are noted here: https://docs.google.com/spreadsheets/d/19edb9r5lo2UqMrCOVjV0fwcSdS-R7qvKNL76q7tO8VA/edit#gid=1838773501

One thing missing from this PR is tagging HashMap as insigificant destructor as that needs some discussion.

r? `@Mark-Simulacrum`

cc `@nikomatsakis`

Author: bors

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,22 @@ 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)
+        let is_marked_insig = tcx.has_attr(adt_def.did, sym::rustc_insignificant_dtor);
+        if is_marked_insig {
+            // In some cases like `std::collections::HashMap` where the struct is a wrapper around
+            // a type that is a Drop type, and the wrapped type (eg: `hashbrown::HashMap`) lies
+            // outside stdlib, we might choose to still annotate the the wrapper (std HashMap) with
+            // `rustc_insignificant_dtor`, even if the type itself doesn't have a `Drop` impl.
+            Some(DtorType::Insignificant)
+        } else if adt_def.destructor(tcx).is_some() {
+            // There is a Drop impl and the type isn't marked insignificant, therefore Drop must be
+            // significant.
+            Some(DtorType::Significant)
+        } else {
+            // No destructor found nor the type is annotated with `rustc_insignificant_dtor`, we
+            // treat this as the simple case of Drop impl for type.
+            None
+        }
     };
     adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
 }

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

@@ -155,6 +155,7 @@ pub(super) const MIN_LEN: usize = node::MIN_LEN_AFTER_SPLIT;
 /// ```
 #[stable(feature = "rust1", since = "1.0.0")]
 #[cfg_attr(not(test), rustc_diagnostic_item = "BTreeMap")]
+#[rustc_insignificant_dtor]
 pub struct BTreeMap<K, V> {
     root: Option<Root<K, V>>,
     length: usize,
@@ -331,6 +332,7 @@ impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for IterMut<'_, K, V> {
 /// [`into_iter`]: IntoIterator::into_iter
 /// [`IntoIterator`]: core::iter::IntoIterator
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_insignificant_dtor]
 pub struct IntoIter<K, V> {
     range: LazyLeafRange<marker::Dying, K, V>,
     length: usize,

library/alloc/src/collections/linked_list.rs

@@ -46,6 +46,7 @@ mod tests;
 /// [`VecDeque`]: super::vec_deque::VecDeque
 #[stable(feature = "rust1", since = "1.0.0")]
 #[cfg_attr(not(test), rustc_diagnostic_item = "LinkedList")]
+#[rustc_insignificant_dtor]
 pub struct LinkedList<T> {
     head: Option<NonNull<Node<T>>>,
     tail: Option<NonNull<Node<T>>>,

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

@@ -90,6 +90,7 @@ const MAXIMUM_ZST_CAPACITY: usize = 1 << (usize::BITS - 1); // Largest possible
 /// [`make_contiguous`]: VecDeque::make_contiguous
 #[cfg_attr(not(test), rustc_diagnostic_item = "vecdeque_type")]
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_insignificant_dtor]
 pub struct VecDeque<
     T,
     #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,

library/alloc/src/rc.rs

@@ -305,6 +305,7 @@ struct RcBox<T: ?Sized> {
 /// [get_mut]: Rc::get_mut
 #[cfg_attr(not(test), rustc_diagnostic_item = "Rc")]
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_insignificant_dtor]
 pub struct Rc<T: ?Sized> {
     ptr: NonNull<RcBox<T>>,
     phantom: PhantomData<RcBox<T>>,

library/alloc/src/vec/into_iter.rs

@@ -22,6 +22,7 @@ use core::slice::{self};
 /// let iter: std::vec::IntoIter<_> = v.into_iter();
 /// ```
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_insignificant_dtor]
 pub struct IntoIter<
     T,
     #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,

library/alloc/src/vec/mod.rs

@@ -396,6 +396,7 @@ mod spec_extend;
 /// [owned slice]: Box
 #[stable(feature = "rust1", since = "1.0.0")]
 #[cfg_attr(not(test), rustc_diagnostic_item = "vec_type")]
+#[rustc_insignificant_dtor]
 pub struct Vec<T, #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global> {
     buf: RawVec<T, A>,
     len: usize,

library/core/src/array/iter.rs

@@ -10,6 +10,7 @@ use crate::{
 
 /// A by-value [array] iterator.
 #[stable(feature = "array_value_iter", since = "1.51.0")]
+#[rustc_insignificant_dtor]
 pub struct IntoIter<T, const N: usize> {
     /// This is the array we are iterating over.
     ///

library/std/src/collections/hash/map.rs

@@ -205,6 +205,7 @@ use crate::sys;
 
 #[cfg_attr(not(test), rustc_diagnostic_item = "hashmap_type")]
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_insignificant_dtor]
 pub struct HashMap<K, V, S = RandomState> {
     base: base::HashMap<K, V, S>,
 }

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