From 5d0fb4bac3443d552b20e3256623a1d9df5bc4a1 Mon Sep 17 00:00:00 2001 From: Shannon Booth Date: Wed, 27 Dec 2023 11:59:32 +1300 Subject: [PATCH] LibJS: Do not inherit TypedArray constructors from TypedArrayConstructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In: https://tc39.es/ecma262/#sec-%typedarray%-intrinsic-object The spec says: > is a constructor function object that all of the TypedArray > constructor objects inherit from. From what I understand from this, it effectively just means is that the prototype for the constructor should simply be set to TypedArrayConstructor. We _were_ doing that, but also inheriting from it in C++. This meant we were invoking TypedArrayConstructor::initialize for each of the typed arrays. This is not actually what we want, since it means that the 'of' and 'from' functions were being defined as native properties in both the concrete typed array (e.g Uint8Array), and the abstract TypedArray. Instead, the properties should only be defined and inherited from the abstract TypedArray class. Diff Tests: +4 ✅ -4 ❌ Co-Authored-By: Andreas Kling --- Userland/Libraries/LibJS/Runtime/TypedArray.cpp | 2 +- Userland/Libraries/LibJS/Runtime/TypedArray.h | 4 ++-- .../LibJS/Tests/builtins/TypedArray/TypedArray.from.js | 8 ++++++++ .../LibJS/Tests/builtins/TypedArray/TypedArray.js | 9 +++++++++ .../LibJS/Tests/builtins/TypedArray/TypedArray.of.js | 8 ++++++++ 5 files changed, 28 insertions(+), 3 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/TypedArray.cpp b/Userland/Libraries/LibJS/Runtime/TypedArray.cpp index 0275e347f3..c23996961c 100644 --- a/Userland/Libraries/LibJS/Runtime/TypedArray.cpp +++ b/Userland/Libraries/LibJS/Runtime/TypedArray.cpp @@ -503,7 +503,7 @@ void TypedArrayBase::visit_edges(Visitor& visitor) } \ \ ConstructorName::ConstructorName(Realm& realm, Object& prototype) \ - : TypedArrayConstructor(realm.vm().names.ClassName.as_string(), prototype) \ + : NativeFunction(realm.vm().names.ClassName.as_string(), prototype) \ { \ } \ \ diff --git a/Userland/Libraries/LibJS/Runtime/TypedArray.h b/Userland/Libraries/LibJS/Runtime/TypedArray.h index 72b0c562b7..d36053a9f7 100644 --- a/Userland/Libraries/LibJS/Runtime/TypedArray.h +++ b/Userland/Libraries/LibJS/Runtime/TypedArray.h @@ -509,8 +509,8 @@ ThrowCompletionOr compare_typed_array_elements(VM&, Value x, Value y, Fu private: \ PrototypeName(Object& prototype); \ }; \ - class ConstructorName final : public TypedArrayConstructor { \ - JS_OBJECT(ConstructorName, TypedArrayConstructor); \ + class ConstructorName final : public NativeFunction { \ + JS_OBJECT(ConstructorName, NativeFunction); \ JS_DECLARE_ALLOCATOR(ConstructorName); \ \ public: \ diff --git a/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.from.js b/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.from.js index cc11516827..20bf294212 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.from.js +++ b/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.from.js @@ -27,3 +27,11 @@ test("basic functionality", () => { expect(newTypedArray[2]).toBe(3n); }); }); + +test("is inherited from TypedArray base class", () => { + TYPED_ARRAYS.forEach(T1 => { + TYPED_ARRAYS.forEach(T2 => { + expect(T1.from).toBe(T2.from); + }); + }); +}); diff --git a/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.js b/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.js index 5de4ca8445..454a9adc31 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.js +++ b/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.js @@ -390,3 +390,12 @@ test("source is not the same value as the receiver, and the index is invalid", ( expect(receiver[2]).toBeUndefined(); }); }); + +test("constructor functions are defined in the TypedArray prototype, rather than the object itself", () => { + TYPED_ARRAYS.forEach(T => { + for (property of ["of", "from"]) { + expect(T.hasOwnProperty(property)).toBe(false); + expect(Object.getPrototypeOf(T).hasOwnProperty(property)).toBe(true); + } + }); +}); diff --git a/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.of.js b/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.of.js index 7dc07353f6..7246b1833d 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.of.js +++ b/Userland/Libraries/LibJS/Tests/builtins/TypedArray/TypedArray.of.js @@ -27,3 +27,11 @@ test("basic functionality", () => { expect(newTypedArray[2]).toBe(3n); }); }); + +test("is inherited from TypedArray base class", () => { + TYPED_ARRAYS.forEach(T1 => { + TYPED_ARRAYS.forEach(T2 => { + expect(T1.of).toBe(T2.of); + }); + }); +});