From 79f638d667d3ccef3600e6339eb401400565682b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Juli=C3=A1n=20Espina?= Date: Tue, 15 Nov 2022 21:25:10 +0000 Subject: [PATCH] Implement the `WeakRef` builtin (#2438) 52/60 tests passing. The remaining tests are either features not implemented ([FinalizationRegistry](https://tc39.es/ecma262/multipage/managing-memory.html#sec-finalization-registry-objects)) or features still in development ([symbols-as-weakmap-keys](https://github.com/tc39/proposal-symbols-as-weakmap-keys)). --- boa_engine/src/builtins/array/mod.rs | 5 +- boa_engine/src/builtins/mod.rs | 42 +++--- boa_engine/src/builtins/object/mod.rs | 4 +- boa_engine/src/builtins/weak/mod.rs | 3 + boa_engine/src/builtins/weak/weak_ref.rs | 166 +++++++++++++++++++++ boa_engine/src/context/intrinsics.rs | 7 + boa_engine/src/context/mod.rs | 17 +++ boa_engine/src/object/jsobject.rs | 11 ++ boa_engine/src/object/mod.rs | 25 +++- boa_engine/src/profiler.rs | 1 - boa_engine/src/tests.rs | 2 +- boa_engine/src/value/mod.rs | 3 +- boa_engine/src/value/operations.rs | 4 +- boa_engine/src/vm/code_block.rs | 2 +- boa_engine/src/vm/opcode/binary_ops/mod.rs | 4 +- test_ignore.txt | 6 + 16 files changed, 264 insertions(+), 38 deletions(-) create mode 100644 boa_engine/src/builtins/weak/mod.rs create mode 100644 boa_engine/src/builtins/weak/weak_ref.rs delete mode 100644 boa_engine/src/profiler.rs diff --git a/boa_engine/src/builtins/array/mod.rs b/boa_engine/src/builtins/array/mod.rs index 846cec01c95..2e4245a8eab 100644 --- a/boa_engine/src/builtins/array/mod.rs +++ b/boa_engine/src/builtins/array/mod.rs @@ -420,10 +420,7 @@ impl Array { JsValue::Object(o) if o.is_callable() => Some(o), _ => { return Err(JsNativeError::typ() - .with_message(format!( - "{} is not a function", - mapfn.type_of().to_std_string_escaped() - )) + .with_message(format!("`{}` is not callable", mapfn.type_of())) .into()) } }; diff --git a/boa_engine/src/builtins/mod.rs b/boa_engine/src/builtins/mod.rs index d6aeda78608..3e7aa658d8f 100644 --- a/boa_engine/src/builtins/mod.rs +++ b/boa_engine/src/builtins/mod.rs @@ -33,6 +33,7 @@ pub mod symbol; pub mod typed_array; pub mod undefined; pub mod uri; +pub mod weak; #[cfg(feature = "console")] pub mod console; @@ -82,44 +83,42 @@ use crate::{ builtins::{ array_buffer::ArrayBuffer, async_generator::AsyncGenerator, async_generator_function::AsyncGeneratorFunction, generator::Generator, - generator_function::GeneratorFunction, typed_array::TypedArray, uri::Uri, + generator_function::GeneratorFunction, typed_array::TypedArray, uri::Uri, weak::WeakRef, }, property::{Attribute, PropertyDescriptor}, Context, JsValue, }; -/// Trait representing a global built-in object such as `Math`, `Object` or -/// `String`. +/// Trait representing a global built-in object such as `Math`, `Object` or `String`. /// -/// This trait must be implemented for any global built-in accessible from -/// Javascript. +/// This trait must be implemented for any global built-in accessible from JavaScript. pub(crate) trait BuiltIn { /// Binding name of the built-in inside the global object. /// - /// E.g. If you want access the properties of a `Complex` built-in - /// with the name `Cplx` you must assign `"Cplx"` to this constant, - /// making any property inside it accessible from Javascript as `Cplx.prop` + /// E.g. If you want access the properties of a `Complex` built-in with the name `Cplx` you must + /// assign `"Cplx"` to this constant, making any property inside it accessible from Javascript + /// as `Cplx.prop` const NAME: &'static str; - /// Property attribute flags of the built-in. - /// Check [Attribute] for more information. + /// Property attribute flags of the built-in. Check [`Attribute`] for more information. const ATTRIBUTE: Attribute = Attribute::WRITABLE .union(Attribute::NON_ENUMERABLE) .union(Attribute::CONFIGURABLE); /// Initialization code for the built-in. - /// This is where the methods, properties, static methods and the constructor - /// of a built-in must be initialized to be accessible from Javascript. + /// + /// This is where the methods, properties, static methods and the constructor of a built-in must + /// be initialized to be accessible from Javascript. /// /// # Note /// - /// A return value of `None` indicates that the value must not be added as - /// a global attribute for the global object. + /// A return value of `None` indicates that the value must not be added as a global attribute + /// for the global object. fn init(context: &mut Context) -> Option; } -/// Utility function that checks if a type implements `BuiltIn` before -/// initializing it as a global built-in. +/// Utility function that checks if a type implements `BuiltIn` before initializing it as a global +/// built-in. #[inline] fn init_builtin(context: &mut Context) { if let Some(value) = B::init(context) { @@ -195,7 +194,8 @@ pub fn init(context: &mut Context) { AsyncFunction, AsyncGenerator, AsyncGeneratorFunction, - Uri + Uri, + WeakRef }; #[cfg(feature = "intl")] @@ -206,17 +206,15 @@ pub fn init(context: &mut Context) { } pub trait JsArgs { - /// Utility function to `get` a parameter from - /// a `[JsValue]` or default to `JsValue::Undefined` + /// Utility function to `get` a parameter from a `[JsValue]` or default to `JsValue::Undefined` /// if `get` returns `None`. /// /// Call this if you are thinking of calling something similar to /// `args.get(n).cloned().unwrap_or_default()` or /// `args.get(n).unwrap_or(&undefined)`. /// - /// This returns a reference for efficiency, in case - /// you only need to call methods of `JsValue`, so - /// try to minimize calling `clone`. + /// This returns a reference for efficiency, in case you only need to call methods of `JsValue`, + /// so try to minimize calling `clone`. fn get_or_undefined(&self, index: usize) -> &JsValue; } diff --git a/boa_engine/src/builtins/object/mod.rs b/boa_engine/src/builtins/object/mod.rs index a185a750d97..88859126f59 100644 --- a/boa_engine/src/builtins/object/mod.rs +++ b/boa_engine/src/builtins/object/mod.rs @@ -628,8 +628,8 @@ impl Object { val => { return Err(JsNativeError::typ() .with_message(format!( - "expected an object or null, got {}", - val.type_of().to_std_string_escaped() + "expected an object or null, got `{}`", + val.type_of() )) .into()) } diff --git a/boa_engine/src/builtins/weak/mod.rs b/boa_engine/src/builtins/weak/mod.rs new file mode 100644 index 00000000000..0beedf03846 --- /dev/null +++ b/boa_engine/src/builtins/weak/mod.rs @@ -0,0 +1,3 @@ +mod weak_ref; + +pub(crate) use weak_ref::WeakRef; diff --git a/boa_engine/src/builtins/weak/weak_ref.rs b/boa_engine/src/builtins/weak/weak_ref.rs new file mode 100644 index 00000000000..a111b5cd0fb --- /dev/null +++ b/boa_engine/src/builtins/weak/weak_ref.rs @@ -0,0 +1,166 @@ +use boa_gc::{Finalize, Trace, WeakGc}; +use boa_profiler::Profiler; +use tap::{Conv, Pipe}; + +use crate::{ + builtins::{BuiltIn, JsArgs}, + context::intrinsics::StandardConstructors, + object::{ + internal_methods::get_prototype_from_constructor, ConstructorBuilder, JsObject, ObjectData, + }, + property::Attribute, + symbol::WellKnownSymbols, + Context, JsNativeError, JsResult, JsValue, +}; + +/// The [`WeakRef`][wr] builtin object. +/// +/// [wr]: https://tc39.es/ecma262/#sec-weak-ref-objects +#[derive(Debug, Clone, Trace, Finalize)] +pub(crate) struct WeakRef; + +impl BuiltIn for WeakRef { + const NAME: &'static str = "WeakRef"; + + const ATTRIBUTE: Attribute = Attribute::WRITABLE.union(Attribute::CONFIGURABLE); + + fn init(context: &mut Context) -> Option { + let _timer = Profiler::global().start_event(Self::NAME, "init"); + ConstructorBuilder::with_standard_constructor( + context, + WeakRef::constructor, + context.intrinsics().constructors().weak_ref().clone(), + ) + .name(Self::NAME) + .length(Self::LENGTH) + .property( + WellKnownSymbols::to_string_tag(), + "WeakRef", + Attribute::CONFIGURABLE, + ) + .method(WeakRef::deref, "deref", 0) + .build() + .conv::() + .pipe(Some) + } +} + +impl WeakRef { + /// The amount of arguments the `WeakRef` constructor takes. + pub(crate) const LENGTH: usize = 1; + + /// Constructor [`WeakRef ( target )`][cons] + /// + /// [cons]: https://tc39.es/ecma262/#sec-weak-ref-target + pub(crate) fn constructor( + new_target: &JsValue, + args: &[JsValue], + context: &mut Context, + ) -> JsResult { + // If NewTarget is undefined, throw a TypeError exception. + if new_target.is_undefined() { + return Err(JsNativeError::typ() + .with_message("WeakRef: cannot call constructor without `new`") + .into()); + } + + // 2. If target is not an Object, throw a TypeError exception. + let target = args.get(0).and_then(JsValue::as_object).ok_or_else(|| { + JsNativeError::typ().with_message(format!( + "WeakRef: expected target argument of type `object`, got target of type `{}`", + args.get_or_undefined(0).type_of() + )) + })?; + + // 3. Let weakRef be ? OrdinaryCreateFromConstructor(NewTarget, "%WeakRef.prototype%", « [[WeakRefTarget]] »). + // 5. Set weakRef.[[WeakRefTarget]] to target. + let weak_ref = JsObject::from_proto_and_data( + get_prototype_from_constructor(new_target, StandardConstructors::weak_ref, context)?, + ObjectData::weak_ref(WeakGc::new(target.inner())), + ); + + // 4. Perform AddToKeptObjects(target). + context.kept_alive.push(target.clone()); + + // 6. Return weakRef. + Ok(weak_ref.into()) + } + + /// Method [`WeakRef.prototype.deref ( )`][spec]. + /// + /// If the referenced object hasn't been collected, this method promotes a `WeakRef` into a + /// proper [`JsObject`], or returns `undefined` otherwise. + /// + /// [spec]: https://tc39.es/ecma262/#sec-weak-ref.prototype.deref + pub(crate) fn deref(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { + // 1. Let weakRef be the this value. + // 2. Perform ? RequireInternalSlot(weakRef, [[WeakRefTarget]]). + let weak_ref = this.as_object().map(JsObject::borrow).ok_or_else(|| { + JsNativeError::typ().with_message(format!( + "WeakRef.prototype.deref: expected `this` to be an `object`, got value of type `{}`", + this.type_of() + )) + })?; + + let weak_ref = weak_ref.as_weak_ref().ok_or_else(|| { + JsNativeError::typ() + .with_message("WeakRef.prototype.deref: expected `this` to be a `WeakRef` object") + })?; + + // 3. Return WeakRefDeref(weakRef). + + // `WeakRefDeref` + // 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() { + let object = JsObject::from(object); + + // a. Perform AddToKeptObjects(target). + context.kept_alive.push(object.clone()); + + // b. Return target. + Ok(object.into()) + } else { + // 3. Return undefined. + Ok(JsValue::undefined()) + } + } +} + +#[cfg(test)] +mod tests { + use crate::{Context, JsValue}; + + #[test] + fn weak_ref_collected() { + let context = &mut Context::default(); + + assert!(context + .eval( + r#" + var ptr; + { + let obj = {a: 5, b: 6}; + ptr = new WeakRef(obj); + } + ptr.deref() + "# + ) + .unwrap() + .is_object()); + + boa_gc::force_collect(); + + assert_eq!( + context + .eval( + r#" + ptr.deref() + "# + ) + .unwrap(), + JsValue::undefined() + ) + } +} diff --git a/boa_engine/src/context/intrinsics.rs b/boa_engine/src/context/intrinsics.rs index e8d7dac720e..0da0e55581a 100644 --- a/boa_engine/src/context/intrinsics.rs +++ b/boa_engine/src/context/intrinsics.rs @@ -116,6 +116,7 @@ pub struct StandardConstructors { data_view: StandardConstructor, date_time_format: StandardConstructor, promise: StandardConstructor, + weak_ref: StandardConstructor, } impl Default for StandardConstructors { @@ -175,6 +176,7 @@ impl Default for StandardConstructors { data_view: StandardConstructor::default(), date_time_format: StandardConstructor::default(), promise: StandardConstructor::default(), + weak_ref: StandardConstructor::default(), }; // The value of `Array.prototype` is the Array prototype object. @@ -402,6 +404,11 @@ impl StandardConstructors { pub fn promise(&self) -> &StandardConstructor { &self.promise } + + #[inline] + pub fn weak_ref(&self) -> &StandardConstructor { + &self.weak_ref + } } /// Cached intrinsic objects diff --git a/boa_engine/src/context/mod.rs b/boa_engine/src/context/mod.rs index cc9691d4f58..0fc3ea736e4 100644 --- a/boa_engine/src/context/mod.rs +++ b/boa_engine/src/context/mod.rs @@ -104,6 +104,8 @@ pub struct Context { pub(crate) vm: Vm, pub(crate) promise_job_queue: VecDeque, + + pub(crate) kept_alive: Vec, } impl Default for Context { @@ -538,6 +540,7 @@ impl Context { self.realm.set_global_binding_number(); let result = self.run(); self.vm.pop_frame(); + self.clear_kept_objects(); self.run_queued_jobs()?; let (result, _) = result?; Ok(result) @@ -547,6 +550,7 @@ impl Context { fn run_queued_jobs(&mut self) -> JsResult<()> { while let Some(job) = self.promise_job_queue.pop_front() { job.call_job_callback(&JsValue::Undefined, &[], self)?; + self.clear_kept_objects(); } Ok(()) } @@ -580,6 +584,18 @@ impl Context { // TODO self.promise_job_queue.push_back(job); } + + /// Abstract operation [`ClearKeptObjects`][clear]. + /// + /// Clears all objects maintained alive by calls to the [`AddToKeptObjects`][add] abstract + /// operation, used within the [`WeakRef`][weak] constructor. + /// + /// [clear]: https://tc39.es/ecma262/multipage/executable-code-and-execution-contexts.html#sec-clear-kept-objects + /// [add]: https://tc39.es/ecma262/multipage/executable-code-and-execution-contexts.html#sec-addtokeptobjects + /// [weak]: https://tc39.es/ecma262/multipage/managing-memory.html#sec-weak-ref-objects + pub fn clear_kept_objects(&mut self) { + self.kept_alive.clear(); + } } /// Builder for the [`Context`] type. /// @@ -661,6 +677,7 @@ impl ContextBuilder { #[cfg(feature = "fuzz")] instructions_remaining: self.instructions_remaining, promise_job_queue: VecDeque::new(), + kept_alive: Vec::new(), }; // Add new builtIns to Context Realm diff --git a/boa_engine/src/object/jsobject.rs b/boa_engine/src/object/jsobject.rs index 6bcf077f9bd..a67a7490ee1 100644 --- a/boa_engine/src/object/jsobject.rs +++ b/boa_engine/src/object/jsobject.rs @@ -736,6 +736,10 @@ Cannot both specify accessors and a value or writable attribute", } ) } + + pub(crate) fn inner(&self) -> &Gc> { + &self.inner + } } impl AsRef> for JsObject { @@ -745,6 +749,13 @@ impl AsRef> for JsObject { } } +impl From>> for JsObject { + #[inline] + fn from(inner: Gc>) -> Self { + JsObject { inner } + } +} + impl PartialEq for JsObject { fn eq(&self, other: &Self) -> bool { Self::equals(self, other) diff --git a/boa_engine/src/object/mod.rs b/boa_engine/src/object/mod.rs index 8cbc7fe7a93..64a06e4d0a1 100644 --- a/boa_engine/src/object/mod.rs +++ b/boa_engine/src/object/mod.rs @@ -56,7 +56,7 @@ use crate::{ Context, JsBigInt, JsResult, JsString, JsSymbol, JsValue, }; -use boa_gc::{custom_trace, Finalize, Trace}; +use boa_gc::{custom_trace, Finalize, GcCell, Trace, WeakGc}; use boa_interner::Sym; use rustc_hash::FxHashMap; use std::{ @@ -193,6 +193,7 @@ pub enum ObjectKind { NativeObject(Box), IntegerIndexed(IntegerIndexed), Promise(Promise), + WeakRef(WeakGc>), #[cfg(feature = "intl")] DateTimeFormat(Box), } @@ -222,6 +223,7 @@ unsafe impl Trace for ObjectKind { Self::DateTimeFormat(f) => mark(f), Self::Promise(p) => mark(p), Self::AsyncGenerator(g) => mark(g), + Self::WeakRef(wr) => mark(wr), Self::RegExp(_) | Self::BigInt(_) | Self::Boolean(_) @@ -512,6 +514,14 @@ impl ObjectData { } } + /// Creates the `WeakRef` object data + pub fn weak_ref(weak_ref: WeakGc>) -> Self { + Self { + kind: ObjectKind::WeakRef(weak_ref), + internal_methods: &ORDINARY_INTERNAL_METHODS, + } + } + /// Create the `NativeObject` object data pub fn native_object(native_object: Box) -> Self { Self { @@ -576,6 +586,7 @@ impl Display for ObjectKind { #[cfg(feature = "intl")] Self::DateTimeFormat(_) => "DateTimeFormat", Self::Promise(_) => "Promise", + Self::WeakRef(_) => "WeakRef", }) } } @@ -1424,6 +1435,18 @@ impl Object { } } + /// Gets the `WeakRef`data if the object is a `WeakRef`. + #[inline] + pub fn as_weak_ref(&self) -> Option<&WeakGc>> { + match self.data { + ObjectData { + kind: ObjectKind::WeakRef(ref weak_ref), + .. + } => Some(weak_ref), + _ => None, + } + } + /// Return `true` if it is a native object and the native type is `T`. #[inline] pub fn is(&self) -> bool diff --git a/boa_engine/src/profiler.rs b/boa_engine/src/profiler.rs deleted file mode 100644 index 8b137891791..00000000000 --- a/boa_engine/src/profiler.rs +++ /dev/null @@ -1 +0,0 @@ - diff --git a/boa_engine/src/tests.rs b/boa_engine/src/tests.rs index e5febd0d3f6..7e97b8e52a1 100644 --- a/boa_engine/src/tests.rs +++ b/boa_engine/src/tests.rs @@ -2720,7 +2720,7 @@ fn instanceofoperator_rhs_not_object() { assert_eq!( &exec(scenario), - "\"TypeError: right-hand side of 'instanceof' should be an object, got number\"" + "\"TypeError: right-hand side of 'instanceof' should be an object, got `number`\"" ); } diff --git a/boa_engine/src/value/mod.rs b/boa_engine/src/value/mod.rs index 4b4b46713b7..473a1029d5f 100644 --- a/boa_engine/src/value/mod.rs +++ b/boa_engine/src/value/mod.rs @@ -949,7 +949,7 @@ impl JsValue { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-typeof-operator - pub fn type_of(&self) -> JsString { + pub fn type_of(&self) -> &'static str { match *self { Self::Rational(_) | Self::Integer(_) => "number", Self::String(_) => "string", @@ -966,7 +966,6 @@ impl JsValue { } } } - .into() } /// Abstract operation `IsArray ( argument )` diff --git a/boa_engine/src/value/operations.rs b/boa_engine/src/value/operations.rs index f22478f8627..45dcaaf22d9 100644 --- a/boa_engine/src/value/operations.rs +++ b/boa_engine/src/value/operations.rs @@ -421,8 +421,8 @@ impl JsValue { if !target.is_object() { return Err(JsNativeError::typ() .with_message(format!( - "right-hand side of 'instanceof' should be an object, got {}", - target.type_of().to_std_string_escaped() + "right-hand side of 'instanceof' should be an object, got `{}`", + target.type_of() )) .into()); } diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 02f52e097f2..b79ddf3b4bf 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -434,7 +434,7 @@ impl ToInternedString for CodeBlock { for (i, value) in self.literals.iter().enumerate() { f.push_str(&format!( " {i:04}: <{}> {}\n", - value.type_of().to_std_string_escaped(), + value.type_of(), value.display() )); } diff --git a/boa_engine/src/vm/opcode/binary_ops/mod.rs b/boa_engine/src/vm/opcode/binary_ops/mod.rs index ef2854ef65a..31d1a04b682 100644 --- a/boa_engine/src/vm/opcode/binary_ops/mod.rs +++ b/boa_engine/src/vm/opcode/binary_ops/mod.rs @@ -86,8 +86,8 @@ impl Operation for In { if !rhs.is_object() { return Err(JsNativeError::typ() .with_message(format!( - "right-hand side of 'in' should be an object, got {}", - rhs.type_of().to_std_string_escaped() + "right-hand side of 'in' should be an object, got `{}`", + rhs.type_of() )) .into()); } diff --git a/test_ignore.txt b/test_ignore.txt index 523fbd39d23..4c1e07f946e 100644 --- a/test_ignore.txt +++ b/test_ignore.txt @@ -9,6 +9,7 @@ feature:Temporal feature:tail-call-optimization feature:ShadowRealm feature:FinalizationRegistry +feature:FinalizationRegistry.prototype.cleanupSome feature:Atomics feature:dynamic_import feature:decorators @@ -29,6 +30,11 @@ feature:Intl.Locale // Non-standard feature:caller +// Stage 3 proposals + +// https://github.com/tc39/proposal-symbols-as-weakmap-keys +feature:symbols-as-weakmap-keys + // These generate a stack overflow tco-call tco-member