Skip to content

Commit

Permalink
Handle type params in insig dtors
Browse files Browse the repository at this point in the history
  • Loading branch information
arora-aman committed Sep 21, 2021
1 parent 1c98988 commit d2cbe21
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 27 deletions.
26 changes: 16 additions & 10 deletions compiler/rustc_ty_utils/src/needs_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -12,7 +13,7 @@ type NeedsDropResult<T> = Result<T, AlwaysRequiresDrop>;

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
Expand All @@ -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();
Expand Down Expand Up @@ -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<I>,
F: Fn(&ty::AdtDef, SubstsRef<'tcx>) -> NeedsDropResult<I>,
I: Iterator<Item = Ty<'tcx>>,
{
type Item = NeedsDropResult<Ty<'tcx>>;
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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<DtorType>,
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
let adt_components = move |adt_def: &ty::AdtDef| {
) -> Result<&ty::List<Ty<'tcx>>, 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());
Expand All @@ -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<T> dtor is insignificant when T=i32 but significant when T=Mutex.
return Ok(substs.types().collect::<Vec<Ty<'_>>>().into_iter());
}
}
} else if adt_def.is_union() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@

struct InsignificantDropPoint {
x: i32,
y: Mutex<T>,
y: Mutex<i32>,
}

impl Drop for InsignificantDropPoint {
#[rustc_insignificant_dtor]
fn drop(&mut self) {}
}

struct SigDrop { x: () }
struct SigDrop;

impl Drop for SigDrop {
fn drop(&mut self) {}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<T: ?Sized> {
| ^^^^^ -
help: add missing generic argument
LL | #![deny(rust_2021_incompatible_closure_captures)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html>
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 <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/disjoint-capture-in-closures.html>
help: add a dummy let to cause `t` to be fully captured
|
LL ~ let c = move || {
LL + let _ = &t;
|
LL | y: Mutex<T>,
| ~~~~~~~~

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0107`.

0 comments on commit d2cbe21

Please sign in to comment.