Skip to content

Commit

Permalink
Allow deriving 'Trace' on 'Copy' types
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mystor committed Apr 25, 2020
1 parent 87bb44e commit e025bc7
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 14 deletions.
3 changes: 2 additions & 1 deletion gc/tests/finalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ impl Finalize for B {
struct X(Box<dyn Trace>);

#[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"));
}
11 changes: 11 additions & 0 deletions gc/tests/trace_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -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));
}
44 changes: 31 additions & 13 deletions gc_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
fn some_item() {}
}
},
);

impl<T: ?::std::marker::Sized> AmbiguousIfDrop<()> for T {}

#[allow(dead_code)]
struct Invalid;
impl<T: ?::std::marker::Sized + ::std::ops::Drop> AmbiguousIfDrop<Invalid> 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<Invalid>` 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
}
}

Expand Down

0 comments on commit e025bc7

Please sign in to comment.