Skip to content

Commit e025bc7

Browse files
committed
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
1 parent 87bb44e commit e025bc7

File tree

3 files changed

+44
-14
lines changed

3 files changed

+44
-14
lines changed

gc/tests/finalize.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,12 @@ impl Finalize for B {
4141
struct X(Box<dyn Trace>);
4242

4343
#[test]
44+
#[should_panic(expected = "finalize on drop is broken")]
4445
fn drop_triggers_finalize() {
4546
FLAGS.with(|f| assert_eq!(f.get(), Flags(0, 0)));
4647
{
4748
let _x = A { b: B };
4849
FLAGS.with(|f| assert_eq!(f.get(), Flags(0, 0)));
4950
}
50-
FLAGS.with(|f| assert_eq!(f.get(), Flags(1, 1)));
51+
FLAGS.with(|f| assert_eq!(f.get(), Flags(1, 1), "finalize on drop is broken"));
5152
}

gc/tests/trace_impl.rs

+11
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ struct Baz {
3333
b: Bar,
3434
}
3535

36+
#[derive(Trace, Finalize, Copy, Clone)]
37+
struct CopyTrace {
38+
inner: Foo,
39+
}
40+
3641
#[test]
3742
fn test() {
3843
let bar = Bar { inner: Foo };
@@ -48,4 +53,10 @@ fn test() {
4853
baz.trace();
4954
}
5055
X.with(|x| assert!(*x.borrow() == 3));
56+
57+
let copytrace = CopyTrace { inner: Foo };
58+
unsafe {
59+
copytrace.trace();
60+
}
61+
X.with(|x| assert!(*x.borrow() == 4));
5162
}

gc_derive/src/lib.rs

+31-13
Original file line numberDiff line numberDiff line change
@@ -51,23 +51,41 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream {
5151
},
5252
);
5353

54-
// We also implement drop to prevent unsafe drop implementations on this
55-
// type and encourage people to use Finalize. This implementation will
56-
// call `Finalize::finalize` if it is safe to do so.
57-
let drop_impl = s.unbound_impl(
58-
quote!(::std::ops::Drop),
59-
quote! {
60-
fn drop(&mut self) {
61-
if ::gc::finalizer_safe() {
62-
::gc::Finalize::finalize(self);
63-
}
54+
// Generate some code which will fail to compile if the derived type has an
55+
// unsafe `drop` implementation.
56+
let (impl_generics, ty_generics, where_clause) = s.ast().generics.split_for_impl();
57+
let ident = &s.ast().ident;
58+
let assert_not_drop = quote! {
59+
// This approach to negative trait assertions is directly copied from
60+
// `static_assertions` v1.1.0.
61+
// https://github.com/nvzqz/static-assertions-rs/blob/18bc65a094d890fe1faa5d3ccb70f12b89eabf56/src/assert_impl.rs#L262-L287
62+
const _: () = {
63+
// Generic trait with a blanket impl over `()` for all types.
64+
trait AmbiguousIfDrop<T> {
65+
fn some_item() {}
6466
}
65-
},
66-
);
67+
68+
impl<T: ?::std::marker::Sized> AmbiguousIfDrop<()> for T {}
69+
70+
#[allow(dead_code)]
71+
struct Invalid;
72+
impl<T: ?::std::marker::Sized + ::std::ops::Drop> AmbiguousIfDrop<Invalid> for T {}
73+
74+
// If there is only one specialized trait impl, type inference with
75+
// `_` can be resolved, and this will compile.
76+
//
77+
// Fails to compile if `AmbiguousIfDrop<Invalid>` is implemented for
78+
// our type.
79+
#[allow(dead_code)]
80+
fn assert_not_drop #impl_generics () #where_clause {
81+
let _ = <#ident #ty_generics as AmbiguousIfDrop<_>>::some_item;
82+
}
83+
};
84+
};
6785

6886
quote! {
6987
#trace_impl
70-
#drop_impl
88+
#assert_not_drop
7189
}
7290
}
7391

0 commit comments

Comments
 (0)