mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #84152 - sexxi-goose:insignificant_dtor, r=nikomatsakis

Browse Source 
Insignificant destructors rfc 2229

- Adds new attribute `rustc_insignificant_dtor` to annotate the drop method.
- Adds a query to check if a type has a significant drop.
- Updates closure analysis to check for significant drops rather than just drop.

A type marked with the attribute `rustc_insignificant_dtor` is considered to not be significant. A drop is significant if it is implemented by the user or does anything that will have any observable behavior (other than freeing up memory).

https://github.com/rust-lang/project-rfc-2229/issues/35
This commit is contained in:
bors 2021-05-15 10:00:39 +00:00
commit b439be03c9
11 changed files with 333 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
compiler/rustc_feature/src/active.rs
View File

@ -700,6 +700,7 @@ pub const INCOMPLETE_FEATURES: &[Symbol] = &[
sym::native_link_modifiers_verbatim,
sym::native_link_modifiers_whole_archive,
sym::native_link_modifiers_as_needed,
sym::rustc_insignificant_dtor,
];
/// Some features are not allowed to be used together at the same time, if

1
compiler/rustc_feature/src/builtin_attrs.rs
View File

@ -556,6 +556,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
rustc_attr!(TEST, rustc_outlives, Normal, template!(Word)),
rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word)),
rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word)),
rustc_attr!(TEST, rustc_variance, Normal, template!(Word)),
rustc_attr!(TEST, rustc_layout, Normal, template!(List: "field1, field2, ...")),
rustc_attr!(TEST, rustc_regions, Normal, template!(Word)),

15
compiler/rustc_middle/src/query/mod.rs
View File

@ -1035,6 +1035,10 @@ rustc_queries! {
query needs_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
desc { "computing whether `{}` needs drop", env.value }
}
/// Query backing `TyS::has_significant_drop_raw`.
query has_significant_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
desc { "computing whether `{}` has a significant drop", env.value }
}
/// Query backing `TyS::is_structural_eq_shallow`.
///
@ -1055,6 +1059,17 @@ rustc_queries! {
cache_on_disk_if { true }
}
/// A list of types where the ADT requires drop if and only if any of those types
/// has significant drop. A type marked with the attribute `rustc_insignificant_dtor`
/// is considered to not be significant. A drop is significant if it is implemented
/// by the user or does anything that will have any observable behavior (other than
/// freeing up memory). If the ADT is known to have a significant destructor then
/// `Err(AlwaysRequiresDrop)` is returned.
query adt_significant_drop_tys(def_id: DefId) -> Result<&'tcx ty::List<Ty<'tcx>>, AlwaysRequiresDrop> {
desc { |tcx| "computing when `{}` has a significant destructor", tcx.def_path_str(def_id) }
cache_on_disk_if { false }
}
query layout_raw(
env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>
) -> Result<&'tcx rustc_target::abi::Layout, ty::layout::LayoutError<'tcx>> {

33
compiler/rustc_middle/src/ty/util.rs
View File

@ -791,6 +791,39 @@ impl<'tcx> ty::TyS<'tcx> {
}
}
/// Checks if `ty` has has a significant drop.
///
/// Note that this method can return false even if `ty` has a destructor
/// attached; even if that is the case then the adt has been marked with
/// the attribute `rustc_insignificant_dtor`.
///
/// Note that this method is used to check for change in drop order for
/// 2229 drop reorder migration analysis.
#[inline]
pub fn has_significant_drop(
&'tcx self,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> bool {
// Avoid querying in simple cases.
match needs_drop_components(self, &tcx.data_layout) {
Err(AlwaysRequiresDrop) => true,
Ok(components) => {
let query_ty = match *components {
[] => return false,
// If we've got a single component, call the query with that
// to increase the chance that we hit the query cache.
[component_ty] => component_ty,
_ => self,
};
// This doesn't depend on regions, so try to minimize distinct
// query keys used.
let erased = tcx.normalize_erasing_regions(param_env, query_ty);
tcx.has_significant_drop_raw(param_env.and(erased))
}
}
}
/// Returns `true` if equality for this type is both reflexive and structural.
///
/// Reflexive equality for a type is indicated by an `Eq` impl for that type.

1
compiler/rustc_span/src/symbol.rs
View File

@ -1015,6 +1015,7 @@ symbols! {
rustc_expected_cgu_reuse,
rustc_if_this_changed,
rustc_inherit_overflow_checks,
rustc_insignificant_dtor,
rustc_layout,
rustc_layout_scalar_valid_range_end,
rustc_layout_scalar_valid_range_start,

55
compiler/rustc_ty_utils/src/needs_drop.rs
View File

@ -6,7 +6,7 @@ use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::util::{needs_drop_components, AlwaysRequiresDrop};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::Limit;
use rustc_span::DUMMY_SP;
use rustc_span::{sym, DUMMY_SP};
type NeedsDropResult<T> = Result<T, AlwaysRequiresDrop>;
@ -21,6 +21,19 @@ fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>
res
}
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 res = NeedsDropTypes::new(tcx, query.param_env, query.value, significant_drop_fields)
.next()
.is_some();
debug!("has_significant_drop_raw({:?}) = {:?}", query, res);
res
}
struct NeedsDropTypes<'tcx, F> {
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
@ -155,12 +168,20 @@ where
}
}
fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
// This is a helper function for `adt_drop_tys` and `adt_significant_drop_tys`.
// 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<'_>,
def_id: DefId,
adt_has_dtor: impl Fn(&ty::AdtDef) -> bool,
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
let adt_components = move |adt_def: &ty::AdtDef| {
if adt_def.is_manually_drop() {
debug!("adt_drop_tys: `{:?}` is manually drop", adt_def);
return Ok(Vec::new().into_iter());
} else if adt_def.destructor(tcx).is_some() {
} else if adt_has_dtor(adt_def) {
debug!("adt_drop_tys: `{:?}` implements `Drop`", adt_def);
return Err(AlwaysRequiresDrop);
} else if adt_def.is_union() {
@ -179,6 +200,30 @@ fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, Alw
res.map(|components| tcx.intern_type_list(&components))
}
pub(crate) fn provide(providers: &mut ty::query::Providers) {
*providers = ty::query::Providers { needs_drop_raw, adt_drop_tys, ..*providers };
fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
let adt_has_dtor = |adt_def: &ty::AdtDef| adt_def.destructor(tcx).is_some();
adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
}
fn adt_significant_drop_tys(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
let adt_has_dtor = |adt_def: &ty::AdtDef| {
adt_def
.destructor(tcx)
.map(|dtor| !tcx.has_attr(dtor.did, sym::rustc_insignificant_dtor))
.unwrap_or(false)
};
adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
}
pub(crate) fn provide(providers: &mut ty::query::Providers) {
*providers = ty::query::Providers {
needs_drop_raw,
has_significant_drop_raw,
adt_drop_tys,
adt_significant_drop_tys,
..*providers
};
}

14
compiler/rustc_typeck/src/check/upvar.rs
View File

@ -706,6 +706,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// enabled, **and**
/// - It wasn't completely captured by the closure, **and**
/// - One of the paths starting at this root variable, that is not captured needs Drop.
///
/// This function only returns true for significant drops. A type is considerent to have a
/// significant drop if it's Drop implementation is not annotated by `rustc_insignificant_dtor`.
fn compute_2229_migrations_for_drop(
&self,
closure_def_id: DefId,
@ -716,7 +719,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> bool {
let ty = self.infcx.resolve_vars_if_possible(self.node_ty(var_hir_id));
if !ty.needs_drop(self.tcx, self.tcx.param_env(closure_def_id.expect_local())) {
if !ty.has_significant_drop(self.tcx, self.tcx.param_env(closure_def_id.expect_local())) {
return false;
}
@ -835,11 +838,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// using list of `Projection` slices), it returns true if there is a path that is not
/// captured starting at this root variable that implements Drop.
///
/// FIXME(project-rfc-2229#35): This should return true only for significant drops.
/// A drop is significant if it's implemented by the user or does
/// anything that will have any observable behavior (other than
/// freeing up memory).
///
/// The way this function works is at a given call it looks at type `base_path_ty` of some base
/// path say P and then list of projection slices which represent the different captures moved
/// into the closure starting off of P.
@ -895,7 +893,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// (Ty((w.p).x), [ &[] ]) (Ty((w.p).y), []) // IMP 2
/// | |
/// v v
/// false NeedsDrop(Ty(w.p.y))
/// false NeedsSignificantDrop(Ty(w.p.y))
/// |
/// v
/// true
@ -939,7 +937,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
captured_by_move_projs: Vec<&[Projection<'tcx>]>,
) -> bool {
let needs_drop = |ty: Ty<'tcx>| {
ty.needs_drop(self.tcx, self.tcx.param_env(closure_def_id.expect_local()))
ty.has_significant_drop(self.tcx, self.tcx.param_env(closure_def_id.expect_local()))
};
let is_drop_defined_for_ty = |ty: Ty<'tcx>| {

67
src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.fixed Normal file
View File

@ -0,0 +1,67 @@
// run-rustfix
#![deny(disjoint_capture_migration)]
//~^ NOTE: the lint level is defined here
#![feature(rustc_attrs)]
#![allow(unused)]
struct InsignificantDropPoint {
x: i32,
y: i32,
}
impl Drop for InsignificantDropPoint {
#[rustc_insignificant_dtor]
fn drop(&mut self) {}
}
struct SigDrop;
impl Drop for SigDrop {
fn drop(&mut self) {}
}
struct GenericStruct<T>(T, T);
struct Wrapper<T>(GenericStruct<T>, i32);
impl<T> Drop for GenericStruct<T> {
#[rustc_insignificant_dtor]
fn drop(&mut self) {}
}
// `SigDrop` implements drop and therefore needs to be migrated.
fn significant_drop_needs_migration() {
let t = (SigDrop {}, SigDrop {});
let c = || { let _ = &t;
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
//~| HELP: add a dummy let to cause `t` to be fully captured
let _t = t.0;
};
c();
}
// Even if a type implements an insignificant drop, if it's
// elements have a significant drop then the overall type is
// 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 {}, SigDrop {}), 5);
// move is used to force i32 to be copied instead of being a ref
let c = move || { let _ = &t;
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
//~| HELP: add a dummy let to cause `t` to be fully captured
let _t = t.1;
};
c();
}
fn main() {
significant_drop_needs_migration();
generic_struct_with_significant_drop_needs_migration();
}

67
src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.rs Normal file
View File

@ -0,0 +1,67 @@
// run-rustfix
#![deny(disjoint_capture_migration)]
//~^ NOTE: the lint level is defined here
#![feature(rustc_attrs)]
#![allow(unused)]
struct InsignificantDropPoint {
x: i32,
y: i32,
}
impl Drop for InsignificantDropPoint {
#[rustc_insignificant_dtor]
fn drop(&mut self) {}
}
struct SigDrop;
impl Drop for SigDrop {
fn drop(&mut self) {}
}
struct GenericStruct<T>(T, T);
struct Wrapper<T>(GenericStruct<T>, i32);
impl<T> Drop for GenericStruct<T> {
#[rustc_insignificant_dtor]
fn drop(&mut self) {}
}
// `SigDrop` implements drop and therefore needs to be migrated.
fn significant_drop_needs_migration() {
let t = (SigDrop {}, SigDrop {});
let c = || {
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
//~| HELP: add a dummy let to cause `t` to be fully captured
let _t = t.0;
};
c();
}
// Even if a type implements an insignificant drop, if it's
// elements have a significant drop then the overall type is
// 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 {}, SigDrop {}), 5);
// move is used to force i32 to be copied instead of being a ref
let c = move || {
//~^ ERROR: drop order affected for closure because of `capture_disjoint_fields`
//~| HELP: add a dummy let to cause `t` to be fully captured
let _t = t.1;
};
c();
}
fn main() {
significant_drop_needs_migration();
generic_struct_with_significant_drop_needs_migration();
}

47
src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_migrations.stderr Normal file
View File

@ -0,0 +1,47 @@
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/insignificant_drop_attr_migrations.rs:38:13
|
LL | let c = || {
| _____________^
LL | |
LL | |
LL | | let _t = t.0;
LL | | };
| |_____^
|
note: the lint level is defined here
--> $DIR/insignificant_drop_attr_migrations.rs:3:9
|
LL | #![deny(disjoint_capture_migration)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: add a dummy let to cause `t` to be fully captured
|
LL | let c = || { let _ = &t;
LL |
LL |
LL | let _t = t.0;
LL | };
|
error: drop order affected for closure because of `capture_disjoint_fields`
--> $DIR/insignificant_drop_attr_migrations.rs:55:13
|
LL | let c = move || {
| _____________^
LL | |
LL | |
LL | | let _t = t.1;
LL | | };
| |_____^
|
help: add a dummy let to cause `t` to be fully captured
|
LL | let c = move || { let _ = &t;
LL |
LL |
LL | let _t = t.1;
LL | };
|
error: aborting due to 2 previous errors

45
src/test/ui/closures/2229_closure_analysis/migrations/insignificant_drop_attr_no_migrations.rs Normal file
View File

@ -0,0 +1,45 @@
// run-pass
#![deny(disjoint_capture_migration)]
#![feature(rustc_attrs)]
#![allow(unused)]
struct InsignificantDropPoint {
x: i32,
y: i32,
}
impl Drop for InsignificantDropPoint {
#[rustc_insignificant_dtor]
fn drop(&mut self) {}
}
struct GenericStruct<T>(T, T);
// No drop reordering is required as the elements of `t` implement insignificant drop
fn insignificant_drop_does_not_need_migration() {
let t = (InsignificantDropPoint { x: 4, y: 9 }, InsignificantDropPoint { x: 4, y: 9 });
let c = || {
let _t = t.0;
};
c();
}
// Generic struct whose elements don't have significant drops don't need drop reordering
fn generic_struct_with_insignificant_drop_does_not_need_migration() {
let t =
GenericStruct(InsignificantDropPoint { x: 4, y: 9 }, InsignificantDropPoint { x: 4, y: 9 });
let c = || {
let _t = t.0;
};
c();
}
fn main() {
insignificant_drop_does_not_need_migration();
generic_struct_with_insignificant_drop_does_not_need_migration();
}