Improve typing of DataView and related objects (#3626)

core/engine/src/builtins/array_buffer/mod.rs

@@ -16,22 +16,24 @@ pub(crate) mod utils;
 #[cfg(test)]
 mod tests;
 
+use std::ops::{Deref, DerefMut};
+
 pub use shared::SharedArrayBuffer;
 
 use crate::{
     builtins::BuiltInObject,
     context::intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
     error::JsNativeError,
     js_string,
-    object::{internal_methods::get_prototype_from_constructor, JsObject},
+    object::{internal_methods::get_prototype_from_constructor, JsObject, Object},
     property::Attribute,
     realm::Realm,
     string::common::StaticJsStrings,
     symbol::JsSymbol,
     value::IntegerOrInfinity,
     Context, JsArgs, JsData, JsResult, JsString, JsValue,
 };
-use boa_gc::{Finalize, Trace};
+use boa_gc::{Finalize, GcRef, GcRefMut, Trace};
 use boa_profiler::Profiler;
 
 use self::utils::{SliceRef, SliceRefMut};
@@ -41,16 +43,20 @@ use super::{
 };
 
 #[derive(Debug, Clone, Copy)]
-pub(crate) enum BufferRef<'a> {
-    Buffer(&'a ArrayBuffer),
-    SharedBuffer(&'a SharedArrayBuffer),
+pub(crate) enum BufferRef<B, S> {
+    Buffer(B),
+    SharedBuffer(S),
 }
 
-impl BufferRef<'_> {
+impl<B, S> BufferRef<B, S>
+where
+    B: Deref<Target = ArrayBuffer>,
+    S: Deref<Target = SharedArrayBuffer>,
+{
     pub(crate) fn data(&self) -> Option<SliceRef<'_>> {
         match self {
-            Self::Buffer(buf) => buf.data().map(SliceRef::Slice),
-            Self::SharedBuffer(buf) => Some(SliceRef::AtomicSlice(buf.data())),
+            Self::Buffer(buf) => buf.deref().data().map(SliceRef::Slice),
+            Self::SharedBuffer(buf) => Some(SliceRef::AtomicSlice(buf.deref().data())),
         }
     }
 
@@ -60,16 +66,96 @@ impl BufferRef<'_> {
 }
 
 #[derive(Debug)]
-pub(crate) enum BufferRefMut<'a> {
-    Buffer(&'a mut ArrayBuffer),
-    SharedBuffer(&'a mut SharedArrayBuffer),
+pub(crate) enum BufferRefMut<B, S> {
+    Buffer(B),
+    SharedBuffer(S),
 }
 
-impl BufferRefMut<'_> {
+impl<B, S> BufferRefMut<B, S>
+where
+    B: DerefMut<Target = ArrayBuffer>,
+    S: DerefMut<Target = SharedArrayBuffer>,
+{
     pub(crate) fn data_mut(&mut self) -> Option<SliceRefMut<'_>> {
         match self {
-            Self::Buffer(buf) => buf.data_mut().map(SliceRefMut::Slice),
-            Self::SharedBuffer(buf) => Some(SliceRefMut::AtomicSlice(buf.data())),
+            Self::Buffer(buf) => buf.deref_mut().data_mut().map(SliceRefMut::Slice),
+            Self::SharedBuffer(buf) => Some(SliceRefMut::AtomicSlice(buf.deref_mut().data())),
+        }
+    }
+}
+
+/// A `JsObject` containing a bytes buffer as its inner data.
+#[derive(Debug, Clone, Trace, Finalize)]
+#[boa_gc(unsafe_no_drop)]
+pub(crate) enum BufferObject {
+    Buffer(JsObject<ArrayBuffer>),
+    SharedBuffer(JsObject<SharedArrayBuffer>),
+}
+
+impl From<BufferObject> for JsObject {
+    fn from(value: BufferObject) -> Self {
+        match value {
+            BufferObject::Buffer(buf) => buf.upcast(),
+            BufferObject::SharedBuffer(buf) => buf.upcast(),
+        }
+    }
+}
+
+impl From<BufferObject> for JsValue {
+    fn from(value: BufferObject) -> Self {
+        JsValue::from(JsObject::from(value))
+    }
+}
+
+impl BufferObject {
+    /// Gets the buffer data of the object.
+    #[inline]
+    #[must_use]
+    pub(crate) fn as_buffer(
+        &self,
+    ) -> BufferRef<GcRef<'_, ArrayBuffer>, GcRef<'_, SharedArrayBuffer>> {
+        match self {
+            Self::Buffer(buf) => BufferRef::Buffer(GcRef::map(buf.borrow(), |o| &o.data)),
+            Self::SharedBuffer(buf) => {
+                BufferRef::SharedBuffer(GcRef::map(buf.borrow(), |o| &o.data))
+            }
+        }
+    }
+
+    /// Gets the mutable buffer data of the object
+    #[inline]
+    pub(crate) fn as_buffer_mut(
+        &self,
+    ) -> BufferRefMut<
+        GcRefMut<'_, Object<ArrayBuffer>, ArrayBuffer>,
+        GcRefMut<'_, Object<SharedArrayBuffer>, SharedArrayBuffer>,
+    > {
+        match self {
+            Self::Buffer(buf) => {
+                BufferRefMut::Buffer(GcRefMut::map(buf.borrow_mut(), |o| &mut o.data))
+            }
+            Self::SharedBuffer(buf) => {
+                BufferRefMut::SharedBuffer(GcRefMut::map(buf.borrow_mut(), |o| &mut o.data))
+            }
+        }
+    }
+
+    /// Returns `true` if the buffer objects point to the same buffer.
+    #[inline]
+    pub(crate) fn equals(lhs: &Self, rhs: &Self) -> bool {
+        match (lhs, rhs) {
+            (BufferObject::Buffer(lhs), BufferObject::Buffer(rhs)) => JsObject::equals(lhs, rhs),
+            (BufferObject::SharedBuffer(lhs), BufferObject::SharedBuffer(rhs)) => {
+                if JsObject::equals(lhs, rhs) {
+                    return true;
+                }
+
+                let lhs = lhs.borrow();
+                let rhs = rhs.borrow();
+
+                std::ptr::eq(lhs.data.data().as_ptr(), rhs.data.data().as_ptr())
+            }
+            _ => false,
         }
     }
 }

core/engine/src/builtins/array_buffer/shared.rs

@@ -125,7 +125,9 @@ impl BuiltInConstructor for SharedArrayBuffer {
         let byte_length = args.get_or_undefined(0).to_index(context)?;
 
         // 3. Return ? AllocateSharedArrayBuffer(NewTarget, byteLength, requestedMaxByteLength).
-        Ok(Self::allocate(new_target, byte_length, context)?.into())
+        Ok(Self::allocate(new_target, byte_length, context)?
+            .upcast()
+            .into())
     }
 }
 
@@ -256,7 +258,7 @@ impl SharedArrayBuffer {
         constructor: &JsValue,
         byte_length: u64,
         context: &mut Context,
-    ) -> JsResult<JsObject> {
+    ) -> JsResult<JsObject<SharedArrayBuffer>> {
         // TODO:
         // 1. Let slots be « [[ArrayBufferData]] ».
         // 2. If maxByteLength is present and maxByteLength is not empty, let allocatingGrowableBuffer
@@ -291,11 +293,7 @@ impl SharedArrayBuffer {
 
         // 10. Else,
         //     a. Set obj.[[ArrayBufferByteLength]] to byteLength.
-        let obj = JsObject::from_proto_and_data_with_shared_shape(
-            context.root_shape(),
-            prototype,
-            Self { data },
-        );
+        let obj = JsObject::new(context.root_shape(), prototype, Self { data });
 
         // 11. Return obj.
         Ok(obj)

core/engine/src/builtins/array_buffer/utils.rs

@@ -3,7 +3,6 @@
 use std::{ptr, slice::SliceIndex, sync::atomic};
 
 use portable_atomic::AtomicU8;
-use sptr::Strict;
 
 use crate::{
     builtins::typed_array::{ClampedU8, Element, TypedArrayElement, TypedArrayKind},
@@ -41,7 +40,9 @@ impl SliceRef<'_> {
     }
 
     /// Gets the starting address of this `SliceRef`.
+    #[cfg(debug_assertions)]
     pub(crate) fn addr(&self) -> usize {
+        use sptr::Strict;
         match self {
             Self::Slice(buf) => buf.as_ptr().addr(),
             Self::AtomicSlice(buf) => buf.as_ptr().addr(),
@@ -68,7 +69,8 @@ impl SliceRef<'_> {
 
             // 1. Assert: IsDetachedBuffer(arrayBuffer) is false.
             // 2. Assert: There are sufficient bytes in arrayBuffer starting at byteIndex to represent a value of type.
-            if cfg!(debug_assertions) {
+            #[cfg(debug_assertions)]
+            {
                 assert!(buffer.len() >= std::mem::size_of::<T>());
                 assert_eq!(buffer.addr() % std::mem::align_of::<T>(), 0);
             }
@@ -178,6 +180,7 @@ pub(crate) enum SliceRefMut<'a> {
 
 impl SliceRefMut<'_> {
     /// Gets the byte length of this `SliceRefMut`.
+    #[cfg(debug_assertions)]
     pub(crate) fn len(&self) -> usize {
         match self {
             Self::Slice(buf) => buf.len(),
@@ -201,7 +204,9 @@ impl SliceRefMut<'_> {
     }
 
     /// Gets the starting address of this `SliceRefMut`.
+    #[cfg(debug_assertions)]
     pub(crate) fn addr(&self) -> usize {
+        use sptr::Strict;
         match self {
             Self::Slice(buf) => buf.as_ptr().addr(),
             Self::AtomicSlice(buf) => buf.as_ptr().addr(),
@@ -236,7 +241,8 @@ impl SliceRefMut<'_> {
             // 1. Assert: IsDetachedBuffer(arrayBuffer) is false.
             // 2. Assert: There are sufficient bytes in arrayBuffer starting at byteIndex to represent a value of type.
             // 3. Assert: value is a BigInt if IsBigIntElementType(type) is true; otherwise, value is a Number.
-            if cfg!(debug_assertions) {
+            #[cfg(debug_assertions)]
+            {
                 assert!(buffer.len() >= std::mem::size_of::<T>());
                 assert_eq!(buffer.addr() % std::mem::align_of::<T>(), 0);
             }
@@ -345,7 +351,8 @@ unsafe fn copy_shared_to_shared_backwards(src: &[AtomicU8], dest: &[AtomicU8], c
 ///   (you cannot borrow and mutably borrow a slice at the same time), but cannot be guaranteed
 ///   for atomic slices.
 pub(crate) unsafe fn memcpy(src: SliceRef<'_>, dest: SliceRefMut<'_>, count: usize) {
-    if cfg!(debug_assertions) {
+    #[cfg(debug_assertions)]
+    {
         assert!(src.len() >= count);
         assert!(dest.len() >= count);
         let src_range = src.addr()..src.addr() + src.len();
@@ -387,7 +394,8 @@ pub(crate) unsafe fn memcpy(src: SliceRef<'_>, dest: SliceRefMut<'_>, count: usi
 /// - `buffer` must contain at least `from + count` bytes to be read.
 /// - `buffer` must contain at least `to + count` bytes to be written.
 pub(crate) unsafe fn memmove(buffer: SliceRefMut<'_>, from: usize, to: usize, count: usize) {
-    if cfg!(debug_assertions) {
+    #[cfg(debug_assertions)]
+    {
         assert!(from + count <= buffer.len());
         assert!(to + count <= buffer.len());
     }

core/engine/src/builtins/atomics/mod.rs

@@ -24,7 +24,7 @@ use boa_gc::GcRef;
 use boa_profiler::Profiler;
 
 use super::{
-    array_buffer::{BufferRef, SharedArrayBuffer},
+    array_buffer::BufferRef,
     typed_array::{Atomic, ContentType, Element, TypedArray, TypedArrayElement, TypedArrayKind},
     BuiltInBuilder, IntrinsicObject,
 };
@@ -80,10 +80,7 @@ macro_rules! atomic_op {
             let value = ii.kind().get_element(value, context)?;
 
             // revalidate
-            let mut buffer = ii.viewed_array_buffer().borrow_mut();
-            let mut buffer = buffer
-                .as_buffer_mut()
-                .expect("integer indexed object must contain a valid buffer");
+            let mut buffer = ii.viewed_array_buffer().as_buffer_mut();
             let Some(mut data) = buffer.data_mut() else {
                 return Err(JsNativeError::typ()
                     .with_message("cannot execute atomic operation in detached buffer")
@@ -168,10 +165,7 @@ impl Atomics {
         let pos = validate_atomic_access(&ii, index, context)?;
 
         // 2. Perform ? RevalidateAtomicAccess(typedArray, indexedPosition).
-        let buffer = ii.viewed_array_buffer().borrow();
-        let buffer = buffer
-            .as_buffer()
-            .expect("integer indexed object must contain a valid buffer");
+        let buffer = ii.viewed_array_buffer().as_buffer();
         let Some(data) = buffer.data() else {
             return Err(JsNativeError::typ()
                 .with_message("cannot execute atomic operation in detached buffer")
@@ -217,10 +211,7 @@ impl Atomics {
         let value = ii.kind().get_element(&converted, context)?;
 
         // 4. Perform ? RevalidateAtomicAccess(typedArray, indexedPosition).
-        let mut buffer = ii.viewed_array_buffer().borrow_mut();
-        let mut buffer = buffer
-            .as_buffer_mut()
-            .expect("integer indexed object must contain a valid buffer");
+        let mut buffer = ii.viewed_array_buffer().as_buffer_mut();
         let Some(mut buffer) = buffer.data_mut() else {
             return Err(JsNativeError::typ()
                 .with_message("cannot execute atomic operation in detached buffer")
@@ -269,10 +260,7 @@ impl Atomics {
             .to_bytes();
 
         // 6. Perform ? RevalidateAtomicAccess(typedArray, indexedPosition).
-        let mut buffer = ii.viewed_array_buffer().borrow_mut();
-        let mut buffer = buffer
-            .as_buffer_mut()
-            .expect("integer indexed object must contain a valid buffer");
+        let mut buffer = ii.viewed_array_buffer().as_buffer_mut();
         let Some(mut data) = buffer.data_mut() else {
             return Err(JsNativeError::typ()
                 .with_message("cannot execute atomic operation in detached buffer")
@@ -403,10 +391,7 @@ impl Atomics {
 
         // 1. Let buffer be ? ValidateIntegerTypedArray(typedArray, true).
         let ii = validate_integer_typed_array(array, true)?;
-        let buffer = ii.viewed_array_buffer().borrow();
-        let buffer = buffer
-            .as_buffer()
-            .expect("integer indexed object must contain a valid buffer");
+        let buffer = ii.viewed_array_buffer().as_buffer();
 
         // 2. If IsSharedArrayBuffer(buffer) is false, throw a TypeError exception.
         let BufferRef::SharedBuffer(buffer) = buffer else {
@@ -451,10 +436,10 @@ impl Atomics {
         // SAFETY: the validity of `addr` is verified by our call to `validate_atomic_access`.
         let result = unsafe {
             if ii.kind() == TypedArrayKind::BigInt64 {
-                futex::wait(buffer, offset, value, timeout)?
+                futex::wait(&buffer, offset, value, timeout)?
             } else {
                 // value must fit into `i32` since it came from an `i32` above.
-                futex::wait(buffer, offset, value as i32, timeout)?
+                futex::wait(&buffer, offset, value as i32, timeout)?
             }
         };
 
@@ -496,13 +481,11 @@ impl Atomics {
         // 4. Let buffer be typedArray.[[ViewedArrayBuffer]].
         // 5. Let block be buffer.[[ArrayBufferData]].
         // 6. If IsSharedArrayBuffer(buffer) is false, return +0𝔽.
-        let buffer = ii.viewed_array_buffer();
-        let buffer = buffer.borrow();
-        let Some(shared) = buffer.downcast_ref::<SharedArrayBuffer>() else {
+        let BufferRef::SharedBuffer(shared) = ii.viewed_array_buffer().as_buffer() else {
             return Ok(0.into());
         };
 
-        let count = futex::notify(shared, offset, count)?;
+        let count = futex::notify(&shared, offset, count)?;
 
         // 12. Let n be the number of elements in S.
         // 13. Return 𝔽(n).