Skip to content

gc: add unsafe_empty_trace parity and collector-agnostic helper paths #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Flamki wants to merge 2 commits into
base: main
Choose a base branch
from
Open

gc: add unsafe_empty_trace parity and collector-agnostic helper paths #77

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e29fb9f
gc: add unsafe_empty_trace parity and collector-agnostic trace paths
Flamki Apr 10, 2026
12d2a29
gc: drop gc_trace re-export and keep mark_sweep trace paths
Flamki Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions oscars/src/collectors/mark_sweep/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,35 @@ use super::WeakGc;
use super::WeakMap;
use super::cell::GcRefCell;

#[test]
fn unsafe_empty_trace_runs_finalize() {
use core::sync::atomic::{AtomicUsize, Ordering};

static FINALIZED: AtomicUsize = AtomicUsize::new(0);

struct Probe;

impl Finalize for Probe {
fn finalize(&self) {
FINALIZED.fetch_add(1, Ordering::SeqCst);
}
}

// SAFETY: `Probe` has no GC references to trace.
unsafe impl Trace for Probe {
crate::unsafe_empty_trace!();
}

FINALIZED.store(0, Ordering::SeqCst);
let probe = Probe;
<Probe as Trace>::run_finalizer(&probe);
assert_eq!(
FINALIZED.load(Ordering::SeqCst),
1,
"unsafe_empty_trace! must delegate to Finalize::finalize"
);
}

#[test]
fn basic_gc() {
let collector = &mut MarkSweepGarbageCollector::default()
Expand Down
33 changes: 25 additions & 8 deletions oscars/src/collectors/mark_sweep/trace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,27 @@ pub unsafe trait Trace: Finalize {
macro_rules! empty_trace {
() => {
#[inline]
unsafe fn trace(&self, _color: $crate::collectors::mark_sweep::TraceColor) {}
unsafe fn trace(&self, _color: $crate::gc_trace::TraceColor) {}
#[inline]
fn run_finalizer(&self) {
$crate::collectors::mark_sweep::Finalize::finalize(self);
$crate::gc_trace::Finalize::finalize(self);
}
};
}

/// Utility macro to define an empty implementation of [`Trace`] inside an
/// `unsafe impl Trace` block.
///
/// This mirrors `empty_trace!` semantics while making the unsafety explicit at
/// the call site.
#[macro_export]
macro_rules! unsafe_empty_trace {
() => {
#[inline]
unsafe fn trace(&self, _color: $crate::gc_trace::TraceColor) {}
#[inline]
fn run_finalizer(&self) {
$crate::gc_trace::Finalize::finalize(self);
}
};
}
Expand All @@ -88,22 +105,22 @@ macro_rules! empty_trace {
macro_rules! custom_trace {
($this:ident, $marker:ident, $body:expr) => {
#[inline]
unsafe fn trace(&self, color: $crate::collectors::mark_sweep::TraceColor) {
let $marker = |it: &dyn $crate::collectors::mark_sweep::Trace| {
unsafe fn trace(&self, color: $crate::gc_trace::TraceColor) {
let $marker = |it: &dyn $crate::gc_trace::Trace| {
// SAFETY: The implementor must ensure that `trace` is correctly implemented.
unsafe {
$crate::collectors::mark_sweep::Trace::trace(it, color);
$crate::gc_trace::Trace::trace(it, color);
}
};
let $this = self;
$body
}
#[inline]
fn run_finalizer(&self) {
fn $marker<T: $crate::collectors::mark_sweep::Trace + ?Sized>(it: &T) {
$crate::collectors::mark_sweep::Trace::run_finalizer(it);
fn $marker<T: $crate::gc_trace::Trace + ?Sized>(it: &T) {
$crate::gc_trace::Trace::run_finalizer(it);
}
$crate::collectors::mark_sweep::Finalize::finalize(self);
$crate::gc_trace::Finalize::finalize(self);
let $this = self;
$body
}
Expand Down
29 changes: 29 additions & 0 deletions oscars/src/collectors/mark_sweep_arena2/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,35 @@ use super::WeakGc;
use super::WeakMap;
use super::cell::GcRefCell;

#[test]
fn unsafe_empty_trace_runs_finalize() {
use core::sync::atomic::{AtomicUsize, Ordering};

static FINALIZED: AtomicUsize = AtomicUsize::new(0);

struct Probe;

impl Finalize for Probe {
fn finalize(&self) {
FINALIZED.fetch_add(1, Ordering::SeqCst);
}
}

// SAFETY: `Probe` has no GC references to trace.
unsafe impl Trace for Probe {
crate::unsafe_empty_trace!();
}

FINALIZED.store(0, Ordering::SeqCst);
let probe = Probe;
<Probe as Trace>::run_finalizer(&probe);
assert_eq!(
FINALIZED.load(Ordering::SeqCst),
1,
"unsafe_empty_trace! must delegate to Finalize::finalize"
);
}

#[test]
fn basic_gc() {
let collector = &mut MarkSweepGarbageCollector::default()
Expand Down
6 changes: 3 additions & 3 deletions oscars/src/collectors/mark_sweep_arena2/trace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Both collectors use the exact same `Trace` types
// NOTE: `empty_trace!` and `custom_trace!` hardcode `mark_sweep` paths
// This works now but will silently break if the types ever diverge.
// Both collectors use the exact same trace API types today.
// Helper macros resolve through crate-level `gc_trace`, so collector modules
// do not hardcode paths to a specific implementation.
pub use crate::collectors::mark_sweep::trace::{Finalize, Trace, TraceColor};
7 changes: 7 additions & 0 deletions oscars/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,12 @@ pub mod mark_sweep2 {
#[cfg(feature = "mark_sweep")]
pub use crate::collectors::mark_sweep::Collector;

/// Collector-agnostic trace API re-export used by derive/macros.
///
/// Both mark-sweep collectors currently share the same trace traits.
pub mod gc_trace {

@nekevss nekevss Apr 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: we shouldn't reexport these items from gc_trace

Eventually we should resolve export paths, but as it stands, the trace functionality is very specific to the mark sweep gc.

@Flamki Flamki Apr 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review — agreed, and fixed.

I removed the gc_trace re-export from lib.rs and switched macro/derive paths back to explicit mark_sweep trace exports.

unsafe_empty_trace! parity is still included, and tests remain in place for both collectors.

I also reran fmt, tests, clippy, and miri locally; all green.

pub use crate::collectors::mark_sweep::trace::{Finalize, Trace, TraceColor};
}

pub mod alloc;
pub mod collectors;
26 changes: 13 additions & 13 deletions oscars_derive/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream {
}

return s.unsafe_bound_impl(
quote!(::oscars::mark_sweep::Trace),
quote!(::oscars::gc_trace::Trace),
quote! {
#[inline(always)]
unsafe fn trace(&self, _color: ::oscars::mark_sweep::TraceColor) {}
unsafe fn trace(&self, _color: ::oscars::gc_trace::TraceColor) {}
#[inline]
fn run_finalizer(&self) {
::oscars::mark_sweep::Finalize::finalize(self)
::oscars::gc_trace::Finalize::finalize(self)
}
},
);
Expand All @@ -79,30 +79,30 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream {
.iter()
.any(|attr| attr.path().is_ident("unsafe_ignore_trace"))
});
let trace_body = s.each(|bi| quote!(::oscars::mark_sweep::Trace::trace(#bi, color)));
let trace_body = s.each(|bi| quote!(::oscars::gc_trace::Trace::trace(#bi, color)));
let trace_other_body = s.each(|bi| quote!(mark(#bi)));

s.add_bounds(AddBounds::Fields);
let trace_impl = s.unsafe_bound_impl(
quote!(::oscars::mark_sweep::Trace),
quote!(::oscars::gc_trace::Trace),
quote! {
#[inline]
unsafe fn trace(&self, color: ::oscars::mark_sweep::TraceColor) {
unsafe fn trace(&self, color: ::oscars::gc_trace::TraceColor) {
#[allow(dead_code)]
fn mark<T: ::oscars::mark_sweep::Trace + ?Sized>(it: &T, color: oscars::mark_sweep::TraceColor) {
fn mark<T: ::oscars::gc_trace::Trace + ?Sized>(it: &T, color: oscars::gc_trace::TraceColor) {
unsafe {
::oscars::mark_sweep::Trace::trace(it, color);
::oscars::gc_trace::Trace::trace(it, color);
}
}
match *self { #trace_body }
}
#[inline]
fn run_finalizer(&self) {
::oscars::mark_sweep::Finalize::finalize(self);
::oscars::gc_trace::Finalize::finalize(self);
#[allow(dead_code)]
fn mark<T: ::oscars::mark_sweep::Trace + ?Sized>(it: &T) {
fn mark<T: ::oscars::gc_trace::Trace + ?Sized>(it: &T) {
unsafe {
::oscars::mark_sweep::Trace::run_finalizer(it);
::oscars::gc_trace::Trace::run_finalizer(it);
}
}
match *self { #trace_other_body }
Expand All @@ -120,7 +120,7 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream {
#[allow(clippy::inline_always)]
#[inline(always)]
fn drop(&mut self) {
::oscars::mark_sweep::Finalize::finalize(self);
::oscars::gc_trace::Finalize::finalize(self);
}
},
)
Expand All @@ -143,5 +143,5 @@ decl_derive! {
/// Derives the `Finalize` trait.
#[allow(clippy::needless_pass_by_value)]
fn derive_finalize(s: Structure<'_>) -> proc_macro2::TokenStream {
s.unbound_impl(quote!(::oscars::mark_sweep::Finalize), quote!())
s.unbound_impl(quote!(::oscars::gc_trace::Finalize), quote!())
}