From d2cbe217566de4c95685316c7d59aa2823868a53 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Tue, 21 Sep 2021 05:06:19 -0400 Subject: [PATCH] Handle type params in insig dtors --- compiler/rustc_ty_utils/src/needs_drop.rs | 26 ++++++---- .../insignificant_drop_attr_migrations.fixed | 10 ++-- .../insignificant_drop_attr_migrations.stderr | 52 ++++++++++++++----- 3 files changed, 61 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_ty_utils/src/needs_drop.rs b/compiler/rustc_ty_utils/src/needs_drop.rs index cbbe8f7fffbbf..039cbaf4982ee 100644 --- a/compiler/rustc_ty_utils/src/needs_drop.rs +++ b/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 = Result; 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, + F: Fn(&ty::AdtDef, SubstsRef<'tcx>) -> NeedsDropResult, I: Iterator>, { type Item = NeedsDropResult>; @@ -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, }; @@ -185,12 +187,12 @@ enum DtorType { // 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) -> Option, -) -> Result<&ty::List>, AlwaysRequiresDrop> { - let adt_components = move |adt_def: &ty::AdtDef| { +) -> Result<&ty::List>, 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()); @@ -202,7 +204,11 @@ fn adt_drop_tys_helper( } DtorType::Insignificant => { debug!("adt_drop_tys: `{:?}` drop is insignificant", adt_def); - return Ok(Vec::new().into_iter()); + + // Since the destructor is insignificant, we just want to make sure all of + // the passed in type parameters are also insignificant. + // Eg: Vec dtor is insignificant when T=i32 but significant when T=Mutex. + return Ok(substs.types().collect::>>().into_iter()); } } } else if adt_def.is_union() { 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 0787ac7c77b53..d0fad7b07df0e 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 @@ -9,7 +9,7 @@ struct InsignificantDropPoint { x: i32, - y: Mutex, + y: Mutex, } impl Drop for InsignificantDropPoint { @@ -17,7 +17,7 @@ impl Drop for InsignificantDropPoint { fn drop(&mut self) {} } -struct SigDrop { x: () } +struct SigDrop; impl Drop for SigDrop { fn drop(&mut self) {} @@ -45,9 +45,10 @@ fn insign_dtor() { // `SigDrop` implements drop and therefore needs to be migrated. fn significant_drop_needs_migration() { - let t = (SigDrop { x: () }, SigDrop { x: () }); + let t = (SigDrop {}, SigDrop {}); let c = || { + let _ = &t; //~^ ERROR: drop order //~| NOTE: for more information, see //~| HELP: add a dummy let to cause `t` to be fully captured @@ -64,10 +65,11 @@ fn significant_drop_needs_migration() { // consdered to have an significant drop. Since the elements // of `GenericStruct` implement drop, migration is required. fn generic_struct_with_significant_drop_needs_migration() { - let t = Wrapper(GenericStruct(SigDrop { x: () }, SigDrop { x: () }), 5); + let t = Wrapper(GenericStruct(SigDrop {}, SigDrop {}), 5); // move is used to force i32 to be copied instead of being a ref let c = move || { + let _ = &t; //~^ ERROR: drop order //~| NOTE: for more information, see //~| HELP: add a dummy let to cause `t` to be fully captured 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 80289d6289dde..c20bd572af874 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,19 +1,45 @@ -error[E0107]: missing generics for struct `Mutex` - --> $DIR/insignificant_drop_attr_migrations.rs:12:8 +error: changes to closure capture in Rust 2021 will affect drop order + --> $DIR/insignificant_drop_attr_migrations.rs:50:13 | -LL | y: Mutex, - | ^^^^^ expected 1 generic argument +LL | let c = || { + | ^^ +... +LL | let _t = t.0; + | --- in Rust 2018, this closure captures all of `t`, but in Rust 2021, it will only capture `t.0` +... +LL | } + | - in Rust 2018, `t` is dropped here, but in Rust 2021, only `t.0` will be dropped here as part of the closure | -note: struct defined here, with 1 generic parameter: `T` - --> $SRC_DIR/std/src/sync/mutex.rs:LL:COL +note: the lint level is defined here + --> $DIR/insignificant_drop_attr_migrations.rs:3:9 | -LL | pub struct Mutex { - | ^^^^^ - -help: add missing generic argument +LL | #![deny(rust_2021_incompatible_closure_captures)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: for more information, see +help: add a dummy let to cause `t` to be fully captured + | +LL ~ let c = || { +LL + let _ = &t; + | + +error: changes to closure capture in Rust 2021 will affect drop order + --> $DIR/insignificant_drop_attr_migrations.rs:70:13 + | +LL | let c = move || { + | ^^^^^^^ +... +LL | let _t = t.1; + | --- in Rust 2018, this closure captures all of `t`, but in Rust 2021, it will only capture `t.1` +... +LL | } + | - in Rust 2018, `t` is dropped here, but in Rust 2021, only `t.1` will be dropped here as part of the closure + | + = note: for more information, see +help: add a dummy let to cause `t` to be fully captured + | +LL ~ let c = move || { +LL + let _ = &t; | -LL | y: Mutex, - | ~~~~~~~~ -error: aborting due to previous error +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0107`.