Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken behaviour for NoFinalize trait #39

Merged
merged 1 commit into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/rustc_codegen_cranelift/example/mini_core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,8 @@ impl<T> Deref for Box<T> {
}
}

impl<T: NoFinalize> NoFinalize for Box<T> {}

#[lang = "exchange_malloc"]
unsafe fn allocate(size: usize, _align: usize) -> *mut u8 {
libc::malloc(size)
Expand Down
117 changes: 72 additions & 45 deletions compiler/rustc_ty_utils/src/needs_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,32 +16,68 @@ fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>
// 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
// needs drop.
let res = NeedsDropTypes::new(tcx, query.param_env, query.value, adt_fields).next().is_some();
let res = NeedsDropTypes::new(
tcx,
query.param_env,
query.value,
adt_fields,
DropCheckType::NeedsDrop,
)
.next()
.is_some();
debug!("needs_drop_raw({:?}) = {:?}", query, res);
res
}

fn needs_finalizer_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
let adt_fields =
move |adt_def: &ty::AdtDef| tcx.adt_finalize_tys(adt_def.did).map(|tys| tys.iter());
let res = NeedsDropTypes::new(tcx, query.param_env, query.value, adt_fields).next().is_some();
debug!("needs_finalize_raw({:?}) = {:?}", query, res);
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();
let res = NeedsDropTypes::new(
tcx,
query.param_env,
query.value,
significant_drop_fields,
DropCheckType::NeedsDrop,
)
.next()
.is_some();
debug!("has_significant_drop_raw({:?}) = {:?}", query, res);
res
}

fn needs_finalizer_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
let finalizer_fields =
move |adt_def: &ty::AdtDef| tcx.adt_finalize_tys(adt_def.did).map(|tys| tys.iter());
let res = NeedsDropTypes::new(
tcx,
query.param_env,
query.value,
finalizer_fields,
DropCheckType::NeedsFinalize,
)
.next()
.is_some();
debug!("needs_finalize_raw({:?}) = {:?}", query, res);
res
}

enum DropCheckType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A maybe-silly question: will this enum only ever have two values? If so, is it a boolean in disguise? [The needs_finalizer check suggests it is, but I might be over-simplifying!]

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a bool check in disguise yes. That may change in future because newer versions of rustc seem to have introduced the notion of a significant dropck: same behaviour as dropck but some drop methods can be marked as insignificant with a #[insignificant_drop] attribute.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, OK, then let's keep the enum!

NeedsDrop,
NeedsFinalize,
}

impl DropCheckType {
fn needs_finalizer(&self) -> bool {
match self {
DropCheckType::NeedsDrop => false,
DropCheckType::NeedsFinalize => true,
}
}
}

struct NeedsDropTypes<'tcx, F> {
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
Expand All @@ -54,6 +90,10 @@ struct NeedsDropTypes<'tcx, F> {
unchecked_tys: Vec<(Ty<'tcx>, usize)>,
recursion_limit: Limit,
adt_components: F,
/// The `needs_drop` and `needs_finalizer` rules are subtly different. This
/// field lets us reuse the `NeedsDropTypes` mechanism without heavy
/// refactoring which would make keeping up to date with upstream a pain.
drop_check_type: DropCheckType,
}

impl<'tcx, F> NeedsDropTypes<'tcx, F> {
Expand All @@ -62,6 +102,7 @@ impl<'tcx, F> NeedsDropTypes<'tcx, F> {
param_env: ty::ParamEnv<'tcx>,
ty: Ty<'tcx>,
adt_components: F,
drop_check_type: DropCheckType,
) -> Self {
let mut seen_tys = FxHashSet::default();
seen_tys.insert(ty);
Expand All @@ -73,6 +114,7 @@ impl<'tcx, F> NeedsDropTypes<'tcx, F> {
unchecked_tys: vec![(ty, 0)],
recursion_limit: tcx.recursion_limit(),
adt_components,
drop_check_type,
}
}
}
Expand All @@ -98,6 +140,12 @@ where
return Some(Err(AlwaysRequiresDrop));
}

if self.drop_check_type.needs_finalizer()
&& ty.is_no_finalize_modulo_regions(tcx.at(DUMMY_SP), self.param_env)
{
return None;
}

let components = match needs_drop_components(ty, &tcx.data_layout) {
Err(e) => return Some(Err(e)),
Ok(components) => components,
Expand Down Expand Up @@ -184,6 +232,7 @@ fn adt_drop_tys_helper(
tcx: TyCtxt<'_>,
def_id: DefId,
adt_has_dtor: impl Fn(&ty::AdtDef) -> bool,
dropck_type: DropCheckType,
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
let adt_components = move |adt_def: &ty::AdtDef| {
if adt_def.is_manually_drop() {
Expand All @@ -202,45 +251,15 @@ fn adt_drop_tys_helper(
let adt_ty = tcx.type_of(def_id);
let param_env = tcx.param_env(def_id);
let res: Result<Vec<_>, _> =
NeedsDropTypes::new(tcx, param_env, adt_ty, adt_components).collect();
NeedsDropTypes::new(tcx, param_env, adt_ty, adt_components, dropck_type).collect();

debug!("adt_drop_tys(`{}`) = `{:?}`", tcx.def_path_str(def_id), res);
res.map(|components| tcx.intern_type_list(&components))
}

fn adt_finalize_tys(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
let adt_ty = tcx.type_of(def_id);
let param_env = tcx.param_env(def_id);

let adt_components = move |adt_def: &ty::AdtDef| {
if tcx.type_of(adt_def.did).is_no_finalize_modulo_regions(tcx.at(DUMMY_SP), param_env) {
debug!("adt_finalize_tys: `{:?}` implements `NoFinalize`", adt_def);
return Ok(Vec::new().into_iter());
} else if adt_def.is_manually_drop() {
debug!("adt_finalize_tys: `{:?}` is manually drop", adt_def);
return Ok(Vec::new().into_iter());
} else if adt_def.destructor(tcx).is_some() {
debug!("adt_finalize_tys: `{:?}` implements `Drop`", adt_def);
return Err(AlwaysRequiresDrop);
} else if adt_def.is_union() {
debug!("adt_finalize_tys: `{:?}` is a union", adt_def);
return Ok(Vec::new().into_iter());
}
Ok(adt_def.all_fields().map(|field| tcx.type_of(field.did)).collect::<Vec<_>>().into_iter())
};
let res: Result<Vec<_>, _> =
NeedsDropTypes::new(tcx, param_env, adt_ty, adt_components).collect();

debug!("adt_finalize_tys(`{}`) = `{:?}`", tcx.def_path_str(def_id), res);
res.map(|components| tcx.intern_type_list(&components))
}

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)
adt_drop_tys_helper(tcx, def_id, adt_has_dtor, DropCheckType::NeedsDrop)
}

fn adt_significant_drop_tys(
Expand All @@ -253,7 +272,15 @@ fn adt_significant_drop_tys(
.map(|dtor| !tcx.has_attr(dtor.did, sym::rustc_insignificant_dtor))
.unwrap_or(false)
};
adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
adt_drop_tys_helper(tcx, def_id, adt_has_dtor, DropCheckType::NeedsDrop)
}

fn adt_finalize_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, DropCheckType::NeedsFinalize)
}

pub(crate) fn provide(providers: &mut ty::query::Providers) {
Expand Down
7 changes: 7 additions & 0 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ use core::cmp::Ordering;
use core::convert::{From, TryFrom};
use core::fmt;
use core::future::Future;
use core::gc::NoFinalize;
use core::hash::{Hash, Hasher};
#[cfg(not(no_global_oom_handling))]
use core::iter::FromIterator;
Expand Down Expand Up @@ -1721,3 +1722,9 @@ impl<S: ?Sized + Stream + Unpin> Stream for Box<S> {
(**self).size_hint()
}
}

#[unstable(feature = "gc", issue = "none")]
impl<T: NoFinalize> NoFinalize for Box<T> {}

#[unstable(feature = "gc", issue = "none")]
impl<T: NoFinalize, A: Allocator> NoFinalize for Box<T, A> {}
3 changes: 3 additions & 0 deletions library/alloc/src/raw_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use core::alloc::LayoutError;
use core::cmp;
use core::gc::NoFinalize;
use core::intrinsics;
use core::mem::{self, ManuallyDrop, MaybeUninit};
use core::ops::Drop;
Expand Down Expand Up @@ -558,3 +559,5 @@ fn alloc_guard(alloc_size: usize) -> Result<(), TryReserveError> {
fn capacity_overflow() -> ! {
panic!("capacity overflow");
}

impl<T: NoFinalize, A: Allocator> NoFinalize for RawVec<T, A> {}
7 changes: 7 additions & 0 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ use core::cmp;
use core::cmp::Ordering;
use core::convert::TryFrom;
use core::fmt;
use core::gc::NoFinalize;
use core::hash::{Hash, Hasher};
use core::intrinsics::{arith_offset, assume};
use core::iter;
Expand Down Expand Up @@ -2711,6 +2712,12 @@ impl<T: Ord, A: Allocator> Ord for Vec<T, A> {
}
}

#[unstable(feature = "gc", issue = "none")]
impl<T: NoFinalize, A: Allocator> NoFinalize for Vec<T, A> {}

#[unstable(feature = "gc", issue = "none")]
impl<T: NoFinalize> NoFinalize for Vec<T> {}

#[stable(feature = "rust1", since = "1.0.0")]
unsafe impl<#[may_dangle] T, A: Allocator> Drop for Vec<T, A> {
fn drop(&mut self) {
Expand Down
23 changes: 23 additions & 0 deletions library/core/src/gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,26 @@ pub unsafe fn gc_layout<T>() -> Trace {

impl<T: ?Sized> !NoTrace for *mut T {}
impl<T: ?Sized> !NoTrace for *const T {}

mod impls {
use super::NoFinalize;

macro_rules! impl_nofinalize {
($($t:ty)*) => (
$(
#[unstable(feature = "gc", issue = "none")]
impl NoFinalize for $t {}
)*
)
}

impl_nofinalize! {
usize u8 u16 u32 u64 u128
isize i8 i16 i32 i64 i128
f32 f64
bool char
}

#[unstable(feature = "never_type", issue = "35121")]
impl NoFinalize for ! {}
}
4 changes: 4 additions & 0 deletions library/core/src/tuple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use crate::cmp::Ordering::*;
use crate::cmp::*;
use crate::gc::NoFinalize;

// macro for implementing n-ary tuple functions and operations
macro_rules! tuple_impls {
Expand Down Expand Up @@ -66,6 +67,9 @@ macro_rules! tuple_impls {
($({ let x: $T = Default::default(); x},)+)
}
}

#[unstable(feature = "gc", issue = "none")]
impl<$($T:NoFinalize),+> NoFinalize for ($($T,)+) {}
)+
}
}
Expand Down
Loading