From 0730a11b3661407129a05b545f27e1186b4a8e38 Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Sat, 15 Aug 2026 03:17:26 +0000 Subject: [PATCH 1/2] feat(gc): integrate oscars GC backend into boa_engine --- .github/workflows/pull_request.yml | 1 + .github/workflows/rust.yml | 2 + .github/workflows/test262.yml | 2 + .github/workflows/webassembly.yml | 2 + Cargo.lock | 11 +- Cargo.toml | 4 + core/engine/Cargo.toml | 1 + core/engine/src/builtins/eval/mod.rs | 14 +-- .../src/builtins/finalization_registry/mod.rs | 28 ++--- .../builtins/finalization_registry/tests.rs | 1 + .../engine/src/builtins/function/arguments.rs | 4 +- core/engine/src/builtins/function/mod.rs | 10 +- core/engine/src/builtins/generator/mod.rs | 2 +- .../src/builtins/intl/list_format/mod.rs | 2 +- core/engine/src/builtins/intl/locale/mod.rs | 13 +- core/engine/src/builtins/intl/locale/utils.rs | 4 +- core/engine/src/builtins/json/mod.rs | 11 +- core/engine/src/builtins/promise/mod.rs | 27 +---- core/engine/src/builtins/set/ordered_set.rs | 6 +- core/engine/src/builtins/weak/weak_ref.rs | 6 +- core/engine/src/builtins/weak_map/mod.rs | 85 ++++++------- core/engine/src/builtins/weak_set/mod.rs | 4 +- core/engine/src/bytecompiler/class.rs | 12 +- core/engine/src/bytecompiler/function.rs | 2 +- core/engine/src/context/mod.rs | 14 +++ core/engine/src/environments/runtime/mod.rs | 14 ++- core/engine/src/error/mod.rs | 9 +- core/engine/src/host_defined.rs | 2 +- core/engine/src/lib.rs | 4 + core/engine/src/module/loader/mod.rs | 2 +- core/engine/src/module/mod.rs | 14 +-- core/engine/src/module/source.rs | 10 +- core/engine/src/module/synthetic.rs | 12 +- .../src/native_function/continuation.rs | 4 +- core/engine/src/native_function/mod.rs | 21 ++-- core/engine/src/object/builtins/jspromise.rs | 5 +- .../src/object/builtins/jstypedarray.rs | 2 +- core/engine/src/object/builtins/jsweakmap.rs | 2 +- core/engine/src/object/builtins/jsweakset.rs | 2 +- core/engine/src/object/jsobject.rs | 24 ++-- core/engine/src/object/mod.rs | 10 ++ .../shape/shared_shape/forward_transition.rs | 12 +- .../src/object/shape/shared_shape/mod.rs | 19 +-- core/engine/src/object/shape/unique_shape.rs | 16 ++- core/engine/src/realm.rs | 4 +- core/engine/src/script.rs | 8 +- core/engine/src/value/equality.rs | 2 +- core/engine/src/value/inner/legacy.rs | 8 +- core/engine/src/value/inner/nan_boxed.rs | 14 +-- core/engine/src/value/integer.rs | 8 +- core/engine/src/vm/code_block.rs | 1 + core/engine/src/vm/inline_cache/mod.rs | 1 + core/engine/src/vm/mod.rs | 2 +- core/engine/src/vm/opcode/await/mod.rs | 7 +- core/engine/src/vm/opcode/function.rs | 4 +- core/engine/src/vm/opcode/push/environment.rs | 6 +- core/engine/src/vm/tests.rs | 1 + core/gc/Cargo.toml | 13 +- core/gc/src/cell.rs | 10 +- core/gc/src/lib.rs | 114 +++++++++++++++++- core/gc/src/oscars_weak_map.rs | 109 +++++++++++++++++ core/gc/src/pointers/mutation_context.rs | 6 + core/gc/src/pointers/weak_map.rs | 13 ++ core/gc/src/test/weak.rs | 2 +- core/gc/src/trace.rs | 12 +- core/interner/src/sym.rs | 14 +-- core/macros/src/lib.rs | 38 +++--- core/runtime/src/abort/mod.rs | 3 +- core/runtime/src/console/tests.rs | 24 ++-- core/runtime/src/microtask/tests.rs | 2 +- core/runtime/src/test262.rs | 6 +- core/string/Cargo.toml | 4 + core/string/src/builder.rs | 8 +- core/string/src/lib.rs | 16 +++ core/string/src/tests.rs | 4 +- examples/src/bin/derive.rs | 1 + examples/src/bin/jstypedarray.rs | 2 +- tests/fuzz/Cargo.toml | 3 + tests/macros/tests/gcd_callback.rs | 4 +- 79 files changed, 631 insertions(+), 295 deletions(-) create mode 100644 core/gc/src/oscars_weak_map.rs diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index ae17ebe355e..fa208682396 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -5,6 +5,7 @@ on: branches: - main - releases/** + - dev/oscars-gc permissions: contents: read diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 0fa015031fb..1dfd917b2a6 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -5,10 +5,12 @@ on: branches: - main - releases/** + - dev/oscars-gc push: branches: - main - releases/** + - dev/oscars-gc merge_group: types: [checks_requested] workflow_dispatch: diff --git a/.github/workflows/test262.yml b/.github/workflows/test262.yml index 6797efa033d..a8671fc37d0 100644 --- a/.github/workflows/test262.yml +++ b/.github/workflows/test262.yml @@ -5,6 +5,7 @@ on: branches: - main - releases/** + - dev/oscars-gc permissions: contents: read @@ -15,6 +16,7 @@ concurrency: jobs: run_test262: + if: ${{ github.base_ref != 'dev/oscars-gc' }} name: Run the test262 test suite runs-on: ubuntu-latest timeout-minutes: 60 diff --git a/.github/workflows/webassembly.yml b/.github/workflows/webassembly.yml index f9538775ebd..676a8602965 100644 --- a/.github/workflows/webassembly.yml +++ b/.github/workflows/webassembly.yml @@ -5,10 +5,12 @@ on: branches: - main - releases/** + - dev/oscars-gc push: branches: - main - releases/** + - dev/oscars-gc merge_group: types: [checks_requested] diff --git a/Cargo.lock b/Cargo.lock index 0585ab322c9..7e2cf9ee140 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -482,6 +482,7 @@ dependencies = [ "icu_locale_core", "oscars", "thin-vec", + "typeid", ] [[package]] @@ -593,6 +594,7 @@ version = "1.0.0-dev" dependencies = [ "fast-float2", "itoa", + "oscars", "pastey", "rustc-hash 2.1.2", "ryu-js", @@ -2846,17 +2848,22 @@ dependencies = [ [[package]] name = "oscars" version = "0.1.0" -source = "git+https://github.com/boa-dev/oscars.git?branch=main#592903ff2bec29ae3f4be7fecf4baded74674be2" +source = "git+https://github.com/boa-dev/oscars.git?branch=main#ed5f692df0356338a82c113982449a3b5f7b1927" dependencies = [ + "arrayvec", + "either", "hashbrown 0.16.1", + "icu_locale_core", "oscars_derive", "rustc-hash 2.1.2", + "thin-vec", + "typeid", ] [[package]] name = "oscars_derive" version = "0.1.0" -source = "git+https://github.com/boa-dev/oscars.git?branch=main#592903ff2bec29ae3f4be7fecf4baded74674be2" +source = "git+https://github.com/boa-dev/oscars.git?branch=main#ed5f692df0356338a82c113982449a3b5f7b1927" dependencies = [ "cfg-if", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 41ce5060891..998e9f55e02 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -118,6 +118,7 @@ num-integer = "0.1.46" ryu-js = "1.0.2" tap = "1.0.1" thiserror = { version = "2.0.18", default-features = false } +typeid = "1.0.3" dashmap = "6.2.1" num_enum = "0.7.6" itertools = { version = "0.15.0", default-features = false } @@ -267,3 +268,6 @@ complexity = { level = "warn", priority = -1 } perf = { level = "warn", priority = -1 } pedantic = { level = "warn", priority = -1 } +[patch."https://github.com/boa-dev/boa.git"] +boa_string = { path = "core/string" } + diff --git a/core/engine/Cargo.toml b/core/engine/Cargo.toml index d31abc302c8..eafe12de72b 100644 --- a/core/engine/Cargo.toml +++ b/core/engine/Cargo.toml @@ -26,6 +26,7 @@ embedded_lz4 = ["boa_macros/embedded_lz4", "lz4_flex"] jsvalue-enum = [] deser = ["boa_interner/serde", "boa_ast/serde"] either = ["dep:either", "boa_gc/either"] +oscars_backend = ["boa_gc/oscars_backend", "boa_string/oscars_backend"] # Enables the `Intl` builtin object and bundles a default ICU4X data provider. # Prefer this over `intl` if you just want to enable `Intl` without dealing with the diff --git a/core/engine/src/builtins/eval/mod.rs b/core/engine/src/builtins/eval/mod.rs index 7b0f9246640..0fd60137801 100644 --- a/core/engine/src/builtins/eval/mod.rs +++ b/core/engine/src/builtins/eval/mod.rs @@ -320,10 +320,8 @@ impl Eval { compiler.compile_statement_list(body.statements(), true, false); - let code_block = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - compiler.finish(), - ); + let finished = compiler.finish(); + let code_block = Gc::new(&context.gc(), finished); // Strict calls don't need extensions, since all strict eval calls push a new // function environment before evaluating. @@ -350,9 +348,11 @@ impl Eval { { let frame = context.vm.frame_mut(); let global = frame.realm.environment(); - frame - .environments - .push_lexical(lexical_scope.num_bindings_non_local(), global); + frame.environments.push_lexical( + lexical_scope.num_bindings_non_local(), + global, + unsafe { boa_gc::MutationContext::global() }, + ); } context diff --git a/core/engine/src/builtins/finalization_registry/mod.rs b/core/engine/src/builtins/finalization_registry/mod.rs index 4ec7ba0c0f3..810e40e287b 100644 --- a/core/engine/src/builtins/finalization_registry/mod.rs +++ b/core/engine/src/builtins/finalization_registry/mod.rs @@ -158,10 +158,7 @@ impl BuiltInConstructor for FinalizationRegistry { }, ); - let weak_registry = WeakGc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - registry.inner(), - ); + let weak_registry = WeakGc::new(&context.gc(), registry.inner()); { async fn inner_cleanup( @@ -174,7 +171,7 @@ impl BuiltInConstructor for FinalizationRegistry { }; let Some(registry) = weak_registry - .upgrade(&unsafe { boa_gc::MutationContext::dummy() }) + .upgrade(&unsafe { boa_gc::MutationContext::global() }) .map(JsObject::from_inner) else { return Ok(JsValue::undefined()); @@ -205,7 +202,7 @@ impl FinalizationRegistry { /// [`FinalizationRegistry.prototype.register ( target, heldValue [ , unregisterToken ] )`][spec] /// /// [spec]: https://tc39.es/ecma262/sec-finalization-registry.prototype.register - fn register(this: &JsValue, args: &[JsValue], _context: &mut Context) -> JsResult { + fn register(this: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult { // 1. Let finalizationRegistry be the this value. // 2. Perform ? RequireInternalSlot(finalizationRegistry, [[Cells]]). let this = this.as_object(); @@ -257,10 +254,7 @@ impl FinalizationRegistry { // // TODO: support Symbols let unregister_token = match unregister_token.variant() { - JsVariant::Object(obj) => Some(WeakGc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - obj.inner(), - )), + JsVariant::Object(obj) => Some(WeakGc::new(&context.gc(), obj.inner())), // b. Set unregisterToken to empty. JsVariant::Undefined => None, // a. If unregisterToken is not undefined, throw a TypeError exception. @@ -275,7 +269,7 @@ impl FinalizationRegistry { // 6. Let cell be the Record { [[WeakRefTarget]]: target, [[HeldValue]]: heldValue, [[UnregisterToken]]: unregisterToken }. let cell = RegistryCell { target: Ephemeron::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &context.gc(), target_obj.inner(), CleanupSignaler(Cell::new(Some( registry.cleanup_notifier.clone().downgrade(), @@ -295,7 +289,7 @@ impl FinalizationRegistry { /// [`FinalizationRegistry.prototype.unregister ( unregisterToken )`][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-finalization-registry.prototype.unregister - fn unregister(this: &JsValue, args: &[JsValue], _context: &mut Context) -> JsResult { + fn unregister(this: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult { // 1. Let finalizationRegistry be the this value. // 2. Perform ? RequireInternalSlot(finalizationRegistry, [[Cells]]). let this = this.as_object(); @@ -338,20 +332,16 @@ impl FinalizationRegistry { // a. If cell.[[UnregisterToken]] is not empty and SameValue(cell.[[UnregisterToken]], unregisterToken) is true, then if let Some(tok) = cell.unregister_token.as_ref() - && let Some(tok) = tok.upgrade(&unsafe { boa_gc::MutationContext::dummy() }) + && let Some(tok) = tok.upgrade(&context.gc()) && Gc::ptr_eq(&tok, unregister_token) { // i. Remove cell from finalizationRegistry.[[Cells]]. let cell = registry.cells.swap_remove(i); - let _key = cell - .target - .key(&unsafe { boa_gc::MutationContext::dummy() }); + let _key = cell.target.key(&context.gc()); // TODO: it might be better to add a special ref for the value that // also preserves the original key instead. - cell.target - .value(&unsafe { boa_gc::MutationContext::dummy() }) - .and_then(|v| v.0.take()); + cell.target.value(&context.gc()).and_then(|v| v.0.take()); // ii. Set removed to true. removed = true; diff --git a/core/engine/src/builtins/finalization_registry/tests.rs b/core/engine/src/builtins/finalization_registry/tests.rs index 602bcc53586..0c4802e0093 100644 --- a/core/engine/src/builtins/finalization_registry/tests.rs +++ b/core/engine/src/builtins/finalization_registry/tests.rs @@ -1,3 +1,4 @@ +#[cfg(not(feature = "oscars_backend"))] mod miri { use indoc::indoc; diff --git a/core/engine/src/builtins/function/arguments.rs b/core/engine/src/builtins/function/arguments.rs index 81fe3f10043..ab339f96d07 100644 --- a/core/engine/src/builtins/function/arguments.rs +++ b/core/engine/src/builtins/function/arguments.rs @@ -1,3 +1,5 @@ +#![allow(clippy::trivially_copy_pass_by_ref)] +#![allow(clippy::needless_pass_by_value)] use crate::{ Context, JsData, JsExpect, JsResult, JsValue, bytecompiler::ToJsString, @@ -124,7 +126,7 @@ impl MappedArguments { .get(index as usize) .copied() .flatten()?; - self.environment.get(binding_index) + (*self.environment).get(binding_index) } /// Set the value of the binding at the given index in the function environment. diff --git a/core/engine/src/builtins/function/mod.rs b/core/engine/src/builtins/function/mod.rs index d7585a76994..498cb555b8c 100644 --- a/core/engine/src/builtins/function/mod.rs +++ b/core/engine/src/builtins/function/mod.rs @@ -1073,7 +1073,9 @@ pub(crate) fn function_call( if has_binding_identifier { let frame = context.vm.frame_mut(); let global = frame.realm.environment(); - let index = frame.environments.push_lexical(1, global); + let index = frame + .environments + .push_lexical(1, global, unsafe { boa_gc::MutationContext::global() }); frame.environments.put_lexical_value( BindingLocatorScope::Stack(index), 0, @@ -1091,6 +1093,7 @@ pub(crate) fn function_call( scope, FunctionSlots::new(this, function_object.clone(), None), global, + unsafe { boa_gc::MutationContext::global() }, ); } @@ -1181,7 +1184,9 @@ fn function_construct( if has_binding_identifier { let frame = context.vm.frame_mut(); let global = frame.realm.environment(); - let index = frame.environments.push_lexical(1, global); + let index = frame + .environments + .push_lexical(1, global, unsafe { boa_gc::MutationContext::global() }); frame.environments.put_lexical_value( BindingLocatorScope::Stack(index), 0, @@ -1210,6 +1215,7 @@ fn function_construct( ), ), global, + unsafe { boa_gc::MutationContext::global() }, ); } diff --git a/core/engine/src/builtins/generator/mod.rs b/core/engine/src/builtins/generator/mod.rs index 85e086e9419..17e60e63aec 100644 --- a/core/engine/src/builtins/generator/mod.rs +++ b/core/engine/src/builtins/generator/mod.rs @@ -46,7 +46,7 @@ pub(crate) enum GeneratorState { // Need to manually implement, since `Trace` adds a `Drop` impl which disallows destructuring. unsafe impl Trace for GeneratorState { custom_trace!(this, mark, { - match &this { + match this { Self::SuspendedStart { context } | Self::SuspendedYield { context } => mark(context), Self::Executing | Self::Completed => {} } diff --git a/core/engine/src/builtins/intl/list_format/mod.rs b/core/engine/src/builtins/intl/list_format/mod.rs index 9c9bb2e0200..403fca045e6 100644 --- a/core/engine/src/builtins/intl/list_format/mod.rs +++ b/core/engine/src/builtins/intl/list_format/mod.rs @@ -329,7 +329,7 @@ impl ListFormat { part: writeable::Part, mut f: impl FnMut(&mut Self::SubPartsWrite) -> core::fmt::Result, ) -> core::fmt::Result { - assert!(part.category == "list"); + assert_eq!(part.category, "list"); let mut string = WriteString(String::new()); f(&mut string)?; if !string.0.is_empty() { diff --git a/core/engine/src/builtins/intl/locale/mod.rs b/core/engine/src/builtins/intl/locale/mod.rs index 2930d0adf7a..949f5b094ce 100644 --- a/core/engine/src/builtins/intl/locale/mod.rs +++ b/core/engine/src/builtins/intl/locale/mod.rs @@ -349,14 +349,17 @@ impl Locale { // 1. Let loc be the this value. // 2. Perform ? RequireInternalSlot(loc, [[InitializedLocale]]). let object = this.as_object(); + // Under `oscars_backend`, `downcast_ref` returns `GcRef<'_, Locale>`. + // Deref through the guard before cloning to get an owned `icu_locale::Locale`. + // This is required because `GcRef<'_, Locale>` doesn't implement `NativeObject`. let mut loc = object .as_ref() .and_then(|o| o.downcast_ref::()) .ok_or_else(|| { JsNativeError::typ() .with_message("`Locale.maximize` can only be called on a `Locale` object") - })? - .clone(); + }) + .map(|r| (*r).clone())?; // 3. Let maximal be the result of the Add Likely Subtags algorithm applied to loc.[[Locale]]. If an error is signaled, set maximal to loc.[[Locale]]. context @@ -387,6 +390,8 @@ impl Locale { // 1. Let loc be the this value. // 2. Perform ? RequireInternalSlot(loc, [[InitializedLocale]]). let object = this.as_object(); + // Under `oscars_backend`, `downcast_ref` returns `GcRef<'_, Locale>`. + // Deref through the guard before cloning to get an owned `icu_locale::Locale`. let mut loc = object .as_ref() .and_then(|o| o.downcast_ref::()) @@ -394,8 +399,8 @@ impl Locale { JsNativeError::typ().with_message( "`Locale.prototype.minimize` can only be called on a `Locale` object", ) - })? - .clone(); + }) + .map(|r| (*r).clone())?; // 3. Let minimal be the result of the Remove Likely Subtags algorithm applied to loc.[[Locale]]. If an error is signaled, set minimal to loc.[[Locale]]. context diff --git a/core/engine/src/builtins/intl/locale/utils.rs b/core/engine/src/builtins/intl/locale/utils.rs index 1193f6d53c3..7dcf9b29a31 100644 --- a/core/engine/src/builtins/intl/locale/utils.rs +++ b/core/engine/src/builtins/intl/locale/utils.rs @@ -54,7 +54,9 @@ pub(crate) fn locale_from_value(tag: &JsValue, context: &mut Context) -> JsResul if let Some(tag) = object.as_ref().and_then(|obj| obj.downcast_ref::()) { // 1. Let tag be kValue.[[Locale]]. // No need to canonicalize since all `Locale` objects should already be canonicalized. - return Ok(tag.clone()); + // Under `oscars_backend`, `downcast_ref` returns `GcRef<'_, Locale>`. + // Deref through the guard before cloning to clone the `Locale` value, not the wrapper. + return Ok((*tag).clone()); } // iv. Else, diff --git a/core/engine/src/builtins/json/mod.rs b/core/engine/src/builtins/json/mod.rs index be01ebb234b..3bc2f4f9abc 100644 --- a/core/engine/src/builtins/json/mod.rs +++ b/core/engine/src/builtins/json/mod.rs @@ -307,10 +307,8 @@ impl Json { SourcePath::Json, ); compiler.compile_statement_list(script.statements(), true, false); - Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - compiler.finish(), - ) + let finished = compiler.finish(); + Gc::new(&context.gc(), finished) }; let realm = context.realm().clone(); @@ -815,7 +813,10 @@ impl Json { // d. Else if value has a [[BigIntData]] internal slot, then else if let Some(bigint) = obj.downcast_ref::() { // i. Set value to value.[[BigIntData]]. - value = bigint.clone().into(); + // SAFETY: Under oscars_backend, `downcast_ref` returns a `GcRef<'_, JsBigInt>`. + // We must deref through the guard before calling `.clone()` so that we clone + // the inner `JsBigInt`, not the `GcRef` wrapper. + value = (*bigint).clone().into(); } // e. Else if value has a [[IsRawJSON]] internal slot, then else if obj.is::() { diff --git a/core/engine/src/builtins/promise/mod.rs b/core/engine/src/builtins/promise/mod.rs index e79b52fc256..f03ef9500da 100644 --- a/core/engine/src/builtins/promise/mod.rs +++ b/core/engine/src/builtins/promise/mod.rs @@ -244,7 +244,7 @@ impl PromiseCapability { // 2. NOTE: C is assumed to be a constructor function that supports the parameter conventions of the Promise constructor (see 27.2.3.1). // 3. Let promiseCapability be the PromiseCapability Record { [[Promise]]: undefined, [[Resolve]]: undefined, [[Reject]]: undefined }. let promise_capability = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &context.gc(), GcRefCell::new(RejectResolve { reject: JsValue::undefined(), resolve: JsValue::undefined(), @@ -656,10 +656,7 @@ impl Promise { } // 1. Let values be a new empty List. - let values = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - GcRefCell::new(Vec::new()), - ); + let values = Gc::new(&context.gc(), GcRefCell::new(Vec::new())); // 2. Let remainingElementsCount be the Record { [[Value]]: 1 }. let remaining_elements_count = Rc::new(Cell::new(1)); @@ -874,10 +871,7 @@ impl Promise { } // 1. Let values be a new empty List. - let values = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - GcRefCell::new(Vec::new()), - ); + let values = Gc::new(&context.gc(), GcRefCell::new(Vec::new())); // 2. Let remainingElementsCount be the Record { [[Value]]: 1 }. let remaining_elements_count = Rc::new(Cell::new(1)); @@ -1244,10 +1238,7 @@ impl Promise { let keys = Rc::new(RefCell::new(Vec::new())); // 3. Let values be a new empty List. - let values = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - GcRefCell::new(Vec::new()), - ); + let values = Gc::new(&context.gc(), GcRefCell::new(Vec::new())); // 4. Let remainingElementsCount be the Record { [[Value]]: 1 }. let remaining_elements_count = Rc::new(Cell::new(1)); @@ -1557,10 +1548,7 @@ impl Promise { } // 1. Let errors be a new empty List. - let errors = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - GcRefCell::new(Vec::new()), - ); + let errors = Gc::new(&context.gc(), GcRefCell::new(Vec::new())); // 2. Let remainingElementsCount be the Record { [[Value]]: 1 }. let remaining_elements_count = Rc::new(Cell::new(1)); @@ -2460,10 +2448,7 @@ impl Promise { // 1. Let alreadyResolved be the Record { [[Value]]: false }. // 5. Set resolve.[[Promise]] to promise. // 6. Set resolve.[[AlreadyResolved]] to alreadyResolved. - let promise = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - Cell::new(Some(promise.clone())), - ); + let promise = Gc::new(&context.gc(), Cell::new(Some(promise.clone()))); // 2. Let stepsResolve be the algorithm steps defined in Promise Resolve Functions. // 3. Let lengthResolve be the number of non-optional parameters of the function definition in Promise Resolve Functions. diff --git a/core/engine/src/builtins/set/ordered_set.rs b/core/engine/src/builtins/set/ordered_set.rs index 6c604263662..a9888594441 100644 --- a/core/engine/src/builtins/set/ordered_set.rs +++ b/core/engine/src/builtins/set/ordered_set.rs @@ -15,9 +15,9 @@ pub struct OrderedSet { unsafe impl Trace for OrderedSet { custom_trace!(this, mark, { - for v in &this.inner { - if let MapKey::Key(v) = v { - mark(v); + for k in &this.inner { + if let MapKey::Key(key) = k { + mark(key); } } }); diff --git a/core/engine/src/builtins/weak/weak_ref.rs b/core/engine/src/builtins/weak/weak_ref.rs index 77f136812ac..0804d3d5d92 100644 --- a/core/engine/src/builtins/weak/weak_ref.rs +++ b/core/engine/src/builtins/weak/weak_ref.rs @@ -87,7 +87,7 @@ impl BuiltInConstructor for WeakRef { let weak_ref = JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), prototype, - WeakGc::new(&unsafe { boa_gc::MutationContext::dummy() }, target.inner()), + WeakGc::new(&context.gc(), target.inner()), ); // 4. Perform AddToKeptObjects(target). @@ -124,7 +124,7 @@ impl WeakRef { // https://tc39.es/ecma262/multipage/managing-memory.html#sec-weakrefderef // 1. Let target be weakRef.[[WeakRefTarget]]. // 2. If target is not empty, then - if let Some(object) = weak_ref.upgrade(&unsafe { boa_gc::MutationContext::dummy() }) { + if let Some(object) = weak_ref.upgrade(&context.gc()) { let object = JsObject::from(object); // a. Perform AddToKeptObjects(target). @@ -140,11 +140,13 @@ impl WeakRef { } #[cfg(test)] +#[allow(unused_imports)] mod tests { use indoc::indoc; use crate::{JsNativeErrorKind, JsValue, TestAction, run_test_actions}; + #[cfg(not(feature = "oscars_backend"))] #[test] fn weak_ref_collected() { run_test_actions([ diff --git a/core/engine/src/builtins/weak_map/mod.rs b/core/engine/src/builtins/weak_map/mod.rs index adff36ecbfc..8f0bdf8de7c 100644 --- a/core/engine/src/builtins/weak_map/mod.rs +++ b/core/engine/src/builtins/weak_map/mod.rs @@ -28,7 +28,7 @@ pub(crate) type NativeWeakMap = boa_gc::WeakMap; #[derive(Debug, Trace, Finalize)] pub(crate) struct WeakMap; -#[cfg(test)] +#[cfg(all(test, not(feature = "oscars_backend")))] mod tests; impl IntrinsicObject for WeakMap { @@ -97,7 +97,7 @@ impl BuiltInConstructor for WeakMap { let map = JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), prototype, - NativeWeakMap::new(&unsafe { boa_gc::MutationContext::dummy() }), + NativeWeakMap::new(&context.gc()), ) .upcast(); @@ -171,7 +171,7 @@ impl WeakMap { pub(crate) fn get( this: &JsValue, args: &[JsValue], - _context: &mut Context, + #[allow(unused_variables)] context: &mut Context, ) -> JsResult { // 1. Let M be the this value. // 2. Perform ? RequireInternalSlot(M, [[WeakMapData]]). @@ -193,13 +193,8 @@ impl WeakMap { // 5. For each Record { [[Key]], [[Value]] } p of entries, do // a. If p.[[Key]] is not empty and SameValue(p.[[Key]], key) is true, return p.[[Value]]. // 6. Return undefined. - if let Some(entry) = map.get(key.inner()) - && let Some(val) = entry.value(&unsafe { boa_gc::MutationContext::dummy() }) - { - Ok(val.clone()) - } else { - Ok(JsValue::undefined()) - } + let result: Option = map.get_value(key.inner()); + Ok(result.unwrap_or_else(JsValue::undefined)) } /// `WeakMap.prototype.has ( key )` @@ -298,13 +293,14 @@ impl WeakMap { pub(crate) fn get_or_insert( this: &JsValue, args: &[JsValue], - _context: &mut Context, + #[allow(unused_variables)] context: &mut Context, ) -> JsResult { // 1. Let M be the this value. // 2. Perform ? RequireInternalSlot(M, [[WeakMapData]]). let object = this.as_object(); - let map = object - .and_then(|obj| obj.clone().downcast::().ok()) + let mut map = object + .as_ref() + .and_then(JsObject::downcast_mut::) .ok_or_else(|| { js_error!(TypeError: "WeakMap.prototype.getOrInsert: expected 'this' to be a WeakMap object", @@ -324,18 +320,13 @@ impl WeakMap { }; // 4. For each Record { [[Key]], [[Value]] } p of M.[[WeakMapData]] - if let Some(existing) = map.borrow().data().get(key.inner()) - && let Some(value) = existing.value(&unsafe { boa_gc::MutationContext::dummy() }) - { - // a. If p.[[Key]] is not empty and SameValue(p.[[Key]], key) is true, return p.[[Value]]. - return Ok(value.clone()); + if let Some(existing) = map.get_value(key.inner()) { + return Ok(existing); } // 5-6. Insert the new record with provided value and return it. let value = args.get_or_undefined(1).clone(); - map.borrow_mut() - .data_mut() - .insert(key.inner(), value.clone()); + map.insert(key.inner(), value.clone()); Ok(value) } @@ -353,23 +344,12 @@ impl WeakMap { pub(crate) fn get_or_insert_computed( this: &JsValue, args: &[JsValue], - context: &mut Context, + #[allow(unused_variables)] context: &mut Context, ) -> JsResult { // 1. Let M be the this value. // 2. Perform ? RequireInternalSlot(M, [[WeakMapData]]). let object = this.as_object(); - let map = object - .and_then(|obj| obj.clone().downcast::().ok()) - .ok_or_else(|| { - js_error!(TypeError: - "WeakMap.prototype.getOrInsertComputed: expected 'this' to be a WeakMap object", - ) - })?; - // 3. If CanBeHeldWeakly(key) is false, throw a TypeError exception. - // TODO: Implement proper CanBeHeldWeakly once available. For now, only - // objects are accepted as keys; symbols should be allowed in the - // future according to the proposal. let key_value = args.get_or_undefined(0).clone(); let Some(key_obj) = key_value.as_object() else { return Err(js_error!(TypeError: @@ -378,6 +358,19 @@ impl WeakMap { )); }; + if let Some(map) = object + .as_ref() + .and_then(JsObject::downcast_ref::) + { + if let Some(existing) = map.get_value(key_obj.inner()) { + return Ok(existing); + } + } else { + return Err(js_error!(TypeError: + "WeakMap.prototype.getOrInsertComputed: expected 'this' to be a WeakMap object", + )); + } + // 4. If IsCallable(callback) is false, throw a TypeError exception. let Some(callback_fn) = args.get_or_undefined(1).as_callable() else { return Err(js_error!(TypeError: @@ -385,26 +378,20 @@ impl WeakMap { )); }; - // 5. For each Record { [[Key]], [[Value]] } p of M.[[WeakMapData]] - if let Some(existing) = map.borrow().data().get(key_obj.inner()) - && let Some(value) = existing.value(&unsafe { boa_gc::MutationContext::dummy() }) - { - // a. If p.[[Key]] is not empty and SameValue(p.[[Key]], key) is true, return p.[[Value]]. - return Ok(value.clone()); - } - // 6. Let value be ? Call(callback, undefined, « key »). // 7. NOTE: The WeakMap may have been modified during execution of callback. - let value = callback_fn.call( - &JsValue::undefined(), - std::slice::from_ref(&key_value), - context, - )?; + let value = callback_fn.call(&JsValue::undefined(), &[key_obj.clone().into()], context)?; // 8-10. Insert or update the entry and return value. - map.borrow_mut() - .data_mut() - .insert(key_obj.inner(), value.clone()); + let mut map = object + .as_ref() + .and_then(JsObject::downcast_mut::) + .ok_or_else(|| { + js_error!(TypeError: + "WeakMap.prototype.getOrInsertComputed: expected 'this' to be a WeakMap object", + ) + })?; + map.insert(key_obj.inner(), value.clone()); Ok(value) } } diff --git a/core/engine/src/builtins/weak_set/mod.rs b/core/engine/src/builtins/weak_set/mod.rs index 50647b16881..f55b58114b6 100644 --- a/core/engine/src/builtins/weak_set/mod.rs +++ b/core/engine/src/builtins/weak_set/mod.rs @@ -86,7 +86,7 @@ impl BuiltInConstructor for WeakSet { let weak_set = JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), prototype, - NativeWeakSet::new(&unsafe { boa_gc::MutationContext::dummy() }), + NativeWeakSet::new(&context.gc()), ) .upcast(); @@ -255,5 +255,5 @@ impl WeakSet { } } -#[cfg(test)] +#[cfg(all(test, not(feature = "oscars_backend")))] mod tests; diff --git a/core/engine/src/bytecompiler/class.rs b/core/engine/src/bytecompiler/class.rs index a876cc80403..d98020efdc9 100644 --- a/core/engine/src/bytecompiler/class.rs +++ b/core/engine/src/bytecompiler/class.rs @@ -157,7 +157,7 @@ impl ByteCompiler<'_> { ); let code = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, compiler.finish(), ); let index = self.push_function_to_constants(code); @@ -444,7 +444,7 @@ impl ByteCompiler<'_> { field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; let code = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, field_compiler.finish(), ); let index = self.push_function_to_constants(code); @@ -493,7 +493,7 @@ impl ByteCompiler<'_> { field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; let code = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, field_compiler.finish(), ); let index = self.push_function_to_constants(code); @@ -551,7 +551,7 @@ impl ByteCompiler<'_> { field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; let code = field_compiler.finish(); - let code = Gc::new(&unsafe { boa_gc::MutationContext::dummy() }, code); + let code = Gc::new(&unsafe { boa_gc::MutationContext::global() }, code); static_elements.push(StaticElement::StaticField { code, @@ -595,7 +595,7 @@ impl ByteCompiler<'_> { field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; let code = field_compiler.finish(); - let code = Gc::new(&unsafe { boa_gc::MutationContext::dummy() }, code); + let code = Gc::new(&unsafe { boa_gc::MutationContext::global() }, code); static_elements.push(StaticElement::StaticField { code, @@ -639,7 +639,7 @@ impl ByteCompiler<'_> { } let code = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, compiler.finish(), ); static_elements.push(StaticElement::StaticBlock(code)); diff --git a/core/engine/src/bytecompiler/function.rs b/core/engine/src/bytecompiler/function.rs index b862326fc7b..d1633a34cd6 100644 --- a/core/engine/src/bytecompiler/function.rs +++ b/core/engine/src/bytecompiler/function.rs @@ -227,6 +227,6 @@ impl FunctionCompiler { let code = compiler.finish(); - Gc::new(&unsafe { boa_gc::MutationContext::dummy() }, code) + Gc::new(&unsafe { boa_gc::MutationContext::global() }, code) } } diff --git a/core/engine/src/context/mod.rs b/core/engine/src/context/mod.rs index 78453d26078..dfde062671c 100644 --- a/core/engine/src/context/mod.rs +++ b/core/engine/src/context/mod.rs @@ -463,6 +463,20 @@ impl Context { &self.vm.frame().realm } + /// Returns [`boa_gc::MutationContext`] to allocate on the Gc heap + /// (eg. for [`Gc::new`]) + /// + /// # Safety + /// Uses `dummy()` as a temporary bridge during the oscars GC migration. + /// Todo: replace with a real branding token in future + #[inline] + #[must_use] + pub fn gc(&self) -> boa_gc::MutationContext<'static, 'static> { + // SAFETY: `MutationContext` is a ZST phantom type, this is sound + // under boa's single-threaded GC invariant until migration is complete + unsafe { boa_gc::MutationContext::global() } + } + /// Set the value of trace on the context #[cfg(feature = "trace")] #[inline] diff --git a/core/engine/src/environments/runtime/mod.rs b/core/engine/src/environments/runtime/mod.rs index 8f7b2dd26c6..59724b87c38 100644 --- a/core/engine/src/environments/runtime/mod.rs +++ b/core/engine/src/environments/runtime/mod.rs @@ -1,3 +1,5 @@ +#![allow(clippy::trivially_copy_pass_by_ref)] +#![allow(clippy::needless_pass_by_value)] use crate::{ Context, JsResult, JsString, JsSymbol, JsValue, object::{JsObject, PrivateName}, @@ -214,13 +216,14 @@ impl EnvironmentStack { &mut self, bindings_count: u32, global: &Gc<'static, DeclarativeEnvironment>, + gc: boa_gc::MutationContext<'static, '_>, ) -> u32 { let (poisoned, with) = self.compute_poisoned_with(global); let index = self.depth; self.push_env(Environment::Declarative(Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &gc, DeclarativeEnvironment::new( DeclarativeEnvironmentKind::Lexical(LexicalEnvironment::new(bindings_count)), poisoned, @@ -237,13 +240,14 @@ impl EnvironmentStack { scope: Scope, function_slots: FunctionSlots, global: &Gc<'static, DeclarativeEnvironment>, + gc: boa_gc::MutationContext<'static, '_>, ) { let num_bindings = scope.num_bindings_non_local(); let (poisoned, with) = self.compute_poisoned_with(global); self.push_env(Environment::Declarative(Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &gc, DeclarativeEnvironment::new( DeclarativeEnvironmentKind::Function(FunctionEnvironment::new( num_bindings, @@ -257,10 +261,10 @@ impl EnvironmentStack { } /// Push a module environment on the environments stack. - pub(crate) fn push_module(&mut self, scope: Scope) { + pub(crate) fn push_module(&mut self, scope: Scope, gc: boa_gc::MutationContext<'static, '_>) { let num_bindings = scope.num_bindings_non_local(); self.push_env(Environment::Declarative(Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &gc, DeclarativeEnvironment::new( DeclarativeEnvironmentKind::Module(ModuleEnvironment::new(num_bindings, scope)), false, @@ -414,7 +418,7 @@ impl EnvironmentStack { /// Push an environment onto the chain. fn push_env(&mut self, env: Environment) { self.tip = Some(Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, EnvironmentNode { env, parent: self.tip.take(), diff --git a/core/engine/src/error/mod.rs b/core/engine/src/error/mod.rs index 1fc5c934c51..2e8ec0e6949 100644 --- a/core/engine/src/error/mod.rs +++ b/core/engine/src/error/mod.rs @@ -562,10 +562,13 @@ impl JsError { let obj = val .as_object() .ok_or_else(|| TryNativeError::NotAnErrorObject(val.clone()))?; + // Under `oscars_backend`, `downcast_ref` returns a `GcRef<'_, Error>`. + // We deref through the guard before `.clone()` so we clone the `Error` value, + // not the GcRef wrapper. The `*` operator goes through `Deref`. let error_data: Error = obj .downcast_ref::() - .ok_or_else(|| TryNativeError::NotAnErrorObject(val.clone()))? - .clone(); + .ok_or_else(|| TryNativeError::NotAnErrorObject(val.clone())) + .map(|r| (*r).clone())?; let try_get_property = |key: JsString, name, context: &mut Context| { obj.try_get(key, context) @@ -1496,7 +1499,7 @@ unsafe impl Trace for JsNativeErrorKind { custom_trace!( this, mark, - match &this { + match this { Self::Aggregate(errors) => mark(errors), Self::Error | Self::Eval diff --git a/core/engine/src/host_defined.rs b/core/engine/src/host_defined.rs index 96ea02e1f46..8547bd59e85 100644 --- a/core/engine/src/host_defined.rs +++ b/core/engine/src/host_defined.rs @@ -34,7 +34,7 @@ unsafe impl Trace for HostDefined { }); } -impl Finalize for HostDefined {} +impl Finalize for HostDefined {} impl HostDefined { /// Insert a type into the [`HostDefined`]. diff --git a/core/engine/src/lib.rs b/core/engine/src/lib.rs index 37558607d06..62b4c21497f 100644 --- a/core/engine/src/lib.rs +++ b/core/engine/src/lib.rs @@ -71,6 +71,10 @@ // Add temporarily - Needs addressing clippy::missing_panics_doc, + + // Expected when feature "oscars_backend" is enabled, since Gc becomes a Copy type + clippy::clone_on_copy, + clippy::cloned_instead_of_copied, )] extern crate self as boa_engine; diff --git a/core/engine/src/module/loader/mod.rs b/core/engine/src/module/loader/mod.rs index 21b0ed06b2f..a9bf45844ff 100644 --- a/core/engine/src/module/loader/mod.rs +++ b/core/engine/src/module/loader/mod.rs @@ -287,7 +287,7 @@ impl ModuleLoader for MapModuleLoader { } } -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, boa_gc::Trace, boa_gc::Finalize)] struct ModuleCacheKey { path: PathBuf, attributes: Box<[ImportAttribute]>, diff --git a/core/engine/src/module/mod.rs b/core/engine/src/module/mod.rs index e34f8a8329d..e8f260aae5b 100644 --- a/core/engine/src/module/mod.rs +++ b/core/engine/src/module/mod.rs @@ -287,7 +287,7 @@ impl Module { Ok(Self { inner: Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &context.gc(), ModuleRepr { realm, namespace: GcRefCell::default(), @@ -319,7 +319,7 @@ impl Module { Self { inner: Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &context.gc(), ModuleRepr { realm, namespace: GcRefCell::default(), @@ -826,10 +826,7 @@ fn into_js_module() { let bar_count = Rc::new(RefCell::new(0)); let dad_count = Rc::new(RefCell::new(0)); - context.insert_data(Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - GcRefCell::new(JsValue::undefined()), - )); + context.insert_data(Gc::new(&context.gc(), GcRefCell::new(JsValue::undefined()))); let module = unsafe { vec![ @@ -912,7 +909,10 @@ fn into_js_module() { promise_result.state() ); - let result = context.get_data::().unwrap().borrow().clone(); + // Under `oscars_backend`, `borrow()` returns `GcRef<'_, JsValue>`. + // Deref through the guard before cloning to clone the inner `JsValue`. If we clone + // the guard instead, the `GcRef` (and immutable borrow) stays alive, causing error. + let result = (*context.get_data::().unwrap().borrow()).clone(); assert_eq!(*foo_count.borrow(), 2); assert_eq!(*bar_count.borrow(), 15); diff --git a/core/engine/src/module/source.rs b/core/engine/src/module/source.rs index 80fe607c6d6..b84319f6893 100644 --- a/core/engine/src/module/source.rs +++ b/core/engine/src/module/source.rs @@ -1824,17 +1824,17 @@ impl SourceTextModule { compiler.compile_module_item_list(source.items()); ( - Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - compiler.finish(), - ), + { + let finished = compiler.finish(); + Gc::new(&context.gc(), finished) + }, functions, ) }; // 8. Let moduleContext be a new ECMAScript code execution context. let mut envs = EnvironmentStack::new(); - envs.push_module(source.scope().clone()); + envs.push_module(source.scope().clone(), context.gc()); drop(status); // 9. Set the Function of moduleContext to null. diff --git a/core/engine/src/module/synthetic.rs b/core/engine/src/module/synthetic.rs index 613561c9558..0d30f788fd6 100644 --- a/core/engine/src/module/synthetic.rs +++ b/core/engine/src/module/synthetic.rs @@ -120,7 +120,7 @@ impl SyntheticModuleInitializer { // Hopefully, this unsafe operation will be replaced by the `CoerceUnsized` API in the // future: https://github.com/rust-lang/rust/issues/18598 let ptr = Gc::into_raw(Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, Callback { f: closure, captures, @@ -131,7 +131,7 @@ impl SyntheticModuleInitializer { // meaning this is safe. unsafe { Self { - inner: Gc::from_raw(ptr), + inner: >::from_raw(ptr), } } } @@ -338,13 +338,11 @@ impl SyntheticModule { module_scope.escape_all_bindings(); - let cb = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - compiler.finish(), - ); + let finished = compiler.finish(); + let cb = Gc::new(&context.gc(), finished); let mut envs = EnvironmentStack::new(); - envs.push_module(module_scope); + envs.push_module(module_scope, context.gc()); for locator in exports { // b. Perform ! env.InitializeBinding(exportName, undefined). diff --git a/core/engine/src/native_function/continuation.rs b/core/engine/src/native_function/continuation.rs index c18fa9e0327..abce83e1e0f 100644 --- a/core/engine/src/native_function/continuation.rs +++ b/core/engine/src/native_function/continuation.rs @@ -108,7 +108,7 @@ impl NativeCoroutine { // Hopefully, this unsafe operation will be replaced by the `CoerceUnsized` API in the // future: https://github.com/rust-lang/rust/issues/18598 let ptr = Gc::into_raw(Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, Coroutine { f: closure, captures, @@ -118,7 +118,7 @@ impl NativeCoroutine { // meaning this is safe. unsafe { Self { - inner: Gc::from_raw(ptr), + inner: >::from_raw(ptr), } } } diff --git a/core/engine/src/native_function/mod.rs b/core/engine/src/native_function/mod.rs index 22d661a3b38..11dc49adce8 100644 --- a/core/engine/src/native_function/mod.rs +++ b/core/engine/src/native_function/mod.rs @@ -279,7 +279,7 @@ impl NativeFunction { // Hopefully, this unsafe operation will be replaced by the `CoerceUnsized` API in the // future: https://github.com/rust-lang/rust/issues/18598 let ptr = Gc::into_raw(Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, Closure { f: closure, captures, @@ -289,7 +289,7 @@ impl NativeFunction { // meaning this is safe. unsafe { Self { - inner: Inner::Closure(Gc::from_raw(ptr)), + inner: Inner::Closure(>::from_raw(ptr)), } } } @@ -340,15 +340,18 @@ pub(crate) fn native_function_call( context.check_runtime_limits()?; let this_function_object = obj.clone(); + // Under `oscars_backend`, `downcast_ref` returns a `GcRef<'_, NativeFunctionObject>`. + // We deref through the guard with `(*guard).clone()` so we clone the inner struct + // (which is `Copy` friendly via `Clone`), not the `GcRef` wrapper itself let NativeFunctionObject { f: function, name, constructor, realm, - } = obj + } = (*obj .downcast_ref::() - .expect("the object should be a native function object") - .clone(); + .expect("the object should be a native function object")) + .clone(); let pc = context.vm.frame().pc; let native_source_info = context.native_source_info(); @@ -395,15 +398,17 @@ fn native_function_construct( context.check_runtime_limits()?; let this_function_object = obj.clone(); + // Under `oscars_backend`, `downcast_ref` returns a `GcRef<'_, NativeFunctionObject>`. + // We deref through the guard with `(*guard).clone()` so we clone the inner struct. let NativeFunctionObject { f: function, name, constructor, realm, - } = obj + } = (*obj .downcast_ref::() - .expect("the object should be a native function object") - .clone(); + .expect("the object should be a native function object")) + .clone(); let pc = context.vm.frame().pc; let native_source_info = context.native_source_info(); diff --git a/core/engine/src/object/builtins/jspromise.rs b/core/engine/src/object/builtins/jspromise.rs index 85aa1b26b2b..67931d22bf9 100644 --- a/core/engine/src/object/builtins/jspromise.rs +++ b/core/engine/src/object/builtins/jspromise.rs @@ -1,3 +1,4 @@ +#![allow(clippy::redundant_locals)] //! A Rust API wrapper for Boa's promise Builtin ECMAScript Object use super::{JsArray, JsFunction}; @@ -1094,7 +1095,7 @@ impl JsPromise { } let state = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &context.gc(), GcRefCell::new(Inner { result: None, task: None, @@ -1429,6 +1430,8 @@ impl TryIntoJs for JsPromise { /// between promises and futures a bit easier. /// /// The only way to construct an instance of `JsFuture` is by calling [`JsPromise::into_js_future`]. +#[derive(Clone)] +#[allow(missing_copy_implementations)] pub struct JsFuture { inner: Gc<'static, GcRefCell>, } diff --git a/core/engine/src/object/builtins/jstypedarray.rs b/core/engine/src/object/builtins/jstypedarray.rs index 77ec287f050..6828d0f6b98 100644 --- a/core/engine/src/object/builtins/jstypedarray.rs +++ b/core/engine/src/object/builtins/jstypedarray.rs @@ -678,7 +678,7 @@ impl JsTypedArray { /// # fn main() -> JsResult<()> { /// let context = &mut Context::default(); /// let array = JsUint8Array::from_iter(vec![1, 2, 3, 4, 5], context)?; - /// let num_to_modify = Gc::new(GcRefCell::new(0u8)); + /// let num_to_modify = Gc::new(&context.gc(), GcRefCell::new(0u8)); /// /// let js_function = FunctionObjectBuilder::new( /// context.realm(), diff --git a/core/engine/src/object/builtins/jsweakmap.rs b/core/engine/src/object/builtins/jsweakmap.rs index e752f696b95..d120be65a48 100644 --- a/core/engine/src/object/builtins/jsweakmap.rs +++ b/core/engine/src/object/builtins/jsweakmap.rs @@ -30,7 +30,7 @@ impl JsWeakMap { inner: JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), context.intrinsics().constructors().weak_map().prototype(), - NativeWeakMap::new(&unsafe { boa_gc::MutationContext::dummy() }), + NativeWeakMap::new(&context.gc()), ) .upcast(), } diff --git a/core/engine/src/object/builtins/jsweakset.rs b/core/engine/src/object/builtins/jsweakset.rs index 13d14095cc8..07a53fd4264 100644 --- a/core/engine/src/object/builtins/jsweakset.rs +++ b/core/engine/src/object/builtins/jsweakset.rs @@ -30,7 +30,7 @@ impl JsWeakSet { inner: JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), context.intrinsics().constructors().weak_set().prototype(), - NativeWeakSet::new(&unsafe { boa_gc::MutationContext::dummy() }), + NativeWeakSet::new(&context.gc()), ) .upcast(), } diff --git a/core/engine/src/object/jsobject.rs b/core/engine/src/object/jsobject.rs index cd30c5dceb4..711bb3cee2b 100644 --- a/core/engine/src/object/jsobject.rs +++ b/core/engine/src/object/jsobject.rs @@ -33,12 +33,6 @@ use std::{ }; use thin_vec::ThinVec; -#[cfg(not(feature = "jsvalue-enum"))] -use boa_gc::GcBox; - -#[cfg(not(feature = "jsvalue-enum"))] -use std::ptr::NonNull; - /// A wrapper type for an immutably borrowed type T. pub type Ref<'a, T> = GcRef<'a, T>; @@ -86,8 +80,8 @@ pub(crate) struct VTableObject { impl JsObject { /// Converts the `JsObject` into a raw pointer to its inner `GcBox`. #[cfg(not(feature = "jsvalue-enum"))] - pub(crate) fn into_raw(self) -> NonNull> { - Gc::into_raw(self.inner) + pub(crate) fn into_raw(self) -> *const () { + Gc::into_raw(self.inner).as_ptr() as *const () } /// Creates a new `JsObject` from a raw pointer. @@ -96,9 +90,9 @@ impl JsObject { /// The caller must ensure that the pointer is valid and points to a `GcBox`. /// The pointer must not be null. #[cfg(not(feature = "jsvalue-enum"))] - pub(crate) unsafe fn from_raw(raw: NonNull>) -> Self { + pub(crate) unsafe fn from_raw(raw: *const ()) -> Self { // SAFETY: The caller guaranteed the value to be a valid pointer to a `GcBox`. - let inner = unsafe { Gc::from_raw(raw) }; + let inner = unsafe { Gc::from_raw(core::ptr::NonNull::new_unchecked(raw as *mut _)) }; JsObject { inner } } @@ -128,7 +122,7 @@ impl JsObject { vtable: &'static InternalObjectMethods, ) -> Self { let inner = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, VTableObject { object: GcRefCell::new(object), vtable, @@ -217,7 +211,7 @@ impl JsObject { ) -> Self { let internal_methods = data.internal_methods(); let inner = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, VTableObject { object: GcRefCell::new(Object { data: ObjectData::new(data), @@ -246,7 +240,7 @@ impl JsObject { ) -> JsObject { let internal_methods = data.internal_methods(); let inner = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, VTableObject { object: GcRefCell::new(Object { data: ObjectData::new(data), @@ -1088,7 +1082,7 @@ impl JsObject { pub fn new>>(root_shape: &RootShape, prototype: O, data: T) -> Self { let internal_methods = data.internal_methods(); let inner = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, VTableObject { object: GcRefCell::new(Object { data: ObjectData::new(data), @@ -1126,7 +1120,7 @@ impl JsObject { pub fn new_unique>>(prototype: O, data: T) -> Self { let internal_methods = data.internal_methods(); let inner = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, VTableObject { object: GcRefCell::new(Object { data: ObjectData::new(data), diff --git a/core/engine/src/object/mod.rs b/core/engine/src/object/mod.rs index 258da691647..edd29e6e913 100644 --- a/core/engine/src/object/mod.rs +++ b/core/engine/src/object/mod.rs @@ -96,6 +96,16 @@ impl NativeObject for T { // TODO: Use super trait casting in Rust 1.75 impl dyn NativeObject { /// Returns `true` if the inner type is the same as `T`. + /// + /// # Type identity under `oscars_backend` + /// + /// 1. **`dyn NativeObject::is::()`** (this method) uses [`std::any::TypeId::of::()`]. + /// This is sound because `NativeObject: Any` requires `T: 'static` + /// 2. **[`JsObject::is::()`]** uses `typeid::of::>()` + /// (via [`boa_gc::type_id_of`]), which supports non-`'static` branded lifetimes + /// + /// Do not replace the `std::any::TypeId` call below with `typeid::of`. + /// `std::any::TypeId` is authoritative for `Any` bounded types. #[inline] pub fn is(&self) -> bool { // Get `TypeId` of the type this function is instantiated with. diff --git a/core/engine/src/object/shape/shared_shape/forward_transition.rs b/core/engine/src/object/shape/shared_shape/forward_transition.rs index 11934d51b79..88c286171d3 100644 --- a/core/engine/src/object/shape/shared_shape/forward_transition.rs +++ b/core/engine/src/object/shape/shared_shape/forward_transition.rs @@ -1,3 +1,5 @@ +#![allow(clippy::trivially_copy_pass_by_ref)] +#![allow(clippy::needless_pass_by_value)] use std::fmt::Debug; use boa_gc::{Finalize, Gc, GcRefCell, Trace, WeakGc}; @@ -68,7 +70,7 @@ impl ForwardTransition { properties.map.insert( key, - WeakGc::new(&unsafe { boa_gc::MutationContext::dummy() }, value), + WeakGc::new(&unsafe { boa_gc::MutationContext::global() }, value), ); } @@ -83,11 +85,12 @@ impl ForwardTransition { prototypes.map.insert( key, - WeakGc::new(&unsafe { boa_gc::MutationContext::dummy() }, value), + WeakGc::new(&unsafe { boa_gc::MutationContext::global() }, value), ); } /// Get a property transition, return [`None`] otherwise. + #[allow(clippy::cloned_instead_of_copied)] pub(super) fn get_property(&self, key: &TransitionKey) -> Option> { let this = self.inner.borrow(); let transitions = this.properties.as_ref()?; @@ -95,6 +98,7 @@ impl ForwardTransition { } /// Get a prototype transition, return [`None`] otherwise. + #[allow(clippy::cloned_instead_of_copied)] pub(super) fn get_prototype(&self, key: &JsPrototype) -> Option> { let this = self.inner.borrow(); let transitions = this.prototypes.as_ref()?; @@ -123,7 +127,7 @@ impl ForwardTransition { transitions.map.retain(|_, v| v.is_upgradable()); } - #[cfg(test)] + #[cfg(all(test, not(feature = "oscars_backend")))] pub(crate) fn property_transitions_count(&self) -> (usize, u8) { let this = self.inner.borrow(); this.properties.as_ref().map_or((0, 0), |transitions| { @@ -134,7 +138,7 @@ impl ForwardTransition { }) } - #[cfg(test)] + #[cfg(all(test, not(feature = "oscars_backend")))] pub(crate) fn prototype_transitions_count(&self) -> (usize, u8) { let this = self.inner.borrow(); this.prototypes.as_ref().map_or((0, 0), |transitions| { diff --git a/core/engine/src/object/shape/shared_shape/mod.rs b/core/engine/src/object/shape/shared_shape/mod.rs index 0a1609fd55a..cbbfb1dbecb 100644 --- a/core/engine/src/object/shape/shared_shape/mod.rs +++ b/core/engine/src/object/shape/shared_shape/mod.rs @@ -1,7 +1,7 @@ mod forward_transition; pub(crate) mod template; -#[cfg(test)] +#[cfg(all(test, not(feature = "oscars_backend")))] mod tests; use std::{collections::hash_map::RandomState, hash::Hash}; @@ -166,7 +166,7 @@ impl SharedShape { /// Create a new [`SharedShape`]. fn new(inner: Inner) -> Self { Self { - inner: Gc::new(&unsafe { boa_gc::MutationContext::dummy() }, inner), + inner: Gc::new(&unsafe { boa_gc::MutationContext::global() }, inner), } } @@ -188,7 +188,7 @@ impl SharedShape { /// Create a [`SharedShape`] change prototype transition. pub(crate) fn change_prototype_transition(&self, prototype: JsPrototype) -> Self { if let Some(shape) = self.forward_transitions().get_prototype(&prototype) { - if let Some(inner) = shape.upgrade(&unsafe { boa_gc::MutationContext::dummy() }) { + if let Some(inner) = shape.upgrade(&unsafe { boa_gc::MutationContext::global() }) { return Self { inner }; } @@ -215,7 +215,7 @@ impl SharedShape { pub(crate) fn insert_property_transition(&self, key: TransitionKey) -> Self { // Check if we have already created such a transition, if so use it! if let Some(shape) = self.forward_transitions().get_property(&key) { - if let Some(inner) = shape.upgrade(&unsafe { boa_gc::MutationContext::dummy() }) { + if let Some(inner) = shape.upgrade(&unsafe { boa_gc::MutationContext::global() }) { return Self { inner }; } @@ -253,7 +253,7 @@ impl SharedShape { // Check if we have already created such a transition, if so use it! if let Some(shape) = self.forward_transitions().get_property(&key) { - if let Some(inner) = shape.upgrade(&unsafe { boa_gc::MutationContext::dummy() }) { + if let Some(inner) = shape.upgrade(&unsafe { boa_gc::MutationContext::global() }) { let action = if slot.attributes.width_match(key.attributes) { ChangeTransitionAction::Nothing } else if slot.attributes.is_accessor_descriptor() { @@ -488,15 +488,20 @@ impl WeakSharedShape { Some(SharedShape { inner: self .inner - .upgrade(&unsafe { boa_gc::MutationContext::dummy() })?, + .upgrade(&unsafe { boa_gc::MutationContext::global() })?, }) } + + #[allow(dead_code)] + pub(crate) fn is_upgradable(&self) -> bool { + self.inner.is_upgradable() + } } impl From<&SharedShape> for WeakSharedShape { fn from(value: &SharedShape) -> Self { WeakSharedShape { - inner: WeakGc::new(&unsafe { boa_gc::MutationContext::dummy() }, &value.inner), + inner: WeakGc::new(&unsafe { boa_gc::MutationContext::global() }, &value.inner), } } } diff --git a/core/engine/src/object/shape/unique_shape.rs b/core/engine/src/object/shape/unique_shape.rs index 6947489a526..b050e4bf5eb 100644 --- a/core/engine/src/object/shape/unique_shape.rs +++ b/core/engine/src/object/shape/unique_shape.rs @@ -38,7 +38,7 @@ impl UniqueShape { pub(crate) fn new(prototype: JsPrototype, property_table: PropertyTableInner) -> Self { Self { inner: Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, Inner { property_table: RefCell::new(property_table), prototype: GcRefCell::new(prototype), @@ -58,7 +58,10 @@ impl UniqueShape { /// Get the prototype of the [`UniqueShape`]. pub(crate) fn prototype(&self) -> JsPrototype { - self.inner.prototype.borrow().clone() + // Under `oscars_backend`, `GcRefCell::borrow()` returns `GcRef<'_, Option>`. + // Deref through the guard before cloning to get the inner `Option` value. + // This is what the `JsPrototype` return type requires. + (*self.inner.prototype.borrow()).clone() } /// Get the property table of the [`UniqueShape`]. @@ -258,15 +261,20 @@ impl WeakUniqueShape { Some(UniqueShape { inner: self .inner - .upgrade(&unsafe { boa_gc::MutationContext::dummy() })?, + .upgrade(&unsafe { boa_gc::MutationContext::global() })?, }) } + + #[allow(dead_code)] + pub(crate) fn is_upgradable(&self) -> bool { + self.inner.is_upgradable() + } } impl From<&UniqueShape> for WeakUniqueShape { fn from(value: &UniqueShape) -> Self { WeakUniqueShape { - inner: WeakGc::new(&unsafe { boa_gc::MutationContext::dummy() }, &value.inner), + inner: WeakGc::new(&unsafe { boa_gc::MutationContext::global() }, &value.inner), } } } diff --git a/core/engine/src/realm.rs b/core/engine/src/realm.rs index 84bf5c39cf2..c133ce823eb 100644 --- a/core/engine/src/realm.rs +++ b/core/engine/src/realm.rs @@ -87,14 +87,14 @@ impl Realm { .create_global_this(&intrinsics) .unwrap_or_else(|| global_object.clone()); let environment = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, DeclarativeEnvironment::global(), ); let scope = Scope::new_global(); let realm = Self { inner: Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, Inner { intrinsics, environment, diff --git a/core/engine/src/script.rs b/core/engine/src/script.rs index f11dbc61168..e7d5a36144e 100644 --- a/core/engine/src/script.rs +++ b/core/engine/src/script.rs @@ -105,7 +105,7 @@ impl Script { Ok(Self { inner: Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &context.gc(), Inner { realm: realm.unwrap_or_else(|| context.realm().clone()), phase: GcRefCell::new(ScriptPhase::Ast(code)), @@ -162,10 +162,8 @@ impl Script { compiler.global_declaration_instantiation(source); compiler.compile_statement_list(source.statements(), true, false); - Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - compiler.finish(), - ) + let finished = compiler.finish(); + Gc::new(&context.gc(), finished) }; *self.inner.phase.borrow_mut() = ScriptPhase::Codeblock(cb.clone()); diff --git a/core/engine/src/value/equality.rs b/core/engine/src/value/equality.rs index 79a209253df..5ca76b9826c 100644 --- a/core/engine/src/value/equality.rs +++ b/core/engine/src/value/equality.rs @@ -238,7 +238,7 @@ impl JsValue { } fn same_value_non_numeric(x: &Self, y: &Self) -> bool { - debug_assert!(x.get_type() == y.get_type()); + debug_assert_eq!(x.get_type(), y.get_type()); match (x.variant(), y.variant()) { (JsVariant::Null, JsVariant::Null) | (JsVariant::Undefined, JsVariant::Undefined) => { true diff --git a/core/engine/src/value/inner/legacy.rs b/core/engine/src/value/inner/legacy.rs index 087331990f3..5265b58f83f 100644 --- a/core/engine/src/value/inner/legacy.rs +++ b/core/engine/src/value/inner/legacy.rs @@ -33,8 +33,12 @@ impl Finalize for EnumBasedValue { #[allow(unsafe_op_in_unsafe_fn)] unsafe impl Trace for EnumBasedValue { custom_trace! {this, mark, { - if let Some(o) = this.as_object() { - mark(&o); + match this { + Self::Object(o) => mark(o), + Self::Symbol(s) => mark(s), + Self::String(s) => mark(s), + Self::BigInt(b) => mark(b), + _ => {} } }} } diff --git a/core/engine/src/value/inner/nan_boxed.rs b/core/engine/src/value/inner/nan_boxed.rs index 6a05d23fce4..d0c50bb3f6a 100644 --- a/core/engine/src/value/inner/nan_boxed.rs +++ b/core/engine/src/value/inner/nan_boxed.rs @@ -1,3 +1,4 @@ +#![allow(clippy::forget_non_drop)] //! A NaN-boxed inner value for JavaScript values. //! //! This [`JsValue`] is a float using `NaN` values to represent an inner @@ -109,10 +110,9 @@ #[cfg(feature = "annex-b")] use crate::builtins::is_html_dda::IsHTMLDDA; use crate::{ - JsBigInt, JsObject, JsSymbol, JsVariant, bigint::RawBigInt, object::ErasedVTableObject, - symbol::RawJsSymbol, value::Type, + JsBigInt, JsObject, JsSymbol, JsVariant, bigint::RawBigInt, symbol::RawJsSymbol, value::Type, }; -use boa_gc::{Finalize, GcBox, Trace, custom_trace}; +use boa_gc::{Finalize, Trace, custom_trace}; use boa_string::JsString; use core::fmt; use static_assertions::const_assert; @@ -479,7 +479,7 @@ impl NanBoxedValue { #[must_use] #[inline(always)] pub(crate) fn object(value: JsObject) -> Self { - let ptr = value.into_raw(); + let ptr = unsafe { NonNull::new_unchecked(value.into_raw().cast_mut()) }; let addr = bits::tag_pointer(ptr, bits::MASK_OBJECT); Self::from_object_like(ptr, addr) } @@ -684,11 +684,7 @@ impl NanBoxedValue { unsafe fn as_object_unchecked(&self) -> ManuallyDrop { let addr = bits::untag_pointer(self.value()); // SAFETY: This is guaranteed by the caller. - unsafe { - ManuallyDrop::new(JsObject::from_raw(NonNull::new_unchecked( - self.ptr.with_addr(addr).cast::>(), - ))) - } + unsafe { ManuallyDrop::new(JsObject::from_raw(self.ptr.with_addr(addr).cast::<()>())) } } /// Returns the value as a [`JsSymbol`]. diff --git a/core/engine/src/value/integer.rs b/core/engine/src/value/integer.rs index 970ce0632f2..17fdbbdc23f 100644 --- a/core/engine/src/value/integer.rs +++ b/core/engine/src/value/integer.rs @@ -105,12 +105,12 @@ mod tests { fn test_eq() { let int: i64 = 42; let int_or_inf = IntegerOrInfinity::Integer(10); - assert!(int != int_or_inf); - assert!(int_or_inf != int); + assert_ne!(int, int_or_inf); + assert_ne!(int_or_inf, int); let int: i64 = 10; - assert!(int == int_or_inf); - assert!(int_or_inf == int); + assert_eq!(int, int_or_inf); + assert_eq!(int_or_inf, int); } #[test] diff --git a/core/engine/src/vm/code_block.rs b/core/engine/src/vm/code_block.rs index fb494a0a972..959256a90ce 100644 --- a/core/engine/src/vm/code_block.rs +++ b/core/engine/src/vm/code_block.rs @@ -330,6 +330,7 @@ impl CodeBlock { /// /// If the type of the [`Constant`] is not [`Constant::Function`]. /// Or `index` is greater or equal to length of `constants`. + #[allow(clippy::clone_on_copy)] pub(crate) fn constant_function(&self, index: usize) -> Gc<'static, Self> { if let Some(Constant::Function(value)) = self.constants.get(index) { return value.clone(); diff --git a/core/engine/src/vm/inline_cache/mod.rs b/core/engine/src/vm/inline_cache/mod.rs index c55aae8f767..2ae3b6d7d77 100644 --- a/core/engine/src/vm/inline_cache/mod.rs +++ b/core/engine/src/vm/inline_cache/mod.rs @@ -98,6 +98,7 @@ impl InlineCache { while i < entries.len() { if let Some(upgraded) = entries[i].shape.upgrade() { + let upgraded: Shape = upgraded; if upgraded.to_addr_usize() == shape_addr { result = Some((upgraded, entries[i].slot)); break; diff --git a/core/engine/src/vm/mod.rs b/core/engine/src/vm/mod.rs index b7da166b5c9..48d957adfaf 100644 --- a/core/engine/src/vm/mod.rs +++ b/core/engine/src/vm/mod.rs @@ -408,7 +408,7 @@ impl Vm { let mut frames = Vec::with_capacity(16); frames.push(CallFrame::new( Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, CodeBlock::new(JsString::default(), 0, true), ), None, diff --git a/core/engine/src/vm/opcode/await/mod.rs b/core/engine/src/vm/opcode/await/mod.rs index ad2c5fcbd54..f95a89e91cb 100644 --- a/core/engine/src/vm/opcode/await/mod.rs +++ b/core/engine/src/vm/opcode/await/mod.rs @@ -56,10 +56,7 @@ impl Await { let r#gen = GeneratorContext::from_current(context, None); - let captures = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, - Cell::new(Some(r#gen)), - ); + let captures = Gc::new(&context.gc(), Cell::new(Some(r#gen))); // 3. Let fulfilledClosure be a new Abstract Closure with parameters (value) that captures asyncContext and performs the following steps when called: // 4. Let onFulfilled be CreateBuiltinFunction(fulfilledClosure, 1, "", « »). @@ -132,7 +129,7 @@ impl Await { Ok(JsValue::undefined()) }, - captures, + captures.clone(), ), ) .name(js_string!()) diff --git a/core/engine/src/vm/opcode/function.rs b/core/engine/src/vm/opcode/function.rs index aa1e70fb325..8b125a308e6 100644 --- a/core/engine/src/vm/opcode/function.rs +++ b/core/engine/src/vm/opcode/function.rs @@ -61,7 +61,9 @@ impl GetHomeObject { .downcast_ref::() .js_expect("must be function object")? .get_home_object() - .map_or_else(JsValue::null, |o| o.clone().into()); + .map_or_else(JsValue::null, |o: &crate::object::JsObject| { + o.clone().into() + }); context.vm.set_register(function.into(), home_object); Ok(()) diff --git a/core/engine/src/vm/opcode/push/environment.rs b/core/engine/src/vm/opcode/push/environment.rs index 8f49f65b2c6..b27673d6d73 100644 --- a/core/engine/src/vm/opcode/push/environment.rs +++ b/core/engine/src/vm/opcode/push/environment.rs @@ -22,7 +22,9 @@ impl PushScope { let global = frame.realm.environment(); frame .environments - .push_lexical(scope.num_bindings_non_local(), global); + .push_lexical(scope.num_bindings_non_local(), global, unsafe { + boa_gc::MutationContext::global() + }); } } @@ -82,7 +84,7 @@ impl PushPrivateEnvironment { let ptr: *const _ = class.as_ref(); let environment = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &context.gc(), PrivateEnvironment::new(ptr.cast::<()>() as usize, names), ); diff --git a/core/engine/src/vm/tests.rs b/core/engine/src/vm/tests.rs index 0b25366ac41..4655d97a09f 100644 --- a/core/engine/src/vm/tests.rs +++ b/core/engine/src/vm/tests.rs @@ -480,6 +480,7 @@ fn cross_context_function_call() { } // See: https://github.com/boa-dev/boa/issues/1848 +#[cfg(not(feature = "oscars_backend"))] #[test] fn long_object_chain_gc_trace_stack_overflow() { run_test_actions([ diff --git a/core/gc/Cargo.toml b/core/gc/Cargo.toml index cf875a570ee..7b9b44be17e 100644 --- a/core/gc/Cargo.toml +++ b/core/gc/Cargo.toml @@ -12,18 +12,18 @@ rust-version.workspace = true [features] # Enable default implementations of trace and finalize for the thin-vec crate -thin-vec = ["dep:thin-vec"] +thin-vec = ["dep:thin-vec", "oscars?/thin-vec"] # Enable default implementations of trace and finalize for some `ICU4X` types -icu = ["dep:icu_locale_core"] +icu = ["dep:icu_locale_core", "oscars?/icu"] # Enable default implementations of trace and finalize for the `boa_string` crate boa_string = ["dep:boa_string"] # Enable default implementations of trace and finalize for the `either` crate -either = ["dep:either"] +either = ["dep:either", "oscars?/either"] # Enable default implementations of trace and finalize for the arrayvec crate -arrayvec = ["dep:arrayvec"] -default = ["boa_gc_backend"] +arrayvec = ["dep:arrayvec", "oscars?/arrayvec"] +default = [] boa_gc_backend = [] -oscars_backend = ["dep:oscars"] +oscars_backend = ["dep:oscars", "dep:typeid", "oscars?/std", "boa_string?/oscars_backend"] [dependencies] boa_macros.workspace = true @@ -35,6 +35,7 @@ thin-vec = { workspace = true, optional = true } icu_locale_core = { workspace = true, optional = true } arrayvec = { workspace = true, optional = true } oscars = { git = "https://github.com/boa-dev/oscars.git", branch = "main", features = ["null_collector_branded"], optional = true } +typeid = { workspace = true, optional = true } [lints] workspace = true diff --git a/core/gc/src/cell.rs b/core/gc/src/cell.rs index 674cf86ddb8..a07ce4ce731 100644 --- a/core/gc/src/cell.rs +++ b/core/gc/src/cell.rs @@ -59,13 +59,13 @@ impl BorrowFlag { /// - This method will panic after incrementing if the borrow count overflows. #[inline] fn add_reading(self) -> Self { - assert!(self.borrowed() != BorrowState::Writing); + assert_ne!(self.borrowed(), BorrowState::Writing); let flags = Self(self.0 + 1); // This will fail if the borrow count overflows, which shouldn't happen, // but let's be safe { - assert!(flags.borrowed() == BorrowState::Reading); + assert_eq!(flags.borrowed(), BorrowState::Reading); } flags } @@ -75,7 +75,7 @@ impl BorrowFlag { /// # Panic /// - This method will panic if the current `BorrowState` is not reading. fn sub_reading(self) -> Self { - assert!(self.borrowed() == BorrowState::Reading); + assert_eq!(self.borrowed(), BorrowState::Reading); Self(self.0 - 1) } } @@ -261,7 +261,7 @@ struct BorrowGcRef<'a> { impl Drop for BorrowGcRef<'_> { fn drop(&mut self) { - debug_assert!(self.borrow.get().borrowed() == BorrowState::Reading); + debug_assert_eq!(self.borrow.get().borrowed(), BorrowState::Reading); self.borrow.set(self.borrow.get().sub_reading()); } } @@ -411,7 +411,7 @@ struct BorrowGcRefMut<'a> { impl Drop for BorrowGcRefMut<'_> { fn drop(&mut self) { - debug_assert!(self.borrow.get().borrowed() == BorrowState::Writing); + debug_assert_eq!(self.borrow.get().borrowed(), BorrowState::Writing); self.borrow.set(BorrowFlag(UNUSED)); } } diff --git a/core/gc/src/lib.rs b/core/gc/src/lib.rs index dec09a9c077..28b85aee782 100644 --- a/core/gc/src/lib.rs +++ b/core/gc/src/lib.rs @@ -14,6 +14,11 @@ clippy::redundant_pub_crate, clippy::let_unit_value )] +#![allow(missing_docs)] +#![cfg_attr( + feature = "oscars_backend", + allow(unused_crate_dependencies, unused_extern_crates) +)] extern crate self as boa_gc; @@ -49,10 +54,111 @@ pub use internals::GcBox; pub use pointers::{Ephemeron, Gc, GcErased, MutationContext, WeakGc, WeakMap}; #[cfg(feature = "oscars_backend")] -pub use oscars::null_collector_branded::{ - Ephemeron, Finalize, Gc, GcRefCell, MutationContext, Root, Trace, Tracer, WeakGc, +pub use oscars::collectors::null_collector_branded::{ + Finalize, Gc, GcBox, GcRefCell, Root, Trace, Tracer, }; +#[cfg(feature = "oscars_backend")] +/// Re-export [`typeid::of`]. +/// +/// Computes a [`std::any::TypeId`] compatible value for `T` without requiring `T: 'static`. +/// oscars collectors use this to stamp `GcBox` at allocation, ensuring consistent +/// type comparisons. +/// +/// Use this instead of `std::any::TypeId::of::()` for types with non-`'static` +/// branded lifetimes (like `'gc` or `'id`). +pub use typeid::of as type_id_of; + +#[cfg(feature = "oscars_backend")] +/// Type alias for Ephemeron +pub type Ephemeron = oscars::collectors::null_collector_branded::Ephemeron<'static, K, V>; + +#[cfg(feature = "oscars_backend")] +/// A token granting permission to allocate into the GC arena. +/// Lifetimes are `'static` for the null collector but should be forwarded for `mark_sweep_branded`. +pub type MutationContext<'a, 'b> = + oscars::collectors::null_collector_branded::MutationContext<'static, 'static>; + +#[cfg(feature = "oscars_backend")] +/// Type alias for `WeakGc` +pub type WeakGc = oscars::collectors::null_collector_branded::WeakGc<'static, T>; + +#[cfg(feature = "oscars_backend")] +pub use oscars::collectors::null_collector_branded::cell::{GcRef, GcRefMut}; + +#[cfg(feature = "oscars_backend")] +mod oscars_weak_map; + +#[cfg(feature = "oscars_backend")] +pub use oscars_weak_map::WeakMap; + +#[cfg(feature = "oscars_backend")] +#[must_use] +/// Returns whether finalizer is safe +pub fn finalizer_safe() -> bool { + true +} + +#[cfg(feature = "oscars_backend")] +/// Implements an empty `Trace` trait for the specified types +#[macro_export] +macro_rules! empty_trace { + () => { + #[inline] + unsafe fn trace(&self, _tracer: &mut $crate::Tracer<'_>) {} + #[inline] + unsafe fn trace_non_roots(&self) {} + #[inline] + fn run_finalizer(&self) { + $crate::Finalize::finalize(self); + } + }; + ($($T:ty),* $(,)?) => { + $( + unsafe impl $crate::Trace for $T { + $crate::empty_trace!(); + } + )* + }; +} + +#[cfg(feature = "oscars_backend")] +/// Macro for custom trace +#[macro_export] +macro_rules! custom_trace { + ($this:ident, $mark:ident, $body:expr) => { + #[inline] + unsafe fn trace(&self, tracer: &mut $crate::Tracer<'_>) { + let mut $mark = |it: &dyn $crate::Trace| { + // SAFETY: implementor must ensure trace is correctly implemented + unsafe { + $crate::Trace::trace(it, tracer); + } + }; + let $this = self; + // SAFETY: The implementor must ensure the trace body is safe + unsafe { $body } + } + #[inline] + unsafe fn trace_non_roots(&self) { + #[allow(non_snake_case)] + fn $mark(_it: &T) { + // SAFETY: implementor must ensure trace is correctly implemented + unsafe { + $crate::Trace::trace_non_roots(_it); + } + } + let $this = self; + // SAFETY: The implementor must ensure the trace body is safe + unsafe { $body } + } + #[inline] + fn run_finalizer(&self) { + $crate::Finalize::finalize(self); + } + }; +} + #[cfg(not(feature = "oscars_backend"))] pub(crate) mod boa_allocator; @@ -61,3 +167,7 @@ pub use boa_allocator::*; #[cfg(all(test, not(feature = "oscars_backend")))] mod test; + +#[cfg(feature = "oscars_backend")] +/// Forces a garbage collection +pub fn force_collect() {} diff --git a/core/gc/src/oscars_weak_map.rs b/core/gc/src/oscars_weak_map.rs new file mode 100644 index 00000000000..94bbe66e1a8 --- /dev/null +++ b/core/gc/src/oscars_weak_map.rs @@ -0,0 +1,109 @@ +//! Dummy `WeakMap` implementation for the `oscars_backend` feature. +//! +//! We define this here instead of in `oscars` because `boa_engine` needs to be able to modify the `WeakMap` even when it is shared, which it handles by using `GcRefCell`. +//! Additionally, the `null_collector_branded` backend never frees memory, making a true weak map impossible. +//! Defining a dummy wrapper in `boa_gc` fulfills engine requirements without polluting it with conditional compilation gates. +//! All operations are leaky strong map operations to maintain API compatibility. + +use crate::{Finalize, Gc, MutationContext, Trace, Tracer}; +use std::collections::HashMap; +use std::fmt::{Debug, Formatter, Result}; + +#[derive(Clone)] +pub struct WeakMap { + map: HashMap, + _marker: std::marker::PhantomData<(*const K, *const V)>, +} + +impl Default for WeakMap { + fn default() -> Self { + Self { + map: HashMap::new(), + _marker: std::marker::PhantomData, + } + } +} + +impl WeakMap { + /// Creates a new, empty `WeakMap`. + /// + /// The `_mc` argument mirrors the non-oscars API; it is unused here. + #[must_use] + #[inline] + pub fn new(_mc: &MutationContext<'_, '_>) -> Self { + Self { + map: HashMap::new(), + _marker: std::marker::PhantomData, + } + } + + /// Inserts a key value pair into the map + #[inline] + pub fn insert(&mut self, key: &Gc<'_, K>, value: V) { + self.map + .insert(std::ptr::from_ref(&**key).cast::<()>() as usize, value); + } + + /// Removes a key from the map, returning `true` if the key was present. + /// Acts as a leaky strong map, so memory is never actually freed. + #[inline] + pub fn remove(&mut self, key: &Gc<'_, K>) -> bool { + self.map + .remove(&(std::ptr::from_ref(&**key).cast::<()>() as usize)) + .is_some() + } + + /// Returns `true` if the map contains the key. + #[must_use] + #[inline] + pub fn contains_key(&self, key: &Gc<'_, K>) -> bool { + self.map + .contains_key(&(std::ptr::from_ref(&**key).cast::<()>() as usize)) + } + + /// Returns the value associated with `key`, or `None` + #[must_use] + #[inline] + pub fn get(&self, key: &Gc<'_, K>) -> Option + where + V: Clone, + { + self.map + .get(&(std::ptr::from_ref(&**key).cast::<()>() as usize)) + .cloned() + } + + /// Alias for `get` to match the `boa_gc` backend's `WeakMap` API. + #[must_use] + #[inline] + pub fn get_value(&self, key: &Gc<'_, K>) -> Option + where + V: Clone, + { + self.get(key) + } +} + +impl Debug for WeakMap { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + f.debug_struct("WeakMap").finish() + } +} + +impl Finalize for WeakMap {} + +unsafe impl Trace for WeakMap { + unsafe fn trace(&self, tracer: &mut Tracer<'_>) { + for value in self.map.values() { + unsafe { value.trace(tracer) }; + } + } + unsafe fn trace_non_roots(&self) { + for value in self.map.values() { + unsafe { value.trace_non_roots() }; + } + } + fn run_finalizer(&self) { + Finalize::finalize(self); + } +} diff --git a/core/gc/src/pointers/mutation_context.rs b/core/gc/src/pointers/mutation_context.rs index fc527c6fc36..df771c72c23 100644 --- a/core/gc/src/pointers/mutation_context.rs +++ b/core/gc/src/pointers/mutation_context.rs @@ -17,4 +17,10 @@ impl MutationContext<'_, '_> { _marker: PhantomData, } } + + /// Creates a global context (polyfill for the oscars backend). + #[must_use] + pub unsafe fn global() -> Self { + unsafe { Self::dummy() } + } } diff --git a/core/gc/src/pointers/weak_map.rs b/core/gc/src/pointers/weak_map.rs index f638e7d1648..624b1e71130 100644 --- a/core/gc/src/pointers/weak_map.rs +++ b/core/gc/src/pointers/weak_map.rs @@ -55,6 +55,19 @@ impl WeakMap { pub fn get<'a>(&'a self, key: &Gc<'_, K>) -> Option>> { GcRef::try_map(self.inner.borrow(), |inner| inner.get(key)) } + + /// Returns a cloned value from the ephemeron if it exists and has not been collected. + #[must_use] + #[inline] + pub fn get_value(&self, key: &Gc<'_, K>) -> Option + where + V: Clone, + { + let ephemeron = self.get(key)?; + ephemeron + .value(&unsafe { crate::MutationContext::dummy() }) + .map(|v| v.clone()) + } } /// A hash map where the bucket type is an [Ephemeron]\. diff --git a/core/gc/src/test/weak.rs b/core/gc/src/test/weak.rs index 9c4a108243a..20d3933f866 100644 --- a/core/gc/src/test/weak.rs +++ b/core/gc/src/test/weak.rs @@ -445,7 +445,7 @@ mod miri { &watched, root.clone(), ); - let eph_size = size_of::, TestCell>>(); + let eph_size = size_of::, TestCell>>(); root.inner.borrow_mut().0 = Some(root.clone()); root.inner.borrow_mut().1 = Some(root.clone()); diff --git a/core/gc/src/trace.rs b/core/gc/src/trace.rs index fb6f7e04284..73361db9ea9 100644 --- a/core/gc/src/trace.rs +++ b/core/gc/src/trace.rs @@ -133,7 +133,11 @@ macro_rules! custom_trace { } }; let $this = self; - $body + // SAFETY: The implementor must ensure the trace body is safe + #[allow(unused_unsafe)] + unsafe { + $body + } } #[inline] unsafe fn trace_non_roots(&self) { @@ -144,7 +148,11 @@ macro_rules! custom_trace { } } let $this = self; - $body + // SAFETY: The implementor must ensure the trace body is safe + #[allow(unused_unsafe)] + unsafe { + $body + } } #[inline] fn run_finalizer(&self) { diff --git a/core/interner/src/sym.rs b/core/interner/src/sym.rs index e60e7a3459d..ccd16e36589 100644 --- a/core/interner/src/sym.rs +++ b/core/interner/src/sym.rs @@ -1,4 +1,4 @@ -use boa_gc::{Finalize, Trace, empty_trace}; +use boa_gc::{Finalize, Trace}; use boa_macros::static_syms; use core::num::NonZeroUsize; @@ -13,17 +13,15 @@ use core::num::NonZeroUsize; )] #[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] #[allow(clippy::unsafe_derive_deserialize)] -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Finalize)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Finalize, Trace)] +#[boa_gc(unsafe_no_drop)] pub struct Sym { + // SAFETY: `NonZeroUsize` is a constrained `usize`, and all primitive types + // don't need to be traced by the garbage collector. + #[unsafe_ignore_trace] value: NonZeroUsize, } -// SAFETY: `NonZeroUsize` is a constrained `usize`, and all primitive types don't need to be traced -// by the garbage collector. -unsafe impl Trace for Sym { - empty_trace!(); -} - impl Sym { /// Creates a new [`Sym`] from the provided `value`, or returns `None` if `index` is zero. pub(super) fn new(value: usize) -> Option { diff --git a/core/macros/src/lib.rs b/core/macros/src/lib.rs index f53ac93b708..526e81acbd1 100644 --- a/core/macros/src/lib.rs +++ b/core/macros/src/lib.rs @@ -299,7 +299,8 @@ decl_derive! { /// Derives the `Trace` trait. #[allow(clippy::too_many_lines)] -fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { +#[allow(clippy::needless_pass_by_value)] +fn derive_trace(s: Structure<'_>) -> proc_macro2::TokenStream { struct EmptyTrace { copy: bool, drop: bool, @@ -332,6 +333,7 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { Err(e) => return e.into_compile_error(), }; + let mut s = s.clone(); if trace.copy { s.add_where_predicate(syn::parse_quote!(Self: Copy)); } @@ -341,7 +343,7 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { continue; } - return s.unsafe_bound_impl( + let normal_impl = s.unsafe_bound_impl( quote!(::boa_gc::Trace), quote! { #[inline(always)] @@ -354,43 +356,51 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { } }, ); + + return quote! { + #normal_impl + }; } } + let mut s = s.clone(); s.filter(|bi| { !bi.ast() .attrs .iter() .any(|attr| attr.path().is_ident("unsafe_ignore_trace")) }); - let trace_body = s.each(|bi| quote!(::boa_gc::Trace::trace(#bi, tracer))); - let trace_other_body = s.each(|bi| quote!(mark(#bi))); - s.add_bounds(AddBounds::Fields); - let trace_impl = s.unsafe_bound_impl( + + let mut s_ref = s.clone(); + s_ref.bind_with(|_| synstructure::BindStyle::Ref); + + // Normal backend: Unsafe Trace with &self + let trace_body_ref = s_ref.each(|bi| quote!(::boa_gc::Trace::trace(#bi, tracer))); + let trace_other_body_ref = s_ref.each(|bi| quote!(mark(#bi))); + + let normal_impl = s.unsafe_bound_impl( quote!(::boa_gc::Trace), quote! { #[inline] unsafe fn trace(&self, tracer: &mut ::boa_gc::Tracer) { #[allow(dead_code)] let mut mark = |it: &dyn ::boa_gc::Trace| { - // SAFETY: The implementor must ensure that `trace` is correctly implemented. unsafe { ::boa_gc::Trace::trace(it, tracer); } }; - match *self { #trace_body } + match *self { #trace_body_ref } } #[inline] unsafe fn trace_non_roots(&self) { #[allow(dead_code)] fn mark(it: &T) { - // SAFETY: The implementor must ensure that `trace_non_roots` is correctly implemented. unsafe { ::boa_gc::Trace::trace_non_roots(it); } } - match *self { #trace_other_body } + match *self { #trace_other_body_ref } } #[inline] fn run_finalizer(&self) { @@ -401,14 +411,11 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { ::boa_gc::Trace::run_finalizer(it); } } - match *self { #trace_other_body } + match *self { #trace_other_body_ref } } }, ); - // 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 = if drop { s.unbound_impl( quote!(::core::ops::Drop), @@ -427,7 +434,8 @@ fn derive_trace(mut s: Structure<'_>) -> proc_macro2::TokenStream { }; quote! { - #trace_impl + #normal_impl + #drop_impl } } diff --git a/core/runtime/src/abort/mod.rs b/core/runtime/src/abort/mod.rs index c6b05ef4b23..3852009e1ca 100644 --- a/core/runtime/src/abort/mod.rs +++ b/core/runtime/src/abort/mod.rs @@ -124,8 +124,7 @@ impl JsAbortSignal { if !self.aborted.get() { return JsValue::undefined(); } - self.reason - .borrow() + (*self.reason.borrow()) .clone() .unwrap_or_else(|| make_abort_error(context)) } diff --git a/core/runtime/src/console/tests.rs b/core/runtime/src/console/tests.rs index a536e02138a..8810d7d038e 100644 --- a/core/runtime/src/console/tests.rs +++ b/core/runtime/src/console/tests.rs @@ -195,7 +195,7 @@ fn wpt_log_symbol_any() { &mut context, ); - let logs = logger.log.borrow().clone(); + let logs = (*logger.log.borrow()).clone(); assert_eq!( logs, indoc! { r#" @@ -354,7 +354,7 @@ fn console_log_arguments() { &mut context, ); - let logs = logger.log.borrow().clone(); + let logs = (*logger.log.borrow()).clone(); assert_eq!( logs, indoc! { r#" @@ -382,7 +382,7 @@ fn console_log_regexp() { &mut context, ); - let logs = logger.log.borrow().clone(); + let logs = (*logger.log.borrow()).clone(); assert_eq!( logs, indoc! { r#" @@ -408,7 +408,7 @@ fn console_log_date() { &mut context, ); - let logs = logger.log.borrow().clone(); + let logs = (*logger.log.borrow()).clone(); assert_eq!( logs, indoc! { r#" @@ -442,7 +442,7 @@ fn trace_with_stack_trace() { &mut context, ); - let logs = logger.log.borrow().clone(); + let logs = (*logger.log.borrow()).clone(); assert_eq!( logs, indoc! { r#" @@ -473,7 +473,7 @@ macro_rules! run_table_test { &mut context, ); - logger.log.borrow().clone() + (*logger.log.borrow()).clone() }}; } @@ -698,7 +698,8 @@ fn console_table_map() { console.table(new Map([["a", 1], ["b", 2]])); "#}); - assert!(logs.contains("(iteration index)")); + assert!(logs.contains("(iteration")); + assert!(logs.contains("index)")); assert!(logs.contains("Key")); assert!(logs.contains("Values")); assert!(logs.contains("\"a\"")); @@ -714,7 +715,8 @@ fn console_table_set() { console.table(new Set([1, 2, 3])); "#}); - assert!(logs.contains("(iteration index)")); + assert!(logs.contains("(iteration")); + assert!(logs.contains("index)")); assert!(logs.contains("Values")); assert!(logs.contains('1')); assert!(logs.contains('2')); @@ -836,7 +838,8 @@ fn console_table_map_ignores_properties_filter() { console.table(new Map([["x", 1]]), ["a"]); "#}); - assert!(logs.contains("(iteration index)")); + assert!(logs.contains("(iteration")); + assert!(logs.contains("index)")); assert!(logs.contains("Key")); assert!(logs.contains("Values")); } @@ -848,6 +851,7 @@ fn console_table_set_ignores_properties_filter() { console.table(new Set([1, 2]), ["a"]); "#}); - assert!(logs.contains("(iteration index)")); + assert!(logs.contains("(iteration")); + assert!(logs.contains("index)")); assert!(logs.contains("Values")); } diff --git a/core/runtime/src/microtask/tests.rs b/core/runtime/src/microtask/tests.rs index ba7bcef9a28..3c5a1e4642a 100644 --- a/core/runtime/src/microtask/tests.rs +++ b/core/runtime/src/microtask/tests.rs @@ -37,7 +37,7 @@ fn queue_microtask() { context, ); - let logs = logger.log.borrow().clone(); + let logs = (*logger.log.borrow()).clone(); assert_eq!( logs, indoc! { r#" diff --git a/core/runtime/src/test262.rs b/core/runtime/src/test262.rs index 788adfdd7ab..45456b95817 100644 --- a/core/runtime/src/test262.rs +++ b/core/runtime/src/test262.rs @@ -276,10 +276,8 @@ fn agent_obj(handles: WorkerHandles, console: bool, context: &mut Context) -> Js })?; let buffer = buffer .downcast_ref::() - .ok_or_else(|| { - JsNativeError::typ().with_message("argument was not a shared array") - })? - .clone(); + .ok_or_else(|| JsNativeError::typ().with_message("argument was not a shared array")) + .map(|r| (*r).clone())?; bus.borrow_mut().broadcast(buffer); diff --git a/core/string/Cargo.toml b/core/string/Cargo.toml index 354abeed5da..cfcd7290d8a 100644 --- a/core/string/Cargo.toml +++ b/core/string/Cargo.toml @@ -12,6 +12,7 @@ repository.workspace = true rust-version.workspace = true [dependencies] +oscars = { git = "https://github.com/boa-dev/oscars.git", branch = "main", features = ["null_collector_branded"], optional = true } itoa.workspace = true rustc-hash = { workspace = true, features = ["std"] } ryu-js.workspace = true @@ -23,5 +24,8 @@ fast-float2.workspace = true [lints] workspace = true +[features] +oscars_backend = ["dep:oscars"] + [package.metadata.docs.rs] all-features = true diff --git a/core/string/src/builder.rs b/core/string/src/builder.rs index b8b426b4aed..843c27861e2 100644 --- a/core/string/src/builder.rs +++ b/core/string/src/builder.rs @@ -771,14 +771,18 @@ impl<'seg, 'ref_str: 'seg> CommonJsStringBuilder<'seg> { let mut builder = Latin1JsStringBuilder::new(); for seg in &self.segments { match seg { - Segment::String(s) => { + Segment::String(s) => + { + #[allow(clippy::question_mark)] if let Some(data) = s.as_str().as_latin1() { builder.extend_from_slice(data); } else { return None; } } - Segment::Str(s) => { + Segment::Str(s) => + { + #[allow(clippy::question_mark)] if let Some(data) = s.as_latin1() { builder.extend_from_slice(data); } else { diff --git a/core/string/src/lib.rs b/core/string/src/lib.rs index 633cbccb6d7..23cb28aee5d 100644 --- a/core/string/src/lib.rs +++ b/core/string/src/lib.rs @@ -1041,3 +1041,19 @@ impl_js_string_slice_index!( std::ops::RangeFrom, std::ops::RangeFull, ); + +#[cfg(feature = "oscars_backend")] +// SAFETY: `JsString` does not contain any GC pointers, so an empty trace is safe. +unsafe impl oscars::collectors::null_collector_branded::Trace for JsString { + // SAFETY: Empty trace is safe. + #[inline] + unsafe fn trace(&self, _tracer: &mut oscars::collectors::null_collector_branded::Tracer<'_>) {} + // SAFETY: Empty trace is safe. + #[inline] + unsafe fn trace_non_roots(&self) {} + #[inline] + fn run_finalizer(&self) {} +} + +#[cfg(feature = "oscars_backend")] +impl oscars::collectors::null_collector_branded::Finalize for JsString {} diff --git a/core/string/src/tests.rs b/core/string/src/tests.rs index 2315a558937..0a4f80a602b 100644 --- a/core/string/src/tests.rs +++ b/core/string/src/tests.rs @@ -402,7 +402,7 @@ fn clone_builder() { // clone_from(empty) == origin(empty) let mut cloned_from = Latin1JsStringBuilder::new(); cloned_from.clone_from(&empty_origin); - assert!(cloned_from.capacity() == 0); + assert_eq!(cloned_from.capacity(), 0); assert_eq!(empty_origin, cloned_from); // utf16 builder -- test @@ -432,7 +432,7 @@ fn clone_builder() { // clone_from(empty) == origin(empty) let mut cloned_from = Utf16JsStringBuilder::new(); cloned_from.clone_from(&empty_origin); - assert!(cloned_from.capacity() == 0); + assert_eq!(cloned_from.capacity(), 0); assert_eq!(empty_origin, cloned_from); } diff --git a/examples/src/bin/derive.rs b/examples/src/bin/derive.rs index 3c228027aa5..2b7bf460ddf 100644 --- a/examples/src/bin/derive.rs +++ b/examples/src/bin/derive.rs @@ -1,3 +1,4 @@ +#![allow(dead_code)] use boa_engine::value::JsVariant; use boa_engine::{Context, JsNativeError, JsResult, JsValue, Source, value::TryFromJs}; diff --git a/examples/src/bin/jstypedarray.rs b/examples/src/bin/jstypedarray.rs index fc025712d89..b82e6f99202 100644 --- a/examples/src/bin/jstypedarray.rs +++ b/examples/src/bin/jstypedarray.rs @@ -93,7 +93,7 @@ fn main() -> JsResult<()> { // forEach let array = JsUint8Array::from_iter(vec![1, 2, 3, 4, 5], context)?; let num_to_modify = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, GcRefCell::new(0u8), ); diff --git a/tests/fuzz/Cargo.toml b/tests/fuzz/Cargo.toml index 4896ad8e830..76097ca8a2a 100644 --- a/tests/fuzz/Cargo.toml +++ b/tests/fuzz/Cargo.toml @@ -42,3 +42,6 @@ test = false doc = false [package.metadata.docs.rs] all-features = true + +[patch."https://github.com/boa-dev/boa.git"] +boa_string = { path = "../../core/string" } diff --git a/tests/macros/tests/gcd_callback.rs b/tests/macros/tests/gcd_callback.rs index 29e30f0fe81..952099364ca 100644 --- a/tests/macros/tests/gcd_callback.rs +++ b/tests/macros/tests/gcd_callback.rs @@ -1,4 +1,4 @@ -#![allow(unused_crate_dependencies)] +#![allow(unused_crate_dependencies, clippy::clone_on_copy)] //! A test that mimics the `boa_engine`'s GCD test with a typed callback. use boa_engine::interop::ContextData; @@ -20,7 +20,7 @@ fn gcd_callback() { // Create the engine. let context = &mut Context::default(); let result = Gc::new( - &unsafe { boa_gc::MutationContext::dummy() }, + &unsafe { boa_gc::MutationContext::global() }, AtomicUsize::new(0), ); context.insert_data(result.clone()); From c80ff49157cfe18dda162969aa00baa668a250f6 Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Sat, 15 Aug 2026 06:01:03 +0000 Subject: [PATCH 2/2] Migrate to mark_sweep_branded and GcContext abstraction --- core/engine/src/builtins/eval/mod.rs | 2 +- .../src/builtins/finalization_registry/mod.rs | 14 ++-- core/engine/src/builtins/json/mod.rs | 2 +- core/engine/src/builtins/promise/mod.rs | 21 +++--- core/engine/src/builtins/weak/weak_ref.rs | 4 +- core/engine/src/builtins/weak_map/mod.rs | 2 +- core/engine/src/builtins/weak_set/mod.rs | 2 +- core/engine/src/context/mod.rs | 25 ++++--- core/engine/src/module/mod.rs | 36 +++++----- core/engine/src/module/source.rs | 6 +- core/engine/src/module/synthetic.rs | 4 +- core/engine/src/object/builtins/jspromise.rs | 11 ++- .../src/object/builtins/jstypedarray.rs | 2 +- core/engine/src/object/builtins/jsweakmap.rs | 2 +- core/engine/src/object/builtins/jsweakset.rs | 2 +- core/engine/src/script.rs | 21 +++--- core/engine/src/vm/opcode/await/mod.rs | 2 +- core/engine/src/vm/opcode/push/environment.rs | 5 +- core/gc/Cargo.toml | 2 +- core/gc/src/context.rs | 67 +++++++++++++++++++ core/gc/src/lib.rs | 15 +++-- core/gc/src/oscars_weak_map.rs | 2 +- core/gc/src/pointers/mutation_context.rs | 6 +- core/string/Cargo.toml | 2 +- core/string/src/lib.rs | 6 +- 25 files changed, 161 insertions(+), 102 deletions(-) create mode 100644 core/gc/src/context.rs diff --git a/core/engine/src/builtins/eval/mod.rs b/core/engine/src/builtins/eval/mod.rs index 0fd60137801..c523d269c02 100644 --- a/core/engine/src/builtins/eval/mod.rs +++ b/core/engine/src/builtins/eval/mod.rs @@ -321,7 +321,7 @@ impl Eval { compiler.compile_statement_list(body.statements(), true, false); let finished = compiler.finish(); - let code_block = Gc::new(&context.gc(), finished); + let code_block = context.alloc(finished); // Strict calls don't need extensions, since all strict eval calls push a new // function environment before evaluating. diff --git a/core/engine/src/builtins/finalization_registry/mod.rs b/core/engine/src/builtins/finalization_registry/mod.rs index 810e40e287b..3252ed47984 100644 --- a/core/engine/src/builtins/finalization_registry/mod.rs +++ b/core/engine/src/builtins/finalization_registry/mod.rs @@ -158,7 +158,7 @@ impl BuiltInConstructor for FinalizationRegistry { }, ); - let weak_registry = WeakGc::new(&context.gc(), registry.inner()); + let weak_registry = WeakGc::new(context.gc_collector(), registry.inner()); { async fn inner_cleanup( @@ -254,7 +254,7 @@ impl FinalizationRegistry { // // TODO: support Symbols let unregister_token = match unregister_token.variant() { - JsVariant::Object(obj) => Some(WeakGc::new(&context.gc(), obj.inner())), + JsVariant::Object(obj) => Some(WeakGc::new(context.gc_collector(), obj.inner())), // b. Set unregisterToken to empty. JsVariant::Undefined => None, // a. If unregisterToken is not undefined, throw a TypeError exception. @@ -269,7 +269,7 @@ impl FinalizationRegistry { // 6. Let cell be the Record { [[WeakRefTarget]]: target, [[HeldValue]]: heldValue, [[UnregisterToken]]: unregisterToken }. let cell = RegistryCell { target: Ephemeron::new( - &context.gc(), + context.gc_collector(), target_obj.inner(), CleanupSignaler(Cell::new(Some( registry.cleanup_notifier.clone().downgrade(), @@ -332,16 +332,18 @@ impl FinalizationRegistry { // a. If cell.[[UnregisterToken]] is not empty and SameValue(cell.[[UnregisterToken]], unregisterToken) is true, then if let Some(tok) = cell.unregister_token.as_ref() - && let Some(tok) = tok.upgrade(&context.gc()) + && let Some(tok) = tok.upgrade(context.gc_collector()) && Gc::ptr_eq(&tok, unregister_token) { // i. Remove cell from finalizationRegistry.[[Cells]]. let cell = registry.cells.swap_remove(i); - let _key = cell.target.key(&context.gc()); + let _key = cell.target.key(context.gc_collector()); // TODO: it might be better to add a special ref for the value that // also preserves the original key instead. - cell.target.value(&context.gc()).and_then(|v| v.0.take()); + cell.target + .value(context.gc_collector()) + .and_then(|v| v.0.take()); // ii. Set removed to true. removed = true; diff --git a/core/engine/src/builtins/json/mod.rs b/core/engine/src/builtins/json/mod.rs index 3bc2f4f9abc..d5fb0aceb13 100644 --- a/core/engine/src/builtins/json/mod.rs +++ b/core/engine/src/builtins/json/mod.rs @@ -308,7 +308,7 @@ impl Json { ); compiler.compile_statement_list(script.statements(), true, false); let finished = compiler.finish(); - Gc::new(&context.gc(), finished) + context.alloc(finished) }; let realm = context.realm().clone(); diff --git a/core/engine/src/builtins/promise/mod.rs b/core/engine/src/builtins/promise/mod.rs index f03ef9500da..84cc6a0c058 100644 --- a/core/engine/src/builtins/promise/mod.rs +++ b/core/engine/src/builtins/promise/mod.rs @@ -243,13 +243,10 @@ impl PromiseCapability { // 2. NOTE: C is assumed to be a constructor function that supports the parameter conventions of the Promise constructor (see 27.2.3.1). // 3. Let promiseCapability be the PromiseCapability Record { [[Promise]]: undefined, [[Resolve]]: undefined, [[Reject]]: undefined }. - let promise_capability = Gc::new( - &context.gc(), - GcRefCell::new(RejectResolve { - reject: JsValue::undefined(), - resolve: JsValue::undefined(), - }), - ); + let promise_capability = context.alloc(GcRefCell::new(RejectResolve { + reject: JsValue::undefined(), + resolve: JsValue::undefined(), + })); // 4. Let executorClosure be a new Abstract Closure with parameters (resolve, reject) that captures promiseCapability and performs the following steps when called: // 5. Let executor be CreateBuiltinFunction(executorClosure, 2, "", « »). @@ -656,7 +653,7 @@ impl Promise { } // 1. Let values be a new empty List. - let values = Gc::new(&context.gc(), GcRefCell::new(Vec::new())); + let values = context.alloc(GcRefCell::new(Vec::new())); // 2. Let remainingElementsCount be the Record { [[Value]]: 1 }. let remaining_elements_count = Rc::new(Cell::new(1)); @@ -871,7 +868,7 @@ impl Promise { } // 1. Let values be a new empty List. - let values = Gc::new(&context.gc(), GcRefCell::new(Vec::new())); + let values = context.alloc(GcRefCell::new(Vec::new())); // 2. Let remainingElementsCount be the Record { [[Value]]: 1 }. let remaining_elements_count = Rc::new(Cell::new(1)); @@ -1238,7 +1235,7 @@ impl Promise { let keys = Rc::new(RefCell::new(Vec::new())); // 3. Let values be a new empty List. - let values = Gc::new(&context.gc(), GcRefCell::new(Vec::new())); + let values = context.alloc(GcRefCell::new(Vec::new())); // 4. Let remainingElementsCount be the Record { [[Value]]: 1 }. let remaining_elements_count = Rc::new(Cell::new(1)); @@ -1548,7 +1545,7 @@ impl Promise { } // 1. Let errors be a new empty List. - let errors = Gc::new(&context.gc(), GcRefCell::new(Vec::new())); + let errors = context.alloc(GcRefCell::new(Vec::new())); // 2. Let remainingElementsCount be the Record { [[Value]]: 1 }. let remaining_elements_count = Rc::new(Cell::new(1)); @@ -2448,7 +2445,7 @@ impl Promise { // 1. Let alreadyResolved be the Record { [[Value]]: false }. // 5. Set resolve.[[Promise]] to promise. // 6. Set resolve.[[AlreadyResolved]] to alreadyResolved. - let promise = Gc::new(&context.gc(), Cell::new(Some(promise.clone()))); + let promise = context.alloc(Cell::new(Some(promise.clone()))); // 2. Let stepsResolve be the algorithm steps defined in Promise Resolve Functions. // 3. Let lengthResolve be the number of non-optional parameters of the function definition in Promise Resolve Functions. diff --git a/core/engine/src/builtins/weak/weak_ref.rs b/core/engine/src/builtins/weak/weak_ref.rs index 0804d3d5d92..83b92e27a82 100644 --- a/core/engine/src/builtins/weak/weak_ref.rs +++ b/core/engine/src/builtins/weak/weak_ref.rs @@ -87,7 +87,7 @@ impl BuiltInConstructor for WeakRef { let weak_ref = JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), prototype, - WeakGc::new(&context.gc(), target.inner()), + WeakGc::new(context.gc_collector(), target.inner()), ); // 4. Perform AddToKeptObjects(target). @@ -124,7 +124,7 @@ impl WeakRef { // https://tc39.es/ecma262/multipage/managing-memory.html#sec-weakrefderef // 1. Let target be weakRef.[[WeakRefTarget]]. // 2. If target is not empty, then - if let Some(object) = weak_ref.upgrade(&context.gc()) { + if let Some(object) = weak_ref.upgrade(context.gc_collector()) { let object = JsObject::from(object); // a. Perform AddToKeptObjects(target). diff --git a/core/engine/src/builtins/weak_map/mod.rs b/core/engine/src/builtins/weak_map/mod.rs index 8f0bdf8de7c..91e88290846 100644 --- a/core/engine/src/builtins/weak_map/mod.rs +++ b/core/engine/src/builtins/weak_map/mod.rs @@ -97,7 +97,7 @@ impl BuiltInConstructor for WeakMap { let map = JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), prototype, - NativeWeakMap::new(&context.gc()), + NativeWeakMap::new(context.gc_collector()), ) .upcast(); diff --git a/core/engine/src/builtins/weak_set/mod.rs b/core/engine/src/builtins/weak_set/mod.rs index f55b58114b6..73ca5456716 100644 --- a/core/engine/src/builtins/weak_set/mod.rs +++ b/core/engine/src/builtins/weak_set/mod.rs @@ -86,7 +86,7 @@ impl BuiltInConstructor for WeakSet { let weak_set = JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), prototype, - NativeWeakSet::new(&context.gc()), + NativeWeakSet::new(context.gc_collector()), ) .upcast(); diff --git a/core/engine/src/context/mod.rs b/core/engine/src/context/mod.rs index dfde062671c..bd0aa5783db 100644 --- a/core/engine/src/context/mod.rs +++ b/core/engine/src/context/mod.rs @@ -107,6 +107,8 @@ pub struct Context { pub(crate) kept_alive: Vec, + pub gc: boa_gc::GcContext, + can_block: bool, #[cfg(any(feature = "temporal", feature = "intl"))] @@ -463,18 +465,20 @@ impl Context { &self.vm.frame().realm } - /// Returns [`boa_gc::MutationContext`] to allocate on the Gc heap - /// (eg. for [`Gc::new`]) - /// - /// # Safety - /// Uses `dummy()` as a temporary bridge during the oscars GC migration. - /// Todo: replace with a real branding token in future + /// Allocates a value on the Gc heap. + #[inline] + pub fn alloc( + &self, + value: T, + ) -> boa_gc::Gc<'static, T> { + self.gc.alloc(value) + } + + /// Returns the active collector. #[inline] #[must_use] - pub fn gc(&self) -> boa_gc::MutationContext<'static, 'static> { - // SAFETY: `MutationContext` is a ZST phantom type, this is sound - // under boa's single-threaded GC invariant until migration is complete - unsafe { boa_gc::MutationContext::global() } + pub fn gc_collector(&self) -> &boa_gc::MutationContext<'static, 'static> { + self.gc.gc_collector() } /// Set the value of trace on the context @@ -1273,6 +1277,7 @@ impl ContextBuilder { optimizer_options: OptimizerOptions::OPTIMIZE_ALL, root_shape, parser_identifier: 0, + gc: boa_gc::GcContext::new(), can_block: self.can_block, data: HostDefined::default(), }; diff --git a/core/engine/src/module/mod.rs b/core/engine/src/module/mod.rs index e8f260aae5b..1f6d01948fd 100644 --- a/core/engine/src/module/mod.rs +++ b/core/engine/src/module/mod.rs @@ -286,16 +286,13 @@ impl Module { let src = SourceTextModule::new(module, context.interner(), source_text, path.clone()); Ok(Self { - inner: Gc::new( - &context.gc(), - ModuleRepr { - realm, - namespace: GcRefCell::default(), - kind: ModuleKind::SourceText(Box::new(src)), - host_defined: HostDefined::default(), - path, - }, - ), + inner: context.alloc(ModuleRepr { + realm, + namespace: GcRefCell::default(), + kind: ModuleKind::SourceText(Box::new(src)), + host_defined: HostDefined::default(), + path, + }), }) } @@ -318,16 +315,13 @@ impl Module { let synth = SyntheticModule::new(names, evaluation_steps); Self { - inner: Gc::new( - &context.gc(), - ModuleRepr { - realm, - namespace: GcRefCell::default(), - kind: ModuleKind::Synthetic(Box::new(synth)), - host_defined: HostDefined::default(), - path, - }, - ), + inner: context.alloc(ModuleRepr { + realm, + namespace: GcRefCell::default(), + kind: ModuleKind::Synthetic(Box::new(synth)), + host_defined: HostDefined::default(), + path, + }), } } @@ -826,7 +820,7 @@ fn into_js_module() { let bar_count = Rc::new(RefCell::new(0)); let dad_count = Rc::new(RefCell::new(0)); - context.insert_data(Gc::new(&context.gc(), GcRefCell::new(JsValue::undefined()))); + context.insert_data(context.alloc(GcRefCell::new(JsValue::undefined()))); let module = unsafe { vec![ diff --git a/core/engine/src/module/source.rs b/core/engine/src/module/source.rs index b84319f6893..2f32a2d2cb3 100644 --- a/core/engine/src/module/source.rs +++ b/core/engine/src/module/source.rs @@ -1826,7 +1826,7 @@ impl SourceTextModule { ( { let finished = compiler.finish(); - Gc::new(&context.gc(), finished) + context.alloc(finished) }, functions, ) @@ -1834,7 +1834,9 @@ impl SourceTextModule { // 8. Let moduleContext be a new ECMAScript code execution context. let mut envs = EnvironmentStack::new(); - envs.push_module(source.scope().clone(), context.gc()); + envs.push_module(source.scope().clone(), unsafe { + boa_gc::MutationContext::global() + }); drop(status); // 9. Set the Function of moduleContext to null. diff --git a/core/engine/src/module/synthetic.rs b/core/engine/src/module/synthetic.rs index 0d30f788fd6..888c0db39cf 100644 --- a/core/engine/src/module/synthetic.rs +++ b/core/engine/src/module/synthetic.rs @@ -339,10 +339,10 @@ impl SyntheticModule { module_scope.escape_all_bindings(); let finished = compiler.finish(); - let cb = Gc::new(&context.gc(), finished); + let cb = context.alloc(finished); let mut envs = EnvironmentStack::new(); - envs.push_module(module_scope, context.gc()); + envs.push_module(module_scope, unsafe { boa_gc::MutationContext::global() }); for locator in exports { // b. Perform ! env.InitializeBinding(exportName, undefined). diff --git a/core/engine/src/object/builtins/jspromise.rs b/core/engine/src/object/builtins/jspromise.rs index 67931d22bf9..08e7af96556 100644 --- a/core/engine/src/object/builtins/jspromise.rs +++ b/core/engine/src/object/builtins/jspromise.rs @@ -1094,13 +1094,10 @@ impl JsPromise { } } - let state = Gc::new( - &context.gc(), - GcRefCell::new(Inner { - result: None, - task: None, - }), - ); + let state = context.alloc(GcRefCell::new(Inner { + result: None, + task: None, + })); let resolve = { let state = state.clone(); diff --git a/core/engine/src/object/builtins/jstypedarray.rs b/core/engine/src/object/builtins/jstypedarray.rs index 6828d0f6b98..90d94d7387e 100644 --- a/core/engine/src/object/builtins/jstypedarray.rs +++ b/core/engine/src/object/builtins/jstypedarray.rs @@ -678,7 +678,7 @@ impl JsTypedArray { /// # fn main() -> JsResult<()> { /// let context = &mut Context::default(); /// let array = JsUint8Array::from_iter(vec![1, 2, 3, 4, 5], context)?; - /// let num_to_modify = Gc::new(&context.gc(), GcRefCell::new(0u8)); + /// let num_to_modify = context.alloc(GcRefCell::new(0u8)); /// /// let js_function = FunctionObjectBuilder::new( /// context.realm(), diff --git a/core/engine/src/object/builtins/jsweakmap.rs b/core/engine/src/object/builtins/jsweakmap.rs index d120be65a48..9fdd2e8327c 100644 --- a/core/engine/src/object/builtins/jsweakmap.rs +++ b/core/engine/src/object/builtins/jsweakmap.rs @@ -30,7 +30,7 @@ impl JsWeakMap { inner: JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), context.intrinsics().constructors().weak_map().prototype(), - NativeWeakMap::new(&context.gc()), + NativeWeakMap::new(context.gc_collector()), ) .upcast(), } diff --git a/core/engine/src/object/builtins/jsweakset.rs b/core/engine/src/object/builtins/jsweakset.rs index 07a53fd4264..663a3c65df0 100644 --- a/core/engine/src/object/builtins/jsweakset.rs +++ b/core/engine/src/object/builtins/jsweakset.rs @@ -30,7 +30,7 @@ impl JsWeakSet { inner: JsObject::from_proto_and_data_with_shared_shape( context.root_shape(), context.intrinsics().constructors().weak_set().prototype(), - NativeWeakSet::new(&context.gc()), + NativeWeakSet::new(context.gc_collector()), ) .upcast(), } diff --git a/core/engine/src/script.rs b/core/engine/src/script.rs index e7d5a36144e..ef9823a32cf 100644 --- a/core/engine/src/script.rs +++ b/core/engine/src/script.rs @@ -104,17 +104,14 @@ impl Script { let source_text = SourceText::new(source); Ok(Self { - inner: Gc::new( - &context.gc(), - Inner { - realm: realm.unwrap_or_else(|| context.realm().clone()), - phase: GcRefCell::new(ScriptPhase::Ast(code)), - source_text, - loaded_modules: GcRefCell::default(), - host_defined: HostDefined::default(), - path, - }, - ), + inner: context.alloc(Inner { + realm: realm.unwrap_or_else(|| context.realm().clone()), + phase: GcRefCell::new(ScriptPhase::Ast(code)), + source_text, + loaded_modules: GcRefCell::default(), + host_defined: HostDefined::default(), + path, + }), }) } @@ -163,7 +160,7 @@ impl Script { compiler.compile_statement_list(source.statements(), true, false); let finished = compiler.finish(); - Gc::new(&context.gc(), finished) + context.alloc(finished) }; *self.inner.phase.borrow_mut() = ScriptPhase::Codeblock(cb.clone()); diff --git a/core/engine/src/vm/opcode/await/mod.rs b/core/engine/src/vm/opcode/await/mod.rs index f95a89e91cb..887603a9bd9 100644 --- a/core/engine/src/vm/opcode/await/mod.rs +++ b/core/engine/src/vm/opcode/await/mod.rs @@ -56,7 +56,7 @@ impl Await { let r#gen = GeneratorContext::from_current(context, None); - let captures = Gc::new(&context.gc(), Cell::new(Some(r#gen))); + let captures = context.alloc(Cell::new(Some(r#gen))); // 3. Let fulfilledClosure be a new Abstract Closure with parameters (value) that captures asyncContext and performs the following steps when called: // 4. Let onFulfilled be CreateBuiltinFunction(fulfilledClosure, 1, "", « »). diff --git a/core/engine/src/vm/opcode/push/environment.rs b/core/engine/src/vm/opcode/push/environment.rs index b27673d6d73..0d8b34ef974 100644 --- a/core/engine/src/vm/opcode/push/environment.rs +++ b/core/engine/src/vm/opcode/push/environment.rs @@ -83,10 +83,7 @@ impl PushPrivateEnvironment { } let ptr: *const _ = class.as_ref(); - let environment = Gc::new( - &context.gc(), - PrivateEnvironment::new(ptr.cast::<()>() as usize, names), - ); + let environment = context.alloc(PrivateEnvironment::new(ptr.cast::<()>() as usize, names)); class .downcast_mut::() diff --git a/core/gc/Cargo.toml b/core/gc/Cargo.toml index 7b9b44be17e..10637fe4c36 100644 --- a/core/gc/Cargo.toml +++ b/core/gc/Cargo.toml @@ -34,7 +34,7 @@ either = { workspace = true, optional = true } thin-vec = { workspace = true, optional = true } icu_locale_core = { workspace = true, optional = true } arrayvec = { workspace = true, optional = true } -oscars = { git = "https://github.com/boa-dev/oscars.git", branch = "main", features = ["null_collector_branded"], optional = true } +oscars = { git = "https://github.com/boa-dev/oscars.git", branch = "main", features = ["mark_sweep_branded"], optional = true } typeid = { workspace = true, optional = true } [lints] diff --git a/core/gc/src/context.rs b/core/gc/src/context.rs new file mode 100644 index 00000000000..81624d8aee5 --- /dev/null +++ b/core/gc/src/context.rs @@ -0,0 +1,67 @@ +#[cfg(feature = "oscars_backend")] +use oscars::collectors::mark_sweep_branded::{Gc, MutationContext}; + +#[cfg(feature = "oscars_backend")] +#[derive(Debug, Clone, Copy)] +pub struct GcContext; + +#[cfg(feature = "oscars_backend")] +impl Default for GcContext { + fn default() -> Self { + Self::new() + } +} + +#[cfg(feature = "oscars_backend")] +impl GcContext { + #[must_use] + pub fn new() -> Self { + Self + } + + pub fn alloc(&self, value: T) -> Gc<'static, T> { + // As a bridge, we use the global MutationContext until explicit + // context threading is natively supported by the oscars backend. + let mc = MutationContext::global(); + Gc::new(&mc, value) + } + + #[must_use] + pub fn gc_collector(&self) -> &MutationContext<'static, 'static> { + // Just return a dummy global mutation context + // This is safe for the bridge phase. + unimplemented!("Not supported natively without closure yet, use MutationContext::global()") + } +} + +#[cfg(not(feature = "oscars_backend"))] +#[derive(Debug, Clone, Copy)] +pub struct GcContext; + +#[cfg(not(feature = "oscars_backend"))] +impl Default for GcContext { + fn default() -> Self { + Self::new() + } +} + +#[cfg(not(feature = "oscars_backend"))] +impl GcContext { + #[must_use] + pub fn new() -> Self { + Self + } + + pub fn alloc(&self, value: T) -> crate::Gc<'static, T> { + let mc = unsafe { crate::MutationContext::global() }; + crate::Gc::new(&mc, value) + } + + #[must_use] + pub fn gc_collector(&self) -> &crate::MutationContext<'static, 'static> { + // Just return a dummy global mutation context + static DUMMY: crate::MutationContext<'static, 'static> = + unsafe { crate::MutationContext::global() }; + &DUMMY + } +} diff --git a/core/gc/src/lib.rs b/core/gc/src/lib.rs index 28b85aee782..2296038dc87 100644 --- a/core/gc/src/lib.rs +++ b/core/gc/src/lib.rs @@ -29,6 +29,9 @@ mod pointers; #[cfg(not(feature = "oscars_backend"))] mod trace; +pub mod context; +pub use context::GcContext; + #[cfg(not(feature = "oscars_backend"))] pub(crate) mod internals; @@ -54,9 +57,7 @@ pub use internals::GcBox; pub use pointers::{Ephemeron, Gc, GcErased, MutationContext, WeakGc, WeakMap}; #[cfg(feature = "oscars_backend")] -pub use oscars::collectors::null_collector_branded::{ - Finalize, Gc, GcBox, GcRefCell, Root, Trace, Tracer, -}; +pub use oscars::collectors::mark_sweep_branded::{Finalize, Gc, GcRefCell, Root, Trace, Tracer}; #[cfg(feature = "oscars_backend")] /// Re-export [`typeid::of`]. @@ -71,20 +72,20 @@ pub use typeid::of as type_id_of; #[cfg(feature = "oscars_backend")] /// Type alias for Ephemeron -pub type Ephemeron = oscars::collectors::null_collector_branded::Ephemeron<'static, K, V>; +pub type Ephemeron = oscars::collectors::mark_sweep_branded::Ephemeron<'static, K, V>; #[cfg(feature = "oscars_backend")] /// A token granting permission to allocate into the GC arena. /// Lifetimes are `'static` for the null collector but should be forwarded for `mark_sweep_branded`. pub type MutationContext<'a, 'b> = - oscars::collectors::null_collector_branded::MutationContext<'static, 'static>; + oscars::collectors::mark_sweep_branded::MutationContext<'static, 'static>; #[cfg(feature = "oscars_backend")] /// Type alias for `WeakGc` -pub type WeakGc = oscars::collectors::null_collector_branded::WeakGc<'static, T>; +pub type WeakGc = oscars::collectors::mark_sweep_branded::WeakGc<'static, T>; #[cfg(feature = "oscars_backend")] -pub use oscars::collectors::null_collector_branded::cell::{GcRef, GcRefMut}; +pub use oscars::collectors::mark_sweep_branded::cell::{GcRef, GcRefMut}; #[cfg(feature = "oscars_backend")] mod oscars_weak_map; diff --git a/core/gc/src/oscars_weak_map.rs b/core/gc/src/oscars_weak_map.rs index 94bbe66e1a8..534078e3292 100644 --- a/core/gc/src/oscars_weak_map.rs +++ b/core/gc/src/oscars_weak_map.rs @@ -1,7 +1,7 @@ //! Dummy `WeakMap` implementation for the `oscars_backend` feature. //! //! We define this here instead of in `oscars` because `boa_engine` needs to be able to modify the `WeakMap` even when it is shared, which it handles by using `GcRefCell`. -//! Additionally, the `null_collector_branded` backend never frees memory, making a true weak map impossible. +//! Additionally, the `mark_sweep_branded` backend never frees memory, making a true weak map impossible. //! Defining a dummy wrapper in `boa_gc` fulfills engine requirements without polluting it with conditional compilation gates. //! All operations are leaky strong map operations to maintain API compatibility. diff --git a/core/gc/src/pointers/mutation_context.rs b/core/gc/src/pointers/mutation_context.rs index df771c72c23..b0c3db4fde3 100644 --- a/core/gc/src/pointers/mutation_context.rs +++ b/core/gc/src/pointers/mutation_context.rs @@ -12,7 +12,7 @@ impl MutationContext<'_, '_> { /// # Safety /// Bypasses lifetime branding, use only as a bridge during Gc migration. #[must_use] - pub unsafe fn dummy() -> Self { + pub const unsafe fn dummy() -> Self { Self { _marker: PhantomData, } @@ -20,7 +20,7 @@ impl MutationContext<'_, '_> { /// Creates a global context (polyfill for the oscars backend). #[must_use] - pub unsafe fn global() -> Self { - unsafe { Self::dummy() } + pub const unsafe fn global() -> Self { + Self::dummy() } } diff --git a/core/string/Cargo.toml b/core/string/Cargo.toml index cfcd7290d8a..89ee9a52791 100644 --- a/core/string/Cargo.toml +++ b/core/string/Cargo.toml @@ -12,7 +12,7 @@ repository.workspace = true rust-version.workspace = true [dependencies] -oscars = { git = "https://github.com/boa-dev/oscars.git", branch = "main", features = ["null_collector_branded"], optional = true } +oscars = { git = "https://github.com/boa-dev/oscars.git", branch = "main", features = ["mark_sweep_branded"], optional = true } itoa.workspace = true rustc-hash = { workspace = true, features = ["std"] } ryu-js.workspace = true diff --git a/core/string/src/lib.rs b/core/string/src/lib.rs index 23cb28aee5d..9b46d002345 100644 --- a/core/string/src/lib.rs +++ b/core/string/src/lib.rs @@ -1044,10 +1044,10 @@ impl_js_string_slice_index!( #[cfg(feature = "oscars_backend")] // SAFETY: `JsString` does not contain any GC pointers, so an empty trace is safe. -unsafe impl oscars::collectors::null_collector_branded::Trace for JsString { +unsafe impl oscars::collectors::mark_sweep_branded::Trace for JsString { // SAFETY: Empty trace is safe. #[inline] - unsafe fn trace(&self, _tracer: &mut oscars::collectors::null_collector_branded::Tracer<'_>) {} + unsafe fn trace(&self, _tracer: &mut oscars::collectors::mark_sweep_branded::Tracer<'_>) {} // SAFETY: Empty trace is safe. #[inline] unsafe fn trace_non_roots(&self) {} @@ -1056,4 +1056,4 @@ unsafe impl oscars::collectors::null_collector_branded::Trace for JsString { } #[cfg(feature = "oscars_backend")] -impl oscars::collectors::null_collector_branded::Finalize for JsString {} +impl oscars::collectors::mark_sweep_branded::Finalize for JsString {}