Skip to content

Commit 945cf4f

Browse files
borsehuss
bors
authored and
ehuss
committed
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`
1 parent f8e5fda commit 945cf4f

31 files changed

+307
-545
lines changed

Diff for: ‎compiler/rustc_ty_utils/src/needs_drop.rs

+58-18
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use rustc_data_structures::fx::FxHashSet;
44
use rustc_hir::def_id::DefId;
55
use rustc_middle::ty::subst::Subst;
6+
use rustc_middle::ty::subst::SubstsRef;
67
use rustc_middle::ty::util::{needs_drop_components, AlwaysRequiresDrop};
78
use rustc_middle::ty::{self, Ty, TyCtxt};
89
use rustc_session::Limit;
@@ -12,7 +13,8 @@ type NeedsDropResult<T> = Result<T, AlwaysRequiresDrop>;
1213

1314
fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
1415
let adt_fields =
15-
move |adt_def: &ty::AdtDef| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter());
16+
move |adt_def: &ty::AdtDef, _| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter());
17+
1618
// If we don't know a type doesn't need drop, for example if it's a type
1719
// parameter without a `Copy` bound, then we conservatively return that it
1820
// needs drop.
@@ -25,8 +27,9 @@ fn has_significant_drop_raw<'tcx>(
2527
tcx: TyCtxt<'tcx>,
2628
query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
2729
) -> bool {
28-
let significant_drop_fields =
29-
move |adt_def: &ty::AdtDef| tcx.adt_significant_drop_tys(adt_def.did).map(|tys| tys.iter());
30+
let significant_drop_fields = move |adt_def: &ty::AdtDef, _| {
31+
tcx.adt_significant_drop_tys(adt_def.did).map(|tys| tys.iter())
32+
};
3033
let res = NeedsDropTypes::new(tcx, query.param_env, query.value, significant_drop_fields)
3134
.next()
3235
.is_some();
@@ -71,7 +74,7 @@ impl<'tcx, F> NeedsDropTypes<'tcx, F> {
7174

7275
impl<'tcx, F, I> Iterator for NeedsDropTypes<'tcx, F>
7376
where
74-
F: Fn(&ty::AdtDef) -> NeedsDropResult<I>,
77+
F: Fn(&ty::AdtDef, SubstsRef<'tcx>) -> NeedsDropResult<I>,
7578
I: Iterator<Item = Ty<'tcx>>,
7679
{
7780
type Item = NeedsDropResult<Ty<'tcx>>;
@@ -135,7 +138,7 @@ where
135138
// `ManuallyDrop`. If it's a struct or enum without a `Drop`
136139
// impl then check whether the field types need `Drop`.
137140
ty::Adt(adt_def, substs) => {
138-
let tys = match (self.adt_components)(adt_def) {
141+
let tys = match (self.adt_components)(adt_def, substs) {
139142
Err(e) => return Some(Err(e)),
140143
Ok(tys) => tys,
141144
};
@@ -168,22 +171,44 @@ where
168171
}
169172
}
170173

174+
enum DtorType {
175+
/// Type has a `Drop` but it is considered insignificant.
176+
/// Check the query `adt_significant_drop_tys` for understanding
177+
/// "significant" / "insignificant".
178+
Insignificant,
179+
180+
/// Type has a `Drop` implentation.
181+
Significant,
182+
}
183+
171184
// This is a helper function for `adt_drop_tys` and `adt_significant_drop_tys`.
172185
// Depending on the implentation of `adt_has_dtor`, it is used to check if the
173186
// ADT has a destructor or if the ADT only has a significant destructor. For
174187
// understanding significant destructor look at `adt_significant_drop_tys`.
175-
fn adt_drop_tys_helper(
176-
tcx: TyCtxt<'_>,
188+
fn adt_drop_tys_helper<'tcx>(
189+
tcx: TyCtxt<'tcx>,
177190
def_id: DefId,
178-
adt_has_dtor: impl Fn(&ty::AdtDef) -> bool,
179-
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
180-
let adt_components = move |adt_def: &ty::AdtDef| {
191+
adt_has_dtor: impl Fn(&ty::AdtDef) -> Option<DtorType>,
192+
) -> Result<&ty::List<Ty<'tcx>>, AlwaysRequiresDrop> {
193+
let adt_components = move |adt_def: &ty::AdtDef, substs: SubstsRef<'tcx>| {
181194
if adt_def.is_manually_drop() {
182195
debug!("adt_drop_tys: `{:?}` is manually drop", adt_def);
183196
return Ok(Vec::new().into_iter());
184-
} else if adt_has_dtor(adt_def) {
185-
debug!("adt_drop_tys: `{:?}` implements `Drop`", adt_def);
186-
return Err(AlwaysRequiresDrop);
197+
} else if let Some(dtor_info) = adt_has_dtor(adt_def) {
198+
match dtor_info {
199+
DtorType::Significant => {
200+
debug!("adt_drop_tys: `{:?}` implements `Drop`", adt_def);
201+
return Err(AlwaysRequiresDrop);
202+
}
203+
DtorType::Insignificant => {
204+
debug!("adt_drop_tys: `{:?}` drop is insignificant", adt_def);
205+
206+
// Since the destructor is insignificant, we just want to make sure all of
207+
// the passed in type parameters are also insignificant.
208+
// Eg: Vec<T> dtor is insignificant when T=i32 but significant when T=Mutex.
209+
return Ok(substs.types().collect::<Vec<Ty<'_>>>().into_iter());
210+
}
211+
}
187212
} else if adt_def.is_union() {
188213
debug!("adt_drop_tys: `{:?}` is a union", adt_def);
189214
return Ok(Vec::new().into_iter());
@@ -201,7 +226,10 @@ fn adt_drop_tys_helper(
201226
}
202227

203228
fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
204-
let adt_has_dtor = |adt_def: &ty::AdtDef| adt_def.destructor(tcx).is_some();
229+
// This is for the "needs_drop" query, that considers all `Drop` impls, therefore all dtors are
230+
// significant.
231+
let adt_has_dtor =
232+
|adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
205233
adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
206234
}
207235

@@ -210,10 +238,22 @@ fn adt_significant_drop_tys(
210238
def_id: DefId,
211239
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
212240
let adt_has_dtor = |adt_def: &ty::AdtDef| {
213-
adt_def
214-
.destructor(tcx)
215-
.map(|dtor| !tcx.has_attr(dtor.did, sym::rustc_insignificant_dtor))
216-
.unwrap_or(false)
241+
let is_marked_insig = tcx.has_attr(adt_def.did, sym::rustc_insignificant_dtor);
242+
if is_marked_insig {
243+
// In some cases like `std::collections::HashMap` where the struct is a wrapper around
244+
// a type that is a Drop type, and the wrapped type (eg: `hashbrown::HashMap`) lies
245+
// outside stdlib, we might choose to still annotate the the wrapper (std HashMap) with
246+
// `rustc_insignificant_dtor`, even if the type itself doesn't have a `Drop` impl.
247+
Some(DtorType::Insignificant)
248+
} else if adt_def.destructor(tcx).is_some() {
249+
// There is a Drop impl and the type isn't marked insignificant, therefore Drop must be
250+
// significant.
251+
Some(DtorType::Significant)
252+
} else {
253+
// No destructor found nor the type is annotated with `rustc_insignificant_dtor`, we
254+
// treat this as the simple case of Drop impl for type.
255+
None
256+
}
217257
};
218258
adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
219259
}

Diff for: ‎library/alloc/src/collections/btree/map.rs

+2
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ pub(super) const MIN_LEN: usize = node::MIN_LEN_AFTER_SPLIT;
153153
/// ```
154154
#[stable(feature = "rust1", since = "1.0.0")]
155155
#[cfg_attr(not(test), rustc_diagnostic_item = "BTreeMap")]
156+
#[rustc_insignificant_dtor]
156157
pub struct BTreeMap<K, V> {
157158
root: Option<Root<K, V>>,
158159
length: usize,
@@ -328,6 +329,7 @@ impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for IterMut<'_, K, V> {
328329
///
329330
/// [`into_iter`]: IntoIterator::into_iter
330331
#[stable(feature = "rust1", since = "1.0.0")]
332+
#[rustc_insignificant_dtor]
331333
pub struct IntoIter<K, V> {
332334
range: LazyLeafRange<marker::Dying, K, V>,
333335
length: usize,

Diff for: ‎library/alloc/src/collections/linked_list.rs

+1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ mod tests;
4343
/// more memory efficient, and make better use of CPU cache.
4444
#[stable(feature = "rust1", since = "1.0.0")]
4545
#[cfg_attr(not(test), rustc_diagnostic_item = "LinkedList")]
46+
#[rustc_insignificant_dtor]
4647
pub struct LinkedList<T> {
4748
head: Option<NonNull<Node<T>>>,
4849
tail: Option<NonNull<Node<T>>>,

Diff for: ‎library/alloc/src/collections/vec_deque/mod.rs

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

Diff for: ‎library/alloc/src/rc.rs

+1
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,7 @@ struct RcBox<T: ?Sized> {
305305
/// [get_mut]: Rc::get_mut
306306
#[cfg_attr(not(test), rustc_diagnostic_item = "Rc")]
307307
#[stable(feature = "rust1", since = "1.0.0")]
308+
#[rustc_insignificant_dtor]
308309
pub struct Rc<T: ?Sized> {
309310
ptr: NonNull<RcBox<T>>,
310311
phantom: PhantomData<RcBox<T>>,

Diff for: ‎library/alloc/src/vec/into_iter.rs

+1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use core::slice::{self};
2222
/// let iter: std::vec::IntoIter<_> = v.into_iter();
2323
/// ```
2424
#[stable(feature = "rust1", since = "1.0.0")]
25+
#[rustc_insignificant_dtor]
2526
pub struct IntoIter<
2627
T,
2728
#[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global,

Diff for: ‎library/alloc/src/vec/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@ mod spec_extend;
394394
/// [owned slice]: Box
395395
#[stable(feature = "rust1", since = "1.0.0")]
396396
#[cfg_attr(not(test), rustc_diagnostic_item = "vec_type")]
397+
#[rustc_insignificant_dtor]
397398
pub struct Vec<T, #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global> {
398399
buf: RawVec<T, A>,
399400
len: usize,

Diff for: ‎library/core/src/array/iter.rs

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::{
1010

1111
/// A by-value [array] iterator.
1212
#[stable(feature = "array_value_iter", since = "1.51.0")]
13+
#[cfg_attr(not(bootstrap), rustc_insignificant_dtor)]
1314
pub struct IntoIter<T, const N: usize> {
1415
/// This is the array we are iterating over.
1516
///

Diff for: ‎library/std/src/collections/hash/map.rs

+1
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ use crate::sys;
205205
206206
#[cfg_attr(not(test), rustc_diagnostic_item = "hashmap_type")]
207207
#[stable(feature = "rust1", since = "1.0.0")]
208+
#[cfg_attr(not(bootstrap), rustc_insignificant_dtor)]
208209
pub struct HashMap<K, V, S = RandomState> {
209210
base: base::HashMap<K, V, S>,
210211
}

Diff for: ‎src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.fixed

+11-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@
44

55
use std::thread;
66

7+
#[derive(Debug)]
8+
struct Foo(i32);
9+
impl Drop for Foo {
10+
fn drop(&mut self) {
11+
println!("{:?} dropped", self.0);
12+
}
13+
}
14+
715
/* Test Send Trait Migration */
816
struct SendPointer(*mut i32);
917
unsafe impl Send for SendPointer {}
@@ -42,19 +50,19 @@ fn test_sync_trait() {
4250
}
4351

4452
/* Test Clone Trait Migration */
45-
struct S(String);
53+
struct S(Foo);
4654
struct T(i32);
4755

4856
struct U(S, T);
4957

5058
impl Clone for U {
5159
fn clone(&self) -> Self {
52-
U(S(String::from("Hello World")), T(0))
60+
U(S(Foo(0)), T(0))
5361
}
5462
}
5563

5664
fn test_clone_trait() {
57-
let f = U(S(String::from("Hello World")), T(0));
65+
let f = U(S(Foo(0)), T(0));
5866
let c = || {
5967
let _ = &f;
6068
//~^ ERROR: `Clone` trait implementation for closure and drop order

Diff for: ‎src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.rs

+11-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@
44

55
use std::thread;
66

7+
#[derive(Debug)]
8+
struct Foo(i32);
9+
impl Drop for Foo {
10+
fn drop(&mut self) {
11+
println!("{:?} dropped", self.0);
12+
}
13+
}
14+
715
/* Test Send Trait Migration */
816
struct SendPointer(*mut i32);
917
unsafe impl Send for SendPointer {}
@@ -42,19 +50,19 @@ fn test_sync_trait() {
4250
}
4351

4452
/* Test Clone Trait Migration */
45-
struct S(String);
53+
struct S(Foo);
4654
struct T(i32);
4755

4856
struct U(S, T);
4957

5058
impl Clone for U {
5159
fn clone(&self) -> Self {
52-
U(S(String::from("Hello World")), T(0))
60+
U(S(Foo(0)), T(0))
5361
}
5462
}
5563

5664
fn test_clone_trait() {
57-
let f = U(S(String::from("Hello World")), T(0));
65+
let f = U(S(Foo(0)), T(0));
5866
let c = || {
5967
//~^ ERROR: `Clone` trait implementation for closure and drop order
6068
//~| 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`

Diff for: ‎src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: changes to closure capture in Rust 2021 will affect `Send` trait implementation for closure
2-
--> $DIR/auto_traits.rs:14:19
2+
--> $DIR/auto_traits.rs:22:19
33
|
44
LL | thread::spawn(move || unsafe {
55
| ^^^^^^^^^^^^^^ 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;
2424
...
2525

2626
error: changes to closure capture in Rust 2021 will affect `Sync`, `Send` trait implementation for closure
27-
--> $DIR/auto_traits.rs:34:19
27+
--> $DIR/auto_traits.rs:42:19
2828
|
2929
LL | thread::spawn(move || unsafe {
3030
| ^^^^^^^^^^^^^^ 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;
4444
...
4545

4646
error: changes to closure capture in Rust 2021 will affect `Clone` trait implementation for closure and drop order
47-
--> $DIR/auto_traits.rs:58:13
47+
--> $DIR/auto_traits.rs:66:13
4848
|
4949
LL | let c = || {
5050
| ^^ 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`

Diff for: ‎src/test/ui/closures/2229_closure_analysis/migrations/closure-body-macro-fragment.fixed

+10-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@
33
// check-pass
44
#![warn(rust_2021_compatibility)]
55

6+
#[derive(Debug)]
7+
struct Foo(i32);
8+
impl Drop for Foo {
9+
fn drop(&mut self) {
10+
println!("{:?} dropped", self.0);
11+
}
12+
}
13+
614
macro_rules! m {
715
(@ $body:expr) => {{
816
let f = || $body;
@@ -15,11 +23,11 @@ macro_rules! m {
1523
}
1624

1725
fn main() {
18-
let a = (1.to_string(), 2.to_string());
26+
let a = (Foo(0), Foo(1));
1927
m!({
2028
let _ = &a;
2129
//~^ HELP: add a dummy
2230
let x = a.0;
23-
println!("{}", x);
31+
println!("{:?}", x);
2432
});
2533
}

Diff for: ‎src/test/ui/closures/2229_closure_analysis/migrations/closure-body-macro-fragment.rs

+10-2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@
33
// check-pass
44
#![warn(rust_2021_compatibility)]
55

6+
#[derive(Debug)]
7+
struct Foo(i32);
8+
impl Drop for Foo {
9+
fn drop(&mut self) {
10+
println!("{:?} dropped", self.0);
11+
}
12+
}
13+
614
macro_rules! m {
715
(@ $body:expr) => {{
816
let f = || $body;
@@ -15,10 +23,10 @@ macro_rules! m {
1523
}
1624

1725
fn main() {
18-
let a = (1.to_string(), 2.to_string());
26+
let a = (Foo(0), Foo(1));
1927
m!({
2028
//~^ HELP: add a dummy
2129
let x = a.0;
22-
println!("{}", x);
30+
println!("{:?}", x);
2331
});
2432
}

Diff for: ‎src/test/ui/closures/2229_closure_analysis/migrations/closure-body-macro-fragment.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
warning: changes to closure capture in Rust 2021 will affect drop order
2-
--> $DIR/closure-body-macro-fragment.rs:8:17
2+
--> $DIR/closure-body-macro-fragment.rs:16:17
33
|
44
LL | let f = || $body;
55
| _________________^
@@ -15,7 +15,7 @@ LL | / m!({
1515
LL | |
1616
LL | | let x = a.0;
1717
| | --- in Rust 2018, this closure captures all of `a`, but in Rust 2021, it will only capture `a.0`
18-
LL | | println!("{}", x);
18+
LL | | println!("{:?}", x);
1919
LL | | });
2020
| |_______- in this macro invocation
2121
|

0 commit comments

Comments
 (0)