From e025bc73a7c3f546e223f396f246c88330765745 Mon Sep 17 00:00:00 2001 From: Nika Layzell Date: Sat, 25 Apr 2020 17:31:06 -0400 Subject: [PATCH] Allow deriving 'Trace' on 'Copy' types This change means that the `Finalize::finalize` method is not automatically called on drop. This might be a reasonable approach to take, and would avoid the need for a separate derive, but would be a breaking change. Fixes #87 --- gc/tests/finalize.rs | 3 ++- gc/tests/trace_impl.rs | 11 +++++++++++ gc_derive/src/lib.rs | 44 +++++++++++++++++++++++++++++------------- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/gc/tests/finalize.rs b/gc/tests/finalize.rs index 524aa02..27febc9 100644 --- a/gc/tests/finalize.rs +++ b/gc/tests/finalize.rs @@ -41,11 +41,12 @@ impl Finalize for B { struct X(Box); #[test] +#[should_panic(expected = "finalize on drop is broken")] fn drop_triggers_finalize() { FLAGS.with(|f| assert_eq!(f.get(), Flags(0, 0))); { let _x = A { b: B }; FLAGS.with(|f| assert_eq!(f.get(), Flags(0, 0))); } - FLAGS.with(|f| assert_eq!(f.get(), Flags(1, 1))); + FLAGS.with(|f| assert_eq!(f.get(), Flags(1, 1), "finalize on drop is broken")); } diff --git a/gc/tests/trace_impl.rs b/gc/tests/trace_impl.rs index a3109ab..23a09ae 100644 --- a/gc/tests/trace_impl.rs +++ b/gc/tests/trace_impl.rs @@ -33,6 +33,11 @@ struct Baz { b: Bar, } +#[derive(Trace, Finalize, Copy, Clone)] +struct CopyTrace { + inner: Foo, +} + #[test] fn test() { let bar = Bar { inner: Foo }; @@ -48,4 +53,10 @@ fn test() { baz.trace(); } X.with(|x| assert!(*x.borrow() == 3)); + + let copytrace = CopyTrace { inner: Foo }; + unsafe { + copytrace.trace(); + } + X.with(|x| assert!(*x.borrow() == 4)); } diff --git a/gc_derive/src/lib.rs b/gc_derive/src/lib.rs index 8a35ae3..7b90be3 100644 --- a/gc_derive/src/lib.rs +++ b/gc_derive/src/lib.rs @@ -51,23 +51,41 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { }, ); - // We also implement drop to prevent unsafe drop implementations on this - // type and encourage people to use Finalize. This implementation will - // call `Finalize::finalize` if it is safe to do so. - let drop_impl = s.unbound_impl( - quote!(::std::ops::Drop), - quote! { - fn drop(&mut self) { - if ::gc::finalizer_safe() { - ::gc::Finalize::finalize(self); - } + // Generate some code which will fail to compile if the derived type has an + // unsafe `drop` implementation. + let (impl_generics, ty_generics, where_clause) = s.ast().generics.split_for_impl(); + let ident = &s.ast().ident; + let assert_not_drop = quote! { + // This approach to negative trait assertions is directly copied from + // `static_assertions` v1.1.0. + // https://github.com/nvzqz/static-assertions-rs/blob/18bc65a094d890fe1faa5d3ccb70f12b89eabf56/src/assert_impl.rs#L262-L287 + const _: () = { + // Generic trait with a blanket impl over `()` for all types. + trait AmbiguousIfDrop { + fn some_item() {} } - }, - ); + + impl AmbiguousIfDrop<()> for T {} + + #[allow(dead_code)] + struct Invalid; + impl AmbiguousIfDrop for T {} + + // If there is only one specialized trait impl, type inference with + // `_` can be resolved, and this will compile. + // + // Fails to compile if `AmbiguousIfDrop` is implemented for + // our type. + #[allow(dead_code)] + fn assert_not_drop #impl_generics () #where_clause { + let _ = <#ident #ty_generics as AmbiguousIfDrop<_>>::some_item; + } + }; + }; quote! { #trace_impl - #drop_impl + #assert_not_drop } }