From e29fb9ff3befe25f4f086c45ba25c678ba41b098 Mon Sep 17 00:00:00 2001 From: Flamki <9833ayush@gmail.com> Date: Fri, 10 Apr 2026 22:00:55 +0530 Subject: [PATCH 1/2] gc: add unsafe_empty_trace parity and collector-agnostic trace paths --- oscars/src/collectors/mark_sweep/tests.rs | 29 ++++++++++++++++ oscars/src/collectors/mark_sweep/trace.rs | 33 ++++++++++++++----- .../src/collectors/mark_sweep_arena2/tests.rs | 29 ++++++++++++++++ .../src/collectors/mark_sweep_arena2/trace.rs | 6 ++-- oscars/src/lib.rs | 7 ++++ oscars_derive/src/lib.rs | 26 +++++++-------- 6 files changed, 106 insertions(+), 24 deletions(-) diff --git a/oscars/src/collectors/mark_sweep/tests.rs b/oscars/src/collectors/mark_sweep/tests.rs index 454e5e6..dff3901 100644 --- a/oscars/src/collectors/mark_sweep/tests.rs +++ b/oscars/src/collectors/mark_sweep/tests.rs @@ -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; + ::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() diff --git a/oscars/src/collectors/mark_sweep/trace.rs b/oscars/src/collectors/mark_sweep/trace.rs index 580120d..2a7e62b 100644 --- a/oscars/src/collectors/mark_sweep/trace.rs +++ b/oscars/src/collectors/mark_sweep/trace.rs @@ -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); } }; } @@ -88,11 +105,11 @@ 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; @@ -100,10 +117,10 @@ macro_rules! custom_trace { } #[inline] fn run_finalizer(&self) { - fn $marker(it: &T) { - $crate::collectors::mark_sweep::Trace::run_finalizer(it); + fn $marker(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 } diff --git a/oscars/src/collectors/mark_sweep_arena2/tests.rs b/oscars/src/collectors/mark_sweep_arena2/tests.rs index e556a56..bcf7fcc 100644 --- a/oscars/src/collectors/mark_sweep_arena2/tests.rs +++ b/oscars/src/collectors/mark_sweep_arena2/tests.rs @@ -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; + ::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() diff --git a/oscars/src/collectors/mark_sweep_arena2/trace.rs b/oscars/src/collectors/mark_sweep_arena2/trace.rs index 716d347..3b913f0 100644 --- a/oscars/src/collectors/mark_sweep_arena2/trace.rs +++ b/oscars/src/collectors/mark_sweep_arena2/trace.rs @@ -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}; diff --git a/oscars/src/lib.rs b/oscars/src/lib.rs index 7786ea0..0bb9121 100644 --- a/oscars/src/lib.rs +++ b/oscars/src/lib.rs @@ -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 { + pub use crate::collectors::mark_sweep::trace::{Finalize, Trace, TraceColor}; +} + pub mod alloc; pub mod collectors; diff --git a/oscars_derive/src/lib.rs b/oscars_derive/src/lib.rs index dd53004..f9f51a9 100644 --- a/oscars_derive/src/lib.rs +++ b/oscars_derive/src/lib.rs @@ -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) } }, ); @@ -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(it: &T, color: oscars::mark_sweep::TraceColor) { + fn mark(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(it: &T) { + fn mark(it: &T) { unsafe { - ::oscars::mark_sweep::Trace::run_finalizer(it); + ::oscars::gc_trace::Trace::run_finalizer(it); } } match *self { #trace_other_body } @@ -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); } }, ) @@ -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!()) } From 12d2a2951df6f0851ee29f6ba6a32d64f9a2146c Mon Sep 17 00:00:00 2001 From: Flamki <9833ayush@gmail.com> Date: Wed, 22 Apr 2026 01:45:33 +0530 Subject: [PATCH 2/2] gc: drop gc_trace re-export and keep mark_sweep trace paths --- oscars/src/collectors/mark_sweep/trace.rs | 20 +++++++------- .../src/collectors/mark_sweep_arena2/trace.rs | 6 ++--- oscars/src/lib.rs | 7 ----- oscars_derive/src/lib.rs | 26 +++++++++---------- 4 files changed, 26 insertions(+), 33 deletions(-) diff --git a/oscars/src/collectors/mark_sweep/trace.rs b/oscars/src/collectors/mark_sweep/trace.rs index 2a7e62b..9e39e0b 100644 --- a/oscars/src/collectors/mark_sweep/trace.rs +++ b/oscars/src/collectors/mark_sweep/trace.rs @@ -67,10 +67,10 @@ pub unsafe trait Trace: Finalize { macro_rules! empty_trace { () => { #[inline] - unsafe fn trace(&self, _color: $crate::gc_trace::TraceColor) {} + unsafe fn trace(&self, _color: $crate::collectors::mark_sweep::TraceColor) {} #[inline] fn run_finalizer(&self) { - $crate::gc_trace::Finalize::finalize(self); + $crate::collectors::mark_sweep::Finalize::finalize(self); } }; } @@ -84,10 +84,10 @@ macro_rules! empty_trace { macro_rules! unsafe_empty_trace { () => { #[inline] - unsafe fn trace(&self, _color: $crate::gc_trace::TraceColor) {} + unsafe fn trace(&self, _color: $crate::collectors::mark_sweep::TraceColor) {} #[inline] fn run_finalizer(&self) { - $crate::gc_trace::Finalize::finalize(self); + $crate::collectors::mark_sweep::Finalize::finalize(self); } }; } @@ -105,11 +105,11 @@ macro_rules! unsafe_empty_trace { macro_rules! custom_trace { ($this:ident, $marker:ident, $body:expr) => { #[inline] - unsafe fn trace(&self, color: $crate::gc_trace::TraceColor) { - let $marker = |it: &dyn $crate::gc_trace::Trace| { + unsafe fn trace(&self, color: $crate::collectors::mark_sweep::TraceColor) { + let $marker = |it: &dyn $crate::collectors::mark_sweep::Trace| { // SAFETY: The implementor must ensure that `trace` is correctly implemented. unsafe { - $crate::gc_trace::Trace::trace(it, color); + $crate::collectors::mark_sweep::Trace::trace(it, color); } }; let $this = self; @@ -117,10 +117,10 @@ macro_rules! custom_trace { } #[inline] fn run_finalizer(&self) { - fn $marker(it: &T) { - $crate::gc_trace::Trace::run_finalizer(it); + fn $marker(it: &T) { + $crate::collectors::mark_sweep::Trace::run_finalizer(it); } - $crate::gc_trace::Finalize::finalize(self); + $crate::collectors::mark_sweep::Finalize::finalize(self); let $this = self; $body } diff --git a/oscars/src/collectors/mark_sweep_arena2/trace.rs b/oscars/src/collectors/mark_sweep_arena2/trace.rs index 3b913f0..6740308 100644 --- a/oscars/src/collectors/mark_sweep_arena2/trace.rs +++ b/oscars/src/collectors/mark_sweep_arena2/trace.rs @@ -1,4 +1,4 @@ -// 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. +// Both collectors use the exact same `Trace` types. +// NOTE: `empty_trace!` and `custom_trace!` resolve through mark_sweep paths. +// This works today because mark_sweep2 depends on mark_sweep. pub use crate::collectors::mark_sweep::trace::{Finalize, Trace, TraceColor}; diff --git a/oscars/src/lib.rs b/oscars/src/lib.rs index 0bb9121..7786ea0 100644 --- a/oscars/src/lib.rs +++ b/oscars/src/lib.rs @@ -25,12 +25,5 @@ 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 { - pub use crate::collectors::mark_sweep::trace::{Finalize, Trace, TraceColor}; -} - pub mod alloc; pub mod collectors; diff --git a/oscars_derive/src/lib.rs b/oscars_derive/src/lib.rs index f9f51a9..dd53004 100644 --- a/oscars_derive/src/lib.rs +++ b/oscars_derive/src/lib.rs @@ -60,13 +60,13 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { } return s.unsafe_bound_impl( - quote!(::oscars::gc_trace::Trace), + quote!(::oscars::mark_sweep::Trace), quote! { #[inline(always)] - unsafe fn trace(&self, _color: ::oscars::gc_trace::TraceColor) {} + unsafe fn trace(&self, _color: ::oscars::mark_sweep::TraceColor) {} #[inline] fn run_finalizer(&self) { - ::oscars::gc_trace::Finalize::finalize(self) + ::oscars::mark_sweep::Finalize::finalize(self) } }, ); @@ -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::gc_trace::Trace::trace(#bi, color))); + let trace_body = s.each(|bi| quote!(::oscars::mark_sweep::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::gc_trace::Trace), + quote!(::oscars::mark_sweep::Trace), quote! { #[inline] - unsafe fn trace(&self, color: ::oscars::gc_trace::TraceColor) { + unsafe fn trace(&self, color: ::oscars::mark_sweep::TraceColor) { #[allow(dead_code)] - fn mark(it: &T, color: oscars::gc_trace::TraceColor) { + fn mark(it: &T, color: oscars::mark_sweep::TraceColor) { unsafe { - ::oscars::gc_trace::Trace::trace(it, color); + ::oscars::mark_sweep::Trace::trace(it, color); } } match *self { #trace_body } } #[inline] fn run_finalizer(&self) { - ::oscars::gc_trace::Finalize::finalize(self); + ::oscars::mark_sweep::Finalize::finalize(self); #[allow(dead_code)] - fn mark(it: &T) { + fn mark(it: &T) { unsafe { - ::oscars::gc_trace::Trace::run_finalizer(it); + ::oscars::mark_sweep::Trace::run_finalizer(it); } } match *self { #trace_other_body } @@ -120,7 +120,7 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { #[allow(clippy::inline_always)] #[inline(always)] fn drop(&mut self) { - ::oscars::gc_trace::Finalize::finalize(self); + ::oscars::mark_sweep::Finalize::finalize(self); } }, ) @@ -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::gc_trace::Finalize), quote!()) + s.unbound_impl(quote!(::oscars::mark_sweep::Finalize), quote!()) }