Skip to content

Commit 05044c2

Browse files
borsbors
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`
2 parents c09d637 + 994793f commit 05044c2

31 files changed

+306
-545
lines changed

‎compiler/rustc_ty_utils/src/needs_drop.rs

Lines changed: 57 additions & 18 deletions
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,7 @@ 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_components =
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());
1617

1718
// If we don't know a type doesn't need drop, for example if it's a type
1819
// parameter without a `Copy` bound, then we conservatively return that it
@@ -28,8 +29,9 @@ fn has_significant_drop_raw<'tcx>(
2829
tcx: TyCtxt<'tcx>,
2930
query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
3031
) -> bool {
31-
let significant_drop_fields =
32-
move |adt_def: &ty::AdtDef| tcx.adt_significant_drop_tys(adt_def.did).map(|tys| tys.iter());
32+
let significant_drop_fields = move |adt_def: &ty::AdtDef, _| {
33+
tcx.adt_significant_drop_tys(adt_def.did).map(|tys| tys.iter())
34+
};
3335
let res = NeedsDropTypes::new(tcx, query.param_env, query.value, significant_drop_fields)
3436
.next()
3537
.is_some();
@@ -74,7 +76,7 @@ impl<'tcx, F> NeedsDropTypes<'tcx, F> {
7476

7577
impl<'tcx, F, I> Iterator for NeedsDropTypes<'tcx, F>
7678
where
77-
F: Fn(&ty::AdtDef) -> NeedsDropResult<I>,
79+
F: Fn(&ty::AdtDef, SubstsRef<'tcx>) -> NeedsDropResult<I>,
7880
I: Iterator<Item = Ty<'tcx>>,
7981
{
8082
type Item = NeedsDropResult<Ty<'tcx>>;
@@ -138,7 +140,7 @@ where
138140
// `ManuallyDrop`. If it's a struct or enum without a `Drop`
139141
// impl then check whether the field types need `Drop`.
140142
ty::Adt(adt_def, substs) => {
141-
let tys = match (self.adt_components)(adt_def) {
143+
let tys = match (self.adt_components)(adt_def, substs) {
142144
Err(e) => return Some(Err(e)),
143145
Ok(tys) => tys,
144146
};
@@ -171,22 +173,44 @@ where
171173
}
172174
}
173175

176+
enum DtorType {
177+
/// Type has a `Drop` but it is considered insignificant.
178+
/// Check the query `adt_significant_drop_tys` for understanding
179+
/// "significant" / "insignificant".
180+
Insignificant,
181+
182+
/// Type has a `Drop` implentation.
183+
Significant,
184+
}
185+
174186
// This is a helper function for `adt_drop_tys` and `adt_significant_drop_tys`.
175187
// Depending on the implentation of `adt_has_dtor`, it is used to check if the
176188
// ADT has a destructor or if the ADT only has a significant destructor. For
177189
// understanding significant destructor look at `adt_significant_drop_tys`.
178-
fn adt_drop_tys_helper(
179-
tcx: TyCtxt<'_>,
190+
fn adt_drop_tys_helper<'tcx>(
191+
tcx: TyCtxt<'tcx>,
180192
def_id: DefId,
181-
adt_has_dtor: impl Fn(&ty::AdtDef) -> bool,
182-
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
183-
let adt_components = move |adt_def: &ty::AdtDef| {
193+
adt_has_dtor: impl Fn(&ty::AdtDef) -> Option<DtorType>,
194+
) -> Result<&ty::List<Ty<'tcx>>, AlwaysRequiresDrop> {
195+
let adt_components = move |adt_def: &ty::AdtDef, substs: SubstsRef<'tcx>| {
184196
if adt_def.is_manually_drop() {
185197
debug!("adt_drop_tys: `{:?}` is manually drop", adt_def);
186198
return Ok(Vec::new().into_iter());
187-
} else if adt_has_dtor(adt_def) {
188-
debug!("adt_drop_tys: `{:?}` implements `Drop`", adt_def);
189-
return Err(AlwaysRequiresDrop);
199+
} else if let Some(dtor_info) = adt_has_dtor(adt_def) {
200+
match dtor_info {
201+
DtorType::Significant => {
202+
debug!("adt_drop_tys: `{:?}` implements `Drop`", adt_def);
203+
return Err(AlwaysRequiresDrop);
204+
}
205+
DtorType::Insignificant => {
206+
debug!("adt_drop_tys: `{:?}` drop is insignificant", adt_def);
207+
208+
// Since the destructor is insignificant, we just want to make sure all of
209+
// the passed in type parameters are also insignificant.
210+
// Eg: Vec<T> dtor is insignificant when T=i32 but significant when T=Mutex.
211+
return Ok(substs.types().collect::<Vec<Ty<'_>>>().into_iter());
212+
}
213+
}
190214
} else if adt_def.is_union() {
191215
debug!("adt_drop_tys: `{:?}` is a union", adt_def);
192216
return Ok(Vec::new().into_iter());
@@ -204,7 +228,10 @@ fn adt_drop_tys_helper(
204228
}
205229

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

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

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ pub(super) const MIN_LEN: usize = node::MIN_LEN_AFTER_SPLIT;
155155
/// ```
156156
#[stable(feature = "rust1", since = "1.0.0")]
157157
#[cfg_attr(not(test), rustc_diagnostic_item = "BTreeMap")]
158+
#[rustc_insignificant_dtor]
158159
pub struct BTreeMap<K, V> {
159160
root: Option<Root<K, V>>,
160161
length: usize,
@@ -331,6 +332,7 @@ impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for IterMut<'_, K, V> {
331332
/// [`into_iter`]: IntoIterator::into_iter
332333
/// [`IntoIterator`]: core::iter::IntoIterator
333334
#[stable(feature = "rust1", since = "1.0.0")]
335+
#[rustc_insignificant_dtor]
334336
pub struct IntoIter<K, V> {
335337
range: LazyLeafRange<marker::Dying, K, V>,
336338
length: usize,

‎library/alloc/src/collections/linked_list.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ mod tests;
4646
/// [`VecDeque`]: super::vec_deque::VecDeque
4747
#[stable(feature = "rust1", since = "1.0.0")]
4848
#[cfg_attr(not(test), rustc_diagnostic_item = "LinkedList")]
49+
#[rustc_insignificant_dtor]
4950
pub struct LinkedList<T> {
5051
head: Option<NonNull<Node<T>>>,
5152
tail: Option<NonNull<Node<T>>>,

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

Lines changed: 1 addition & 0 deletions
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,

‎library/alloc/src/rc.rs

Lines changed: 1 addition & 0 deletions
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>>,

‎library/alloc/src/vec/into_iter.rs

Lines changed: 1 addition & 0 deletions
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,

‎library/alloc/src/vec/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,7 @@ mod spec_extend;
396396
/// [owned slice]: Box
397397
#[stable(feature = "rust1", since = "1.0.0")]
398398
#[cfg_attr(not(test), rustc_diagnostic_item = "vec_type")]
399+
#[rustc_insignificant_dtor]
399400
pub struct Vec<T, #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global> {
400401
buf: RawVec<T, A>,
401402
len: usize,

‎library/core/src/array/iter.rs

Lines changed: 1 addition & 0 deletions
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+
#[rustc_insignificant_dtor]
1314
pub struct IntoIter<T, const N: usize> {
1415
/// This is the array we are iterating over.
1516
///

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

Lines changed: 1 addition & 0 deletions
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+
#[rustc_insignificant_dtor]
208209
pub struct HashMap<K, V, S = RandomState> {
209210
base: base::HashMap<K, V, S>,
210211
}

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

Lines changed: 11 additions & 3 deletions
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

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

Lines changed: 11 additions & 3 deletions
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`

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

Lines changed: 3 additions & 3 deletions
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`

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

Lines changed: 10 additions & 2 deletions
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
}

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

Lines changed: 10 additions & 2 deletions
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
}

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

Lines changed: 2 additions & 2 deletions
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)