From 994793faabba1c490d108504b428ac653433ae44 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Wed, 22 Sep 2021 05:17:30 -0400 Subject: [PATCH] PR fixup --- compiler/rustc_ty_utils/src/needs_drop.rs | 23 +++++++++++++------ library/alloc/src/collections/btree/map.rs | 4 ++-- library/alloc/src/collections/linked_list.rs | 2 +- .../alloc/src/collections/vec_deque/mod.rs | 2 +- library/alloc/src/rc.rs | 2 +- library/alloc/src/vec/into_iter.rs | 2 +- library/alloc/src/vec/mod.rs | 2 +- library/core/src/array/iter.rs | 2 +- library/std/src/collections/hash/map.rs | 1 + library/std/src/lazy.rs | 1 - .../migrations/insignificant_drop.fixed | 8 ++----- .../migrations/insignificant_drop.rs | 8 ++----- .../insignificant_drop_attr_migrations.fixed | 19 ++++----------- .../insignificant_drop_attr_migrations.rs | 19 ++++----------- .../insignificant_drop_attr_migrations.stderr | 4 ++-- .../insignificant_drop_attr_no_migrations.rs | 2 +- .../migrations/significant_drop.fixed | 14 +++++++++++ .../migrations/significant_drop.rs | 14 +++++++++++ .../migrations/significant_drop.stderr | 19 ++++++++++++++- 19 files changed, 86 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_ty_utils/src/needs_drop.rs b/compiler/rustc_ty_utils/src/needs_drop.rs index 039cbaf4982ee..98415a84c569b 100644 --- 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>, 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 index 5486f86271818..10298e117f5b4 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/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 { root: Option>, length: usize, @@ -162,7 +163,6 @@ pub struct BTreeMap { #[stable(feature = "btree_drop", since = "1.7.0")] unsafe impl<#[may_dangle] K, #[may_dangle] V> Drop for BTreeMap { - #[rustc_insignificant_dtor] fn drop(&mut self) { drop(unsafe { ptr::read(self) }.into_iter()) } @@ -331,6 +331,7 @@ impl fmt::Debug for IterMut<'_, K, V> { /// /// [`into_iter`]: IntoIterator::into_iter #[stable(feature = "rust1", since = "1.0.0")] +#[rustc_insignificant_dtor] pub struct IntoIter { range: LazyLeafRange, length: usize, @@ -1460,7 +1461,6 @@ impl IntoIterator for BTreeMap { #[stable(feature = "btree_drop", since = "1.7.0")] impl Drop for IntoIter { - #[rustc_insignificant_dtor] fn drop(&mut self) { struct DropGuard<'a, K, V>(&'a mut IntoIter); diff --git a/library/alloc/src/collections/linked_list.rs b/library/alloc/src/collections/linked_list.rs index 9f390cfc955b4..cef9bb60b885e 100644 --- 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 { head: Option>>, tail: Option>>, @@ -975,7 +976,6 @@ impl LinkedList { #[stable(feature = "rust1", since = "1.0.0")] unsafe impl<#[may_dangle] T> Drop for LinkedList { - #[rustc_insignificant_dtor] fn drop(&mut self) { struct DropGuard<'a, T>(&'a mut LinkedList); diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/library/alloc/src/collections/vec_deque/mod.rs index a79aac39c71b8..cae0f29af8327 100644 --- 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 Clone for VecDeque { #[stable(feature = "rust1", since = "1.0.0")] unsafe impl<#[may_dangle] T, A: Allocator> Drop for VecDeque { - #[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 index 8b0d7f19b1917..78356f7a48ac8 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -305,6 +305,7 @@ struct RcBox { /// [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 { ptr: NonNull>, phantom: PhantomData>, @@ -1441,7 +1442,6 @@ unsafe impl<#[may_dangle] T: ?Sized> Drop for Rc { /// 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 index 36ef96e659569..4cb0a4b10bd0c 100644 --- 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 Clone for IntoIter { #[stable(feature = "rust1", since = "1.0.0")] unsafe impl<#[may_dangle] T, A: Allocator> Drop for IntoIter { - #[rustc_insignificant_dtor] fn drop(&mut self) { struct DropGuard<'a, T, A: Allocator>(&'a mut IntoIter); diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index ef44b6ef82f44..cfbf207aee99c 100644 --- 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 { buf: RawVec, len: usize, @@ -2746,7 +2747,6 @@ impl Ord for Vec { #[stable(feature = "rust1", since = "1.0.0")] unsafe impl<#[may_dangle] T, A: Allocator> Drop for Vec { - #[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 index 06867fd8b8476..822747dd0e824 100644 --- 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 { /// This is the array we are iterating over. /// @@ -180,7 +181,6 @@ impl DoubleEndedIterator for IntoIter { #[stable(feature = "array_value_iter_impls", since = "1.40.0")] impl Drop for IntoIter { - #[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 index 36077a42b48ac..3978e9b1668e5 100644 --- 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 { base: base::HashMap, } diff --git a/library/std/src/lazy.rs b/library/std/src/lazy.rs index 3bb4cacb02a4c..5afdb799f0c74 100644 --- a/library/std/src/lazy.rs +++ b/library/std/src/lazy.rs @@ -492,7 +492,6 @@ impl SyncOnceCell { } unsafe impl<#[may_dangle] T> Drop for SyncOnceCell { - #[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 index abcd92eb6b862..2652bf5988e65 100644 --- 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, vec_i32); test_insig_dtor_for_type!(String, string); test_insig_dtor_for_type!(Vec, vec_string); -//test_insig_dtor_for_type!(HashMap, hash_map); +test_insig_dtor_for_type!(HashMap, hash_map); test_insig_dtor_for_type!(BTreeMap, btree_map); test_insig_dtor_for_type!(LinkedList, linked_list); test_insig_dtor_for_type!(Rc, rc_i32); test_insig_dtor_for_type!(Rc, rc_string); -test_insig_dtor_for_type!(SyncOnceCell, onecell); test_insig_dtor_for_type!(std::vec::IntoIter, vec_into_iter); test_insig_dtor_for_type!(btree_map::IntoIter, btree_map_into_iter); test_insig_dtor_for_type!(std::array::IntoIter, 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 index abcd92eb6b862..2652bf5988e65 100644 --- 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, vec_i32); test_insig_dtor_for_type!(String, string); test_insig_dtor_for_type!(Vec, vec_string); -//test_insig_dtor_for_type!(HashMap, hash_map); +test_insig_dtor_for_type!(HashMap, hash_map); test_insig_dtor_for_type!(BTreeMap, btree_map); test_insig_dtor_for_type!(LinkedList, linked_list); test_insig_dtor_for_type!(Rc, rc_i32); test_insig_dtor_for_type!(Rc, rc_string); -test_insig_dtor_for_type!(SyncOnceCell, onecell); test_insig_dtor_for_type!(std::vec::IntoIter, vec_into_iter); test_insig_dtor_for_type!(btree_map::IntoIter, btree_map_into_iter); test_insig_dtor_for_type!(std::array::IntoIter, 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 index d0fad7b07df0e..d985e3bb9ec74 100644 --- 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, } 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); -struct Wrapper(GenericStruct, i32); - impl Drop for GenericStruct { - #[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(GenericStruct, 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 index ff7af685ea5bb..f95d34eeb299a 100644 --- 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, } 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); -struct Wrapper(GenericStruct, i32); - impl Drop for GenericStruct { - #[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(GenericStruct, 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 index c20bd572af874..832a81711b137 100644 --- 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 index a527bf42e574a..3f184a67fbac9 100644 --- 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,6 +3,7 @@ #![deny(rust_2021_incompatible_closure_captures)] #![feature(rustc_attrs)] #![allow(unused)] +#[rustc_insignificant_dtor] struct InsignificantDropPoint { x: i32, @@ -10,7 +11,6 @@ struct InsignificantDropPoint { } 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 index d3ede331313c2..63e4000e833eb 100644 --- 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 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 index d0046895987f0..9d9c54298cf11 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs +++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.rs @@ -193,6 +193,19 @@ fn test9_drop_order_and_nested_closures() { b(); } +// Test that we migrate if drop order of Vec 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 = || 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(); @@ -203,4 +216,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.stderr b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr index e9170eba3f176..fa1f83c37823f 100644 --- a/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr +++ b/src/test/ui/closures/2229_closure_analysis/migrations/significant_drop.stderr @@ -195,5 +195,22 @@ LL ~ let c = || { LL + let _ = &tuple; | -error: aborting due to 9 previous errors +error: changes to closure capture in Rust 2021 will affect drop order + --> $DIR/significant_drop.rs:201:18 + | +LL | let _c = || tup.0; + | ^^^----- + | | + | in Rust 2018, this closure captures all of `tup`, but in Rust 2021, it will only capture `tup.0` +... +LL | } + | - in Rust 2018, `tup` is dropped here, but in Rust 2021, only `tup.0` will be dropped here as part of the closure + | + = note: for more information, see +help: add a dummy let to cause `tup` to be fully captured + | +LL | let _c = || { let _ = &tup; tup.0 }; + | +++++++++++++++ + + +error: aborting due to 10 previous errors