Skip to content

Commit dc858f3

Browse files
committed
[BREAKING] Split GcBrand up into GcErase + GcRebrand
This helps clarify its two seperate purposes. We stil require T: NullTrace for the `&'a T` implementation. This seems like a requirement for the time beeing :( Give warnings for the auto-derived implementations of GcRebrand and GcErase indicating that we (currently) don't check the correctness of the implementation on each of the fields. I've changed the double-indirection and '&mut &mut T' behavior I had setup back when we used a copying collector. This doesn't break anything right now (because we use a non-moving collector). However, I have to be really careful if we ever switch back! This commit is so incompatible it will force us towards a 0.2.0-alpha release.
1 parent ce3d610 commit dc858f3

File tree

8 files changed

+231
-76
lines changed

8 files changed

+231
-76
lines changed

libs/context/src/handle.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::ptr::NonNull;
55
use std::marker::PhantomData;
66
use std::sync::atomic::{self, AtomicPtr, AtomicUsize, Ordering};
77

8-
use zerogc::{Trace, GcSafe, GcBrand, GcVisitor, NullTrace, TraceImmutable, GcHandleSystem, GcBindHandle};
8+
use zerogc::{Trace, GcSafe, GcErase, GcRebrand, GcVisitor, NullTrace, TraceImmutable, GcHandleSystem, GcBindHandle};
99
use crate::{Gc, WeakCollectorRef, CollectorId, CollectorContext, CollectorRef, CollectionManager};
1010
use crate::collector::RawCollectorImpl;
1111

@@ -434,7 +434,7 @@ unsafe impl<T: GcSafe, C: RawHandleImpl> ::zerogc::GcHandle<T> for GcHandle<T, C
434434
}
435435
}
436436
unsafe impl<'new_gc, T, C> GcBindHandle<'new_gc, T> for GcHandle<T, C>
437-
where T: GcSafe, T: GcBrand<'new_gc, CollectorId<C>>,
437+
where T: GcSafe, T: GcRebrand<'new_gc, CollectorId<C>>,
438438
T::Branded: GcSafe, C: RawHandleImpl {
439439
#[inline]
440440
fn bind_to(&self, context: &'new_gc CollectorContext<C>) -> Gc<'new_gc, T::Branded, CollectorId<C>> {
@@ -590,12 +590,12 @@ unsafe impl<T: GcSafe + Sync, C: RawHandleImpl + Sync> Send for GcHandle<T, C> {
590590
unsafe impl<T: GcSafe + Sync, C: RawHandleImpl + Sync> Sync for GcHandle<T, C> {}
591591

592592
/// We support handles
593-
unsafe impl<'gc, T, C> GcHandleSystem<'gc, T> for CollectorRef<C>
593+
unsafe impl<'gc, 'a, T, C> GcHandleSystem<'gc, 'a, T> for CollectorRef<C>
594594
where C: RawHandleImpl,
595595
T: GcSafe + 'gc,
596-
T: GcBrand<'static, CollectorId<C>>,
597-
T::Branded: GcSafe {
598-
type Handle = GcHandle<T::Branded, C>;
596+
T: GcErase<'a, CollectorId<C>>,
597+
T::Erased: GcSafe {
598+
type Handle = GcHandle<T::Erased, C>;
599599

600600
#[inline]
601601
fn create_handle(gc: Gc<'gc, T, CollectorId<C>>) -> Self::Handle {

libs/derive/src/lib.rs

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
#![feature(
2+
proc_macro_diagnostic, // NEEDED for warnings
3+
)]
14
extern crate proc_macro;
25

36
use quote::{quote, quote_spanned};
@@ -323,12 +326,14 @@ fn impl_derive_trace(input: &DeriveInput) -> Result<TokenStream, syn::Error> {
323326
} else {
324327
impl_trace(&input, &info)?
325328
};
326-
let brand_impl = impl_brand(&input, &info)?;
329+
let rebrand_impl = impl_rebrand(&input, &info)?;
330+
let erase_impl = impl_erase(&input, &info)?;
327331
let gc_safe_impl = impl_gc_safe(&input, &info)?;
328332
let extra_impls = impl_extras(&input, &info)?;
329333
Ok(quote! {
330334
#trace_impl
331-
#brand_impl
335+
#rebrand_impl
336+
#erase_impl
332337
#gc_safe_impl
333338
#extra_impls
334339
})
@@ -458,7 +463,7 @@ fn impl_extras(target: &DeriveInput, info: &GcTypeInfo) -> Result<TokenStream, E
458463
})
459464
}
460465

461-
fn impl_brand(target: &DeriveInput, info: &GcTypeInfo) -> Result<TokenStream, Error> {
466+
fn impl_erase(target: &DeriveInput, info: &GcTypeInfo) -> Result<TokenStream, Error> {
462467
let name = &target.ident;
463468
let mut generics: Generics = target.generics.clone();
464469
let mut rewritten_params = Vec::new();
@@ -468,9 +473,76 @@ fn impl_brand(target: &DeriveInput, info: &GcTypeInfo) -> Result<TokenStream, Er
468473
match param {
469474
GenericParam::Type(ref mut type_param) => {
470475
let original_bounds = type_param.bounds.iter().cloned().collect::<Vec<_>>();
471-
type_param.bounds.push(parse_quote!(::zerogc::GcBrand<'new_gc, S>));
476+
type_param.bounds.push(parse_quote!(::zerogc::GcErase<'max, S>));
477+
type_param.bounds.push(parse_quote!('max));
472478
let param_name = &type_param.ident;
473-
let rewritten_type: Type = parse_quote!(<#param_name as ::zerogc::GcBrand<'new_gc, S>>::Branded);
479+
let rewritten_type: Type = parse_quote!(<#param_name as ::zerogc::GcErase<'max, S>>::Erased);
480+
rewritten_restrictions.push(WherePredicate::Type(PredicateType {
481+
lifetimes: None,
482+
bounded_ty: rewritten_type.clone(),
483+
colon_token: Default::default(),
484+
bounds: original_bounds.into_iter().collect()
485+
}));
486+
rewritten_param = GenericArgument::Type(rewritten_type);
487+
},
488+
GenericParam::Lifetime(ref l) => {
489+
if l.lifetime == info.config.gc_lifetime() {
490+
rewritten_param = parse_quote!('static);
491+
assert!(!info.config.ignored_lifetimes.contains(&l.lifetime));
492+
} else if info.config.ignored_lifetimes.contains(&l.lifetime) {
493+
let lt = l.lifetime.clone();
494+
rewritten_restrictions.push(WherePredicate::Lifetime(PredicateLifetime {
495+
lifetime: parse_quote!('max),
496+
colon_token: Default::default(),
497+
bounds: parse_quote!(#lt)
498+
}));
499+
rewritten_param = GenericArgument::Lifetime(lt);
500+
} else {
501+
return Err(Error::new(
502+
l.span(),
503+
"Unable to handle lifetime"
504+
));
505+
}
506+
},
507+
GenericParam::Const(ref param) => {
508+
let name = &param.ident;
509+
rewritten_param = GenericArgument::Const(parse_quote!(#name));
510+
}
511+
}
512+
rewritten_params.push(rewritten_param);
513+
}
514+
let mut impl_generics = generics.clone();
515+
impl_generics.params.push(GenericParam::Lifetime(parse_quote!('max)));
516+
impl_generics.params.push(GenericParam::Type(parse_quote!(S: ::zerogc::CollectorId)));
517+
impl_generics.make_where_clause().predicates.extend(rewritten_restrictions);
518+
let (_, ty_generics, _) = generics.split_for_impl();
519+
let (impl_generics, _, where_clause) = impl_generics.split_for_impl();
520+
::proc_macro::Diagnostic::spanned(
521+
::proc_macro::Span::call_site(),
522+
::proc_macro::Level::Warning,
523+
"derive(GcErase) doesn't currently verify the correctness of its fields"
524+
).emit();
525+
Ok(quote! {
526+
unsafe impl #impl_generics ::zerogc::GcErase<'max, S>
527+
for #name #ty_generics #where_clause {
528+
type Erased = #name::<#(#rewritten_params),*>;
529+
}
530+
})
531+
}
532+
533+
fn impl_rebrand(target: &DeriveInput, info: &GcTypeInfo) -> Result<TokenStream, Error> {
534+
let name = &target.ident;
535+
let mut generics: Generics = target.generics.clone();
536+
let mut rewritten_params = Vec::new();
537+
let mut rewritten_restrictions = Vec::new();
538+
for param in &mut generics.params {
539+
let rewritten_param: GenericArgument;
540+
match param {
541+
GenericParam::Type(ref mut type_param) => {
542+
let original_bounds = type_param.bounds.iter().cloned().collect::<Vec<_>>();
543+
type_param.bounds.push(parse_quote!(::zerogc::GcRebrand<'new_gc, S>));
544+
let param_name = &type_param.ident;
545+
let rewritten_type: Type = parse_quote!(<#param_name as ::zerogc::GcRebrand<'new_gc, S>>::Branded);
474546
rewritten_restrictions.push(WherePredicate::Type(PredicateType {
475547
lifetimes: None,
476548
bounded_ty: rewritten_type.clone(),
@@ -505,8 +577,13 @@ fn impl_brand(target: &DeriveInput, info: &GcTypeInfo) -> Result<TokenStream, Er
505577
impl_generics.make_where_clause().predicates.extend(rewritten_restrictions);
506578
let (_, ty_generics, _) = generics.split_for_impl();
507579
let (impl_generics, _, where_clause) = impl_generics.split_for_impl();
580+
::proc_macro::Diagnostic::spanned(
581+
::proc_macro::Span::call_site(),
582+
::proc_macro::Level::Warning,
583+
"derive(GcRebrand) doesn't currently verify the correctness of its fields"
584+
).emit();
508585
Ok(quote! {
509-
unsafe impl #impl_generics ::zerogc::GcBrand<'new_gc, S>
586+
unsafe impl #impl_generics ::zerogc::GcRebrand<'new_gc, S>
510587
for #name #ty_generics #where_clause {
511588
type Branded = #name::<#(#rewritten_params),*>;
512589
}

libs/derive/tests/basic.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ struct NopTrace {
6161
wow: Box<NopTrace>
6262
}
6363

64+
#[cfg(disabled)] // TODO: Fixup
6465
#[derive(Trace)]
6566
#[zerogc(nop_trace, ignore_lifetimes("'a"), ignore_params(T))]
6667
#[allow(unused)]

src/lib.rs

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ macro_rules! safepoint_recurse {
8686
* We trust that GcContext::recurse_context
8787
* did not perform a collection after calling the closure.
8888
*/
89-
let result = unsafe { $context.rebrand_self(erased_result) };
89+
let result = unsafe { $context.rebrand_self(**erased_result) };
9090
safepoint!($context, (root, result))
9191
}};
9292
}
@@ -117,7 +117,7 @@ macro_rules! __recurse_context {
117117
* NOTE: Guarenteed to live for the lifetime of the entire closure.
118118
* However, it could be relocated if 'sub_collector' is collected
119119
*/
120-
let $new_root = $sub_context.rebrand_self(erased_root);
120+
let $new_root = $sub_context.rebrand_self(*erased_root);
121121
$closure
122122
})
123123
}};
@@ -208,11 +208,11 @@ pub unsafe trait GcSystem {
208208
/// This type-system hackery is needed because
209209
/// we need to place bounds on `T as GcBrand`
210210
// TODO: Remove when we get more powerful types
211-
pub unsafe trait GcHandleSystem<'gc, T: GcSafe + ?Sized + 'gc>: GcSystem
212-
where T: GcBrand<'static, Self::Id>,
213-
<T as GcBrand<'static, Self::Id>>::Branded: GcSafe {
211+
pub unsafe trait GcHandleSystem<'gc, 'a, T: GcSafe + ?Sized + 'gc>: GcSystem
212+
where T: GcErase<'a, Self::Id>,
213+
<T as GcErase<'a, Self::Id>>::Erased: GcSafe {
214214
/// The type of handles to this object.
215-
type Handle: GcHandle<<T as GcBrand<'static, Self::Id>>::Branded, System=Self>;
215+
type Handle: GcHandle<<T as GcErase<'a, Self::Id>>::Erased, System=Self>;
216216

217217
/// Create a handle to the specified GC pointer,
218218
/// which can be used without a context
@@ -281,16 +281,16 @@ pub unsafe trait GcContext: Sized {
281281

282282
#[inline(always)]
283283
#[doc(hidden)]
284-
unsafe fn rebrand_static<T>(&self, value: T) -> T::Branded
285-
where T: GcBrand<'static, Self::Id> {
284+
unsafe fn rebrand_static<'a, T>(&self, value: T) -> T::Erased
285+
where T: GcErase<'a, Self::Id> {
286286
let branded = mem::transmute_copy(&value);
287287
mem::forget(value);
288288
branded
289289
}
290290
#[inline(always)]
291291
#[doc(hidden)]
292-
unsafe fn rebrand_self<'a, T>(&'a self, value: T) -> T::Branded
293-
where T: GcBrand<'a, Self::Id> {
292+
unsafe fn rebrand_self<'gc, T>(&'gc self, value: T) -> T::Branded
293+
where T: GcRebrand<'gc, Self::Id> {
294294
let branded = mem::transmute_copy(&value);
295295
mem::forget(value);
296296
branded
@@ -414,6 +414,10 @@ pub struct Gc<'gc, T: GcSafe + ?Sized + 'gc, Id: CollectorId> {
414414
/// Used to uniquely identify the collector,
415415
/// to ensure we aren't modifying another collector's pointers
416416
collector_id: Id,
417+
/*
418+
* TODO: I think this lifetime variance is safe
419+
* Better add some tests and an explination.
420+
*/
417421
marker: PhantomData<&'gc T>,
418422
}
419423
impl<'gc, T: GcSafe + ?Sized + 'gc, Id: CollectorId> Gc<'gc, T, Id> {
@@ -447,11 +451,11 @@ impl<'gc, T: GcSafe + ?Sized + 'gc, Id: CollectorId> Gc<'gc, T, Id> {
447451

448452
/// Create a handle to this object, which can be used without a context
449453
#[inline]
450-
pub fn create_handle(&self) -> <Id::System as GcHandleSystem<'gc, T>>::Handle
451-
where Id::System: GcHandleSystem<'gc, T>,
452-
T: GcBrand<'static, Id>,
453-
<T as GcBrand<'static, Id>>::Branded: GcSafe {
454-
<Id::System as GcHandleSystem<'gc, T>>::create_handle(*self)
454+
pub fn create_handle<'a>(&self) -> <Id::System as GcHandleSystem<'gc, 'a, T>>::Handle
455+
where Id::System: GcHandleSystem<'gc, 'a, T>,
456+
T: GcErase<'a, Id> + 'a,
457+
<T as GcErase<'a, Id>>::Erased: GcSafe + 'a {
458+
<Id::System as GcHandleSystem<'gc, 'a, T>>::create_handle(*self)
455459
}
456460

457461
/// Get a reference to the system
@@ -481,10 +485,17 @@ unsafe impl<'gc, T: GcSafe + 'gc, Id: CollectorId> GcSafe for Gc<'gc, T, Id> {
481485
const NEEDS_DROP: bool = true; // We are Copy
482486
}
483487
/// Rebrand
484-
unsafe impl<'gc, 'new_gc, T, Id> GcBrand<'new_gc, Id> for Gc<'gc, T, Id>
485-
where T: GcSafe + GcBrand<'new_gc, Id>,
486-
T::Branded: GcSafe, Id: CollectorId {
487-
type Branded = Gc<'new_gc, <T as GcBrand<'new_gc, Id>>::Branded, Id>;
488+
unsafe impl<'gc, 'new_gc, T, Id> GcRebrand<'new_gc, Id> for Gc<'gc, T, Id>
489+
where T: GcSafe + GcRebrand<'new_gc, Id>,
490+
<T as GcRebrand<'new_gc, Id>>::Branded: GcSafe,
491+
Id: CollectorId, {
492+
type Branded = Gc<'new_gc, <T as GcRebrand<'new_gc, Id>>::Branded, Id>;
493+
}
494+
unsafe impl<'gc, 'a, T, Id> GcErase<'a, Id> for Gc<'gc, T, Id>
495+
where T: GcSafe + GcErase<'a, Id>,
496+
<T as GcErase<'a, Id>>::Erased: GcSafe,
497+
Id: CollectorId, {
498+
type Erased = Gc<'a, <T as GcErase<'a, Id>>::Erased, Id>;
488499
}
489500
unsafe impl<'gc, T: GcSafe + 'gc, Id: CollectorId> Trace for Gc<'gc, T, Id> {
490501
// We always need tracing....
@@ -626,8 +637,8 @@ pub unsafe trait GcHandle<T: GcSafe + ?Sized>: Clone + NullTrace {
626637
///
627638
/// TODO: Remove when we get more powerful types
628639
pub unsafe trait GcBindHandle<'new_gc, T: GcSafe + ?Sized>: GcHandle<T>
629-
where T: GcBrand<'new_gc, Self::Id>,
630-
<T as GcBrand<'new_gc, Self::Id>>::Branded: GcSafe {
640+
where T: GcRebrand<'new_gc, Self::Id>,
641+
<T as GcRebrand<'new_gc, Self::Id>>::Branded: GcSafe {
631642
/// Associate this handle with the specified context,
632643
/// allowing its underlying object to be accessed
633644
/// as long as the context is valid.
@@ -638,7 +649,7 @@ pub unsafe trait GcBindHandle<'new_gc, T: GcSafe + ?Sized>: GcHandle<T>
638649
/// at the next safepoint.
639650
fn bind_to(&self, context: &'new_gc <Self::System as GcSystem>::Context) -> Gc<
640651
'new_gc,
641-
<T as GcBrand<'new_gc, Self::Id>>::Branded,
652+
<T as GcRebrand<'new_gc, Self::Id>>::Branded,
642653
Self::Id
643654
>;
644655
}
@@ -777,19 +788,34 @@ unsafe impl<T> GcSafe for AssumeNotTraced<T> {
777788
}
778789
unsafe_gc_brand!(AssumeNotTraced, T);
779790

780-
/// Changes all references to garbage
781-
/// collected objects to match `'new_gc`.
791+
/// Changes all references to garbage collected
792+
/// objects to match a specific lifetime.
782793
///
783794
/// This indicates that its safe to transmute to the new `Branded` type
784795
/// and all that will change is the lifetimes.
785-
pub unsafe trait GcBrand<'new_gc, Id: CollectorId>: Trace {
796+
// TODO: Can we support lifetimes that are smaller than 'new_gc
797+
pub unsafe trait GcRebrand<'new_gc, Id: CollectorId>: Trace {
786798
/// This type with all garbage collected lifetimes
787799
/// changed to `'new_gc`
788800
///
789801
/// This must have the same in-memory repr as `Self`,
790802
/// so that it's safe to transmute.
791803
type Branded: Trace + 'new_gc;
792804
}
805+
/// Indicates that it's safe to erase all GC lifetimes
806+
/// and change them to 'static (logically an 'unsafe)
807+
///
808+
/// This trait is the opposite of [GcRebrand]
809+
///
810+
/// The lifetime '`a` is the minimum lifetime of all other non-gc references.
811+
pub unsafe trait GcErase<'a, Id: CollectorId>: Trace {
812+
/// This type with all garbage collected lifetimes
813+
/// changed to `'static` (or erased)
814+
///
815+
/// This must have the same in-memory repr as `Self`,
816+
/// so that it's safe to transmute.
817+
type Erased: 'a;
818+
}
793819

794820
/// Indicates that a type can be traced by a garbage collector.
795821
///

0 commit comments

Comments
 (0)