PR fixup

arora-aman · arora-aman · commit 994793faabba · 2021-09-22T05:17:30.000-04:00
diff --git a/compiler/rustc_ty_utils/src/needs_drop.rs b/compiler/rustc_ty_utils/src/needs_drop.rs
@@ -240,13 +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| {
-            if tcx.has_attr(dtor.did, sym::rustc_insignificant_dtor) {
-                DtorType::Insignificant
-            } else {
-                DtorType::Significant
-            }
-        })
+        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)
 }
diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs
@@ -155,14 +155,14 @@ 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,
 }
 
 #[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())
     }
@@ -331,6 +331,7 @@ impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for IterMut<'_, K, V> {
 ///
 /// [`into_iter`]: IntoIterator::into_iter
 #[stable(feature = "rust1", since = "1.0.0")]
+#[rustc_insignificant_dtor]
 pub struct IntoIter<K, V> {
     range: LazyLeafRange<marker::Dying, K, V>,
     length: usize,
@@ -1460,7 +1461,6 @@ 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>);
 
diff --git a/library/alloc/src/collections/linked_list.rs b/library/alloc/src/collections/linked_list.rs
@@ -43,6 +43,7 @@ mod tests;
 /// more memory efficient, and make better use of CPU cache.
 #[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>>>,
@@ -975,7 +976,6 @@ 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>);
 
diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/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,
@@ -130,7 +131,6 @@ 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).
diff --git a/library/alloc/src/rc.rs b/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>>,
@@ -1441,7 +1442,6 @@ 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();
diff --git a/library/alloc/src/vec/into_iter.rs b/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,
@@ -246,7 +247,6 @@ 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>);
 
diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs
@@ -394,6 +394,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,
@@ -2746,7 +2747,6 @@ 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]
diff --git a/library/core/src/array/iter.rs b/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.
     ///
@@ -180,7 +181,6 @@ 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
diff --git a/library/std/src/collections/hash/map.rs b/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>,
 }
diff --git a/library/std/src/lazy.rs b/library/std/src/lazy.rs
@@ -492,7 +492,6 @@ 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
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.fixed b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.fixed
@@ -4,19 +4,16 @@
 #![deny(rust_2021_incompatible_closure_captures)]
 #![allow(unused)]
 
-#![feature(once_cell)]
-
 // Test cases for types that implement an insignificant drop (stlib defined)
 
 macro_rules! test_insig_dtor_for_type {
     ($t: ty, $disambiguator: ident) => {
         mod $disambiguator {
             use std::collections::*;
-            use std::lazy::SyncOnceCell;
             use std::rc::Rc;
             use std::sync::Mutex;
 
-            fn _test_for_type(t: $t) {
+            fn test_for_type(t: $t) {
                 let tup = (Mutex::new(0), t);
 
                 let _c = || tup.0;
@@ -29,12 +26,11 @@ test_insig_dtor_for_type!(i32, prim_i32);
 test_insig_dtor_for_type!(Vec<i32>, vec_i32);
 test_insig_dtor_for_type!(String, string);
 test_insig_dtor_for_type!(Vec<String>, vec_string);
-//test_insig_dtor_for_type!(HashMap<String, String>, hash_map);
+test_insig_dtor_for_type!(HashMap<String, String>, hash_map);
 test_insig_dtor_for_type!(BTreeMap<String, i32>, btree_map);
 test_insig_dtor_for_type!(LinkedList<String>, linked_list);
 test_insig_dtor_for_type!(Rc<i32>, rc_i32);
 test_insig_dtor_for_type!(Rc<String>, rc_string);
-test_insig_dtor_for_type!(SyncOnceCell<String>, onecell);
 test_insig_dtor_for_type!(std::vec::IntoIter<String>, vec_into_iter);
 test_insig_dtor_for_type!(btree_map::IntoIter<String, String>, btree_map_into_iter);
 test_insig_dtor_for_type!(std::array::IntoIter<String, 5>, array_into_iter);
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop.rs
@@ -4,19 +4,16 @@
 #![deny(rust_2021_incompatible_closure_captures)]
 #![allow(unused)]
 
-#![feature(once_cell)]
-
 // Test cases for types that implement an insignificant drop (stlib defined)
 
 macro_rules! test_insig_dtor_for_type {
     ($t: ty, $disambiguator: ident) => {
         mod $disambiguator {
             use std::collections::*;
-            use std::lazy::SyncOnceCell;
             use std::rc::Rc;
             use std::sync::Mutex;
 
-            fn _test_for_type(t: $t) {
+            fn test_for_type(t: $t) {
                 let tup = (Mutex::new(0), t);
 
                 let _c = || tup.0;
@@ -29,12 +26,11 @@ test_insig_dtor_for_type!(i32, prim_i32);
 test_insig_dtor_for_type!(Vec<i32>, vec_i32);
 test_insig_dtor_for_type!(String, string);
 test_insig_dtor_for_type!(Vec<String>, vec_string);
-//test_insig_dtor_for_type!(HashMap<String, String>, hash_map);
+test_insig_dtor_for_type!(HashMap<String, String>, hash_map);
 test_insig_dtor_for_type!(BTreeMap<String, i32>, btree_map);
 test_insig_dtor_for_type!(LinkedList<String>, linked_list);
 test_insig_dtor_for_type!(Rc<i32>, rc_i32);
 test_insig_dtor_for_type!(Rc<String>, rc_string);
-test_insig_dtor_for_type!(SyncOnceCell<String>, onecell);
 test_insig_dtor_for_type!(std::vec::IntoIter<String>, vec_into_iter);
 test_insig_dtor_for_type!(btree_map::IntoIter<String, String>, btree_map_into_iter);
 test_insig_dtor_for_type!(std::array::IntoIter<String, 5>, array_into_iter);
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.fixed b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.fixed
@@ -5,15 +5,15 @@
 #![feature(rustc_attrs)]
 #![allow(unused)]
 
-    use std::sync::Mutex;
+use std::sync::Mutex;
 
+    #[rustc_insignificant_dtor]
 struct InsignificantDropPoint {
     x: i32,
     y: Mutex<i32>,
 }
 
 impl Drop for InsignificantDropPoint {
-    #[rustc_insignificant_dtor]
     fn drop(&mut self) {}
 }
 
@@ -23,25 +23,14 @@ impl Drop for SigDrop {
     fn drop(&mut self) {}
 }
 
+#[rustc_insignificant_dtor]
 struct GenericStruct<T>(T, T);
 
-struct Wrapper<T>(GenericStruct<T>, i32);
-
 impl<T> Drop for GenericStruct<T> {
-    #[rustc_insignificant_dtor]
     fn drop(&mut self) {}
 }
 
-// Test no migration because InsignificantDropPoint is marked as insignificant
-fn insign_dtor() {
-    let t = (
-        InsignificantDropPoint { x: 0, y: Mutex::new(0) },
-        InsignificantDropPoint { x: 0, y: Mutex::new(0) }
-    );
-
-    let c = || t.0;
-
-}
+struct Wrapper<T>(GenericStruct<T>, i32);
 
 // `SigDrop` implements drop and therefore needs to be migrated.
 fn significant_drop_needs_migration() {
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.rs b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.rs
@@ -5,15 +5,15 @@
 #![feature(rustc_attrs)]
 #![allow(unused)]
 
-    use std::sync::Mutex;
+use std::sync::Mutex;
 
+    #[rustc_insignificant_dtor]
 struct InsignificantDropPoint {
     x: i32,
     y: Mutex<i32>,
 }
 
 impl Drop for InsignificantDropPoint {
-    #[rustc_insignificant_dtor]
     fn drop(&mut self) {}
 }
 
@@ -23,25 +23,14 @@ impl Drop for SigDrop {
     fn drop(&mut self) {}
 }
 
+#[rustc_insignificant_dtor]
 struct GenericStruct<T>(T, T);
 
-struct Wrapper<T>(GenericStruct<T>, i32);
-
 impl<T> Drop for GenericStruct<T> {
-    #[rustc_insignificant_dtor]
     fn drop(&mut self) {}
 }
 
-// Test no migration because InsignificantDropPoint is marked as insignificant
-fn insign_dtor() {
-    let t = (
-        InsignificantDropPoint { x: 0, y: Mutex::new(0) },
-        InsignificantDropPoint { x: 0, y: Mutex::new(0) }
-    );
-
-    let c = || t.0;
-
-}
+struct Wrapper<T>(GenericStruct<T>, i32);
 
 // `SigDrop` implements drop and therefore needs to be migrated.
 fn significant_drop_needs_migration() {
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.stderr
@@ -1,5 +1,5 @@
 error: changes to closure capture in Rust 2021 will affect drop order
-  --> $DIR/insignificant_drop_attr_migrations.rs:50:13
+  --> $DIR/insignificant_drop_attr_migrations.rs:39:13
    |
 LL |     let c = || {
    |             ^^
@@ -23,7 +23,7 @@ LL +         let _ = &t;
    |
 
 error: changes to closure capture in Rust 2021 will affect drop order
-  --> $DIR/insignificant_drop_attr_migrations.rs:70:13
+  --> $DIR/insignificant_drop_attr_migrations.rs:59:13
    |
 LL |     let c = move || {
    |             ^^^^^^^
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_no_migrations.rs b/src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_no_migrations.rs
@@ -3,14 +3,14 @@
 #![deny(rust_2021_incompatible_closure_captures)]
 #![feature(rustc_attrs)]
 #![allow(unused)]
+#[rustc_insignificant_dtor]
 
 struct InsignificantDropPoint {
     x: i32,
     y: i32,
 }
 
 impl Drop for InsignificantDropPoint {
-    #[rustc_insignificant_dtor]
     fn drop(&mut self) {}
 }
 
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.fixed b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.fixed
@@ -202,6 +202,19 @@ fn test9_drop_order_and_nested_closures() {
     b();
 }
 
+// Test that we migrate if drop order of Vec<T> would be affected if T is a significant drop type
+fn test10_vec_of_significant_drop_type() {
+
+        let tup = (Foo(0), vec![Foo(3)]);
+
+        let _c = || { let _ = &tup; tup.0 };
+            //~^ ERROR: drop order
+            //~| NOTE: for more information, see
+            //~| HELP: add a dummy let to cause `tup` to be fully captured
+            //~| NOTE: in Rust 2018, this closure captures all of `tup`, but in Rust 2021, it will only capture `tup.0`
+}
+//~^ NOTE: in Rust 2018, `tup` is dropped here, but in Rust 2021, only `tup.0` will be dropped here as part of the closure
+
 fn main() {
     test1_all_need_migration();
     test2_only_precise_paths_need_migration();
@@ -212,4 +225,5 @@ fn main() {
     test7_move_closures_non_copy_types_might_need_migration();
     test8_drop_order_and_blocks();
     test9_drop_order_and_nested_closures();
+    test10_vec_of_significant_drop_type();
 }
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs
diff --git a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr

Original file line number	Diff line number	Diff line change
`@@ -205,6 +205,7 @@ use crate::sys;`
`205`	`205`
`206`	`206`	`#[cfg_attr(not(test), rustc_diagnostic_item = "hashmap_type")]`
`207`	`207`	`#[stable(feature = "rust1", since = "1.0.0")]`
	`208`	`+#[rustc_insignificant_dtor]`
`208`	`209`	`pub struct HashMap<K, V, S = RandomState> {`
`209`	`210`	`base: base::HashMap<K, V, S>,`
`210`	`211`	`}`
Original file line number	Diff line number	Diff line change
`@@ -492,7 +492,6 @@ impl<T> SyncOnceCell<T> {`
`492`	`492`	`}`
`493`	`493`
`494`	`494`	`unsafe impl<#[may_dangle] T> Drop for SyncOnceCell<T> {`
`495`		`- #[rustc_insignificant_dtor]`
`496`	`495`	`fn drop(&mut self) {`
`497`	`496`	`if self.is_initialized() {`
`498`	`497`	`// SAFETY: The cell is initialized and being dropped, so it can't`