Merge #39

39: Fix broken behaviour for NoFinalize trait r=ltratt a=jacob-hughes

Previously `std::mem::needs_finalize<T>()` 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.

Co-authored-by: Jake Hughes <jh@jakehughes.uk>

compiler/rustc_codegen_cranelift/example/mini_core.rs

@@ -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)

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<Ty<'_>>, 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<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(
@@ -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) {

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<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> {}

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<T: NoFinalize, A: Allocator> NoFinalize for RawVec<T, A> {}

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<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) {

library/core/src/gc.rs

@@ -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 ! {}
+}

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,)+) {}
         )+
     }
 }