From 7f625723ed7c22077e300b3849a5302a558a1a99 Mon Sep 17 00:00:00 2001 From: Jake Hughes Date: Fri, 30 Jul 2021 10:02:36 +0100 Subject: [PATCH] Fix broken behaviour for NoFinalize trait Previously `std::mem::needs_finalize()` would only return `true` if the top-level type required finalizing. This now traverses component types correctly. See `test/ui/gc/needs_finalize.rs` for a detailed list of which types need finalizing. A lot of the refactoring in `needs_drop` comes from breaking changes in upstream. --- .../example/mini_core.rs | 2 + compiler/rustc_ty_utils/src/needs_drop.rs | 117 +++++++++++------- library/alloc/src/boxed.rs | 7 ++ library/alloc/src/raw_vec.rs | 3 + library/alloc/src/vec/mod.rs | 7 ++ library/core/src/gc.rs | 23 ++++ library/core/src/tuple.rs | 4 + src/test/ui/gc/needs_finalize.rs | 81 +++++++++--- 8 files changed, 180 insertions(+), 64 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/example/mini_core.rs b/compiler/rustc_codegen_cranelift/example/mini_core.rs index c4834c8040871..b207ea4726043 100644 --- a/compiler/rustc_codegen_cranelift/example/mini_core.rs +++ b/compiler/rustc_codegen_cranelift/example/mini_core.rs @@ -502,6 +502,8 @@ impl Deref for Box { } } +impl NoFinalize for Box {} + #[lang = "exchange_malloc"] unsafe fn allocate(size: usize, _align: usize) -> *mut u8 { libc::malloc(size) diff --git a/compiler/rustc_ty_utils/src/needs_drop.rs b/compiler/rustc_ty_utils/src/needs_drop.rs index 3a4f49446f210..6576c23f3384b 100644 --- a/compiler/rustc_ty_utils/src/needs_drop.rs +++ b/compiler/rustc_ty_utils/src/needs_drop.rs @@ -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 { + 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>, @@ -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> { @@ -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); @@ -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, } } } @@ -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, @@ -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>, AlwaysRequiresDrop> { let adt_components = move |adt_def: &ty::AdtDef| { if adt_def.is_manually_drop() { @@ -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, _> = - 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>, 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::>().into_iter()) - }; - let res: Result, _> = - 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>, 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( @@ -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>, 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) { diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 53bfe02d0e7a3..c8436f492a320 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -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; @@ -1721,3 +1722,9 @@ impl Stream for Box { (**self).size_hint() } } + +#[unstable(feature = "gc", issue = "none")] +impl NoFinalize for Box {} + +#[unstable(feature = "gc", issue = "none")] +impl NoFinalize for Box {} diff --git a/library/alloc/src/raw_vec.rs b/library/alloc/src/raw_vec.rs index d11d4031f7754..48ff609fd2a27 100644 --- a/library/alloc/src/raw_vec.rs +++ b/library/alloc/src/raw_vec.rs @@ -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; @@ -558,3 +559,5 @@ fn alloc_guard(alloc_size: usize) -> Result<(), TryReserveError> { fn capacity_overflow() -> ! { panic!("capacity overflow"); } + +impl NoFinalize for RawVec {} diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 6de9331b18ea4..00dee8286b59d 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -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; @@ -2711,6 +2712,12 @@ impl Ord for Vec { } } +#[unstable(feature = "gc", issue = "none")] +impl NoFinalize for Vec {} + +#[unstable(feature = "gc", issue = "none")] +impl NoFinalize for Vec {} + #[stable(feature = "rust1", since = "1.0.0")] unsafe impl<#[may_dangle] T, A: Allocator> Drop for Vec { fn drop(&mut self) { diff --git a/library/core/src/gc.rs b/library/core/src/gc.rs index 83a5433e28c58..8ef89afc8436e 100644 --- a/library/core/src/gc.rs +++ b/library/core/src/gc.rs @@ -70,3 +70,26 @@ pub unsafe fn gc_layout() -> Trace { impl !NoTrace for *mut T {} impl !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 ! {} +} diff --git a/library/core/src/tuple.rs b/library/core/src/tuple.rs index 9f8a3a1de4201..51817cd2f0e2c 100644 --- a/library/core/src/tuple.rs +++ b/library/core/src/tuple.rs @@ -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 { @@ -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,)+) {} )+ } } diff --git a/src/test/ui/gc/needs_finalize.rs b/src/test/ui/gc/needs_finalize.rs index 4ee6a3dda81a9..24dcd30f59fad 100644 --- a/src/test/ui/gc/needs_finalize.rs +++ b/src/test/ui/gc/needs_finalize.rs @@ -1,48 +1,91 @@ // run-pass +// ignore-tidy-linelength #![feature(gc)] use std::mem; +use std::gc::NoFinalize; -struct NeedsDrop; +struct HasDrop; -impl Drop for NeedsDrop { +impl Drop for HasDrop { fn drop(&mut self) {} } -struct Trivial(u8, f32); +struct HasDropNoFinalize; -struct NonTrivial(u8, NeedsDrop); +impl Drop for HasDropNoFinalize { + fn drop(&mut self) {} +} -struct ExplicitNoFinalize(Trivial, NonTrivial); +impl NoFinalize for HasDropNoFinalize {} -struct NestedNoFinalize(Trivial, ExplicitNoFinalize); +struct FinalizedContainer(T); -impl core::gc::NoFinalize for ExplicitNoFinalize {} +impl Drop for FinalizedContainer { + fn drop(&mut self) {} +} const CONST_U8: bool = mem::needs_finalizer::(); const CONST_STRING: bool = mem::needs_finalizer::(); -const CONST_TRIVIAL: bool = mem::needs_finalizer::(); -const CONST_NON_TRIVIAL: bool = mem::needs_finalizer::(); +const CONST_FINALIZABLE: bool = mem::needs_finalizer::(); +const CONST_UNFINALIZABLE: bool = mem::needs_finalizer::(); static STATIC_U8: bool = mem::needs_finalizer::(); static STATIC_STRING: bool = mem::needs_finalizer::(); -static STATIC_TRIVIAL: bool = mem::needs_finalizer::(); -static STATIC_NON_TRIVIAL: bool = mem::needs_finalizer::(); +static STATIC_FINALIZABLE: bool = mem::needs_finalizer::(); +static STATIC_UNFINALIZABLE: bool = mem::needs_finalizer::(); + +static BOX_TRIVIAL: bool = mem::needs_finalizer::>(); +static BOX_FINALIZABLE: bool = mem::needs_finalizer::>(); +static BOX_UNFINALIZABLE: bool = mem::needs_finalizer::>(); +static BOX_TUPLE_FINALIZABLE: bool = mem::needs_finalizer::>(); +static BOX_TUPLE_UNFINALIZABLE: bool = mem::needs_finalizer::>(); + +static VEC_TRIVIAL: bool = mem::needs_finalizer::>(); +static VEC_FINALIZABLE: bool = mem::needs_finalizer::>(); +static VEC_UNFINALIZABLE: bool = mem::needs_finalizer::>(); +static VEC_TUPLE_UNFINALIZABLE: bool = mem::needs_finalizer::>(); +static VEC_TUPLE_FINALIZABLE: bool = mem::needs_finalizer::>(); +static VEC_TUPLE_CONTAINS_FINALIZABLE: bool = mem::needs_finalizer::>(); -static STATIC_EXPLICIT_NO_FINALIZE: bool = mem::needs_finalizer::(); -static STATIC_NESTED_NO_FINALIZE: bool = mem::needs_finalizer::(); +static VEC_VEC_FINALIZABLE: bool = mem::needs_finalizer::>>(); +static VEC_VEC_UNFINALIZABLE: bool = mem::needs_finalizer::>>(); +static VEC_STRING: bool = mem::needs_finalizer::>(); +static VEC_BOX_FINALIZABLE: bool = mem::needs_finalizer::>>(); +static VEC_BOX_UNFINALIZABLE: bool = mem::needs_finalizer::>>(); + + +static OUTER_NEEDS_FINALIZING: bool = mem::needs_finalizer::>>(); fn main() { assert!(!CONST_U8); assert!(!CONST_STRING); - assert!(!CONST_TRIVIAL); - assert!(CONST_NON_TRIVIAL); + assert!(CONST_FINALIZABLE); + assert!(!CONST_UNFINALIZABLE); assert!(!STATIC_U8); assert!(!STATIC_STRING); - assert!(!STATIC_TRIVIAL); - assert!(STATIC_NON_TRIVIAL); + assert!(STATIC_FINALIZABLE); + assert!(!STATIC_UNFINALIZABLE); + + assert!(!BOX_TRIVIAL); + assert!(BOX_FINALIZABLE); + assert!(!BOX_UNFINALIZABLE); + assert!(BOX_TUPLE_FINALIZABLE); + assert!(!BOX_TUPLE_UNFINALIZABLE); + + assert!(!VEC_TRIVIAL); + assert!(VEC_FINALIZABLE); + assert!(!VEC_UNFINALIZABLE); + assert!(!VEC_TUPLE_UNFINALIZABLE); + assert!(VEC_TUPLE_FINALIZABLE); + assert!(VEC_TUPLE_CONTAINS_FINALIZABLE); + + assert!(VEC_VEC_FINALIZABLE); + assert!(!VEC_VEC_UNFINALIZABLE); + assert!(!VEC_STRING); + assert!(VEC_BOX_FINALIZABLE); + assert!(!VEC_BOX_UNFINALIZABLE); - assert!(!STATIC_EXPLICIT_NO_FINALIZE); - assert!(!STATIC_NESTED_NO_FINALIZE); + assert!(OUTER_NEEDS_FINALIZING); }