Skip to content

Commit

Permalink
[JSC] Rename Array#groupBy to Array#group and enable them
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=243510

Reviewed by Mark Lam, Justin Michaud, Devin Rousso and Ross Kirsling.

We agreed at TC39 to renaming Array#groupBy and Array#groupByToMap to Array#group and Array#groupToMap
to keep web-compatibility. This patch renames and enables them, stage-4 progress is tracked in [1].
We also upstreamed test262 fix for this[2].

[1]: tc39/proposal-array-grouping#32
[2]: tc39/test262#3632

* Source/JavaScriptCore/builtins/ArrayPrototype.js:
(group):
(groupToMap):
(groupBy): Deleted.
(groupByToMap): Deleted.
* Source/JavaScriptCore/runtime/ArrayPrototype.cpp:
(JSC::ArrayPrototype::finishCreation):
* Source/JavaScriptCore/runtime/OptionsList.h:

Canonical link: https://commits.webkit.org/253101@main
  • Loading branch information
@Constellation
Constellation committed Aug 4, 2022
1 parent b5ed1bd commit 317e700
Show file tree
Hide file tree
Showing 15 changed files with 151 additions and 133 deletions.
18 changes: 9 additions & 9 deletions JSTests/stress/array-group-by-null-or-undefined.js
View file
@@ -1,4 +1,4 @@
//@ requireOptions("--useArrayGroupByMethod=1")
//@ requireOptions("--useArrayGroupMethod=1")

function shouldThrow(func, errorMessage) {
var errorThrown = false;
Expand All @@ -16,15 +16,15 @@ function shouldThrow(func, errorMessage) {
}

shouldThrow(() => {
Array.prototype.groupBy.call(null, () => { /* empty */ })
}, `TypeError: Array.prototype.groupBy requires that |this| not be null or undefined`);
Array.prototype.group.call(null, () => { /* empty */ })
}, `TypeError: Array.prototype.group requires that |this| not be null or undefined`);
shouldThrow(() => {
Array.prototype.groupBy.call(undefined, () => { /* empty */ })
}, `TypeError: Array.prototype.groupBy requires that |this| not be null or undefined`);
Array.prototype.group.call(undefined, () => { /* empty */ })
}, `TypeError: Array.prototype.group requires that |this| not be null or undefined`);

shouldThrow(() => {
Array.prototype.groupByToMap.call(null, () => { /* empty */ })
}, `TypeError: Array.prototype.groupByToMap requires that |this| not be null or undefined`);
Array.prototype.groupToMap.call(null, () => { /* empty */ })
}, `TypeError: Array.prototype.groupToMap requires that |this| not be null or undefined`);
shouldThrow(() => {
Array.prototype.groupByToMap.call(undefined, () => { /* empty */ })
}, `TypeError: Array.prototype.groupByToMap requires that |this| not be null or undefined`);
Array.prototype.groupToMap.call(undefined, () => { /* empty */ })
}, `TypeError: Array.prototype.groupToMap requires that |this| not be null or undefined`);
96 changes: 48 additions & 48 deletions JSTests/stress/array-groupBy.js
View file
@@ -1,4 +1,4 @@
//@ requireOptions("--useArrayGroupByMethod=1")
//@ requireOptions("--useArrayGroupMethod=1")

function shouldBe(actual, expected) {
if (actual !== expected)
Expand Down Expand Up @@ -39,7 +39,7 @@ function toObject(array) {
result.length = array.length;
for (let i in array)
result[i] = array[i];
result.groupBy = Array.prototype.groupBy;
result.group = Array.prototype.group;
return result;
}

Expand All @@ -51,7 +51,7 @@ function reverseInsertionOrder(array) {
let result = {};
for (let i = props.length - 1; i >= 0; i--)
result[props[i]] = obj[props[i]];
result.groupBy = Array.prototype.groupBy;
result.group = Array.prototype.group;
return result;
}

Expand All @@ -66,97 +66,97 @@ let objectWithValueOfThatThrows = {

// Basic

shouldBe(Array.prototype.groupBy.length, 1);
shouldBe(Array.prototype.groupBy.name, "groupBy");
shouldBe(Array.prototype.group.length, 1);
shouldBe(Array.prototype.group.name, "group");

shouldBeObject([undefined].groupBy((x) => x === undefined ? "a" : "b"), {"a": [undefined]});
shouldBeObject([undefined].groupBy((x) => x === undefined), {"true": [undefined]});
shouldBeObject([undefined].group((x) => x === undefined ? "a" : "b"), {"a": [undefined]});
shouldBeObject([undefined].group((x) => x === undefined), {"true": [undefined]});

shouldBeObject((new Array(4)).groupBy((x) => x === undefined ? "a" : "b"), {"a": [undefined, undefined, undefined, undefined]});
shouldBeObject((new Array(4)).groupBy((x) => x === undefined), {"true": [undefined, undefined, undefined, undefined]});
shouldBeObject((new Array(4)).group((x) => x === undefined ? "a" : "b"), {"a": [undefined, undefined, undefined, undefined]});
shouldBeObject((new Array(4)).group((x) => x === undefined), {"true": [undefined, undefined, undefined, undefined]});

shouldBeObject([0, 1, 2, 3].groupBy((x) => !(x & 1) ? "a" : "b"), {"a": [0, 2], "b": [1, 3]});
shouldBeObject([0, 1, 2, 3].groupBy((x) => !(x & 1)), {"true": [0, 2], "false": [1, 3]});
shouldBeObject([0, 1, 2, 3].group((x) => !(x & 1) ? "a" : "b"), {"a": [0, 2], "b": [1, 3]});
shouldBeObject([0, 1, 2, 3].group((x) => !(x & 1)), {"true": [0, 2], "false": [1, 3]});

shouldBeObject([0, 1, 2, 3].groupBy((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, 3]});
shouldBeObject([0, 1, 2, 3].groupBy((x, i) => i >= 2), {"false": [0, 1], "true": [2, 3]});
shouldBeObject([0, 1, 2, 3].group((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, 3]});
shouldBeObject([0, 1, 2, 3].group((x, i) => i >= 2), {"false": [0, 1], "true": [2, 3]});

shouldBeObject(mixPartialAndFast.groupBy((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, undefined, undefined, sparseArrayLength - 1]});
shouldBeObject(mixPartialAndFast.groupBy((x, i) => i >= 2), {"false": [0, 1], "true": [2, undefined, undefined, sparseArrayLength - 1]});
shouldBeObject(mixPartialAndFast.group((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, undefined, undefined, sparseArrayLength - 1]});
shouldBeObject(mixPartialAndFast.group((x, i) => i >= 2), {"false": [0, 1], "true": [2, undefined, undefined, sparseArrayLength - 1]});


// Generic Object

shouldBeObject(toObject([undefined]).groupBy((x) => x === undefined ? "a" : "b"), {"a": [undefined]});
shouldBeObject(toObject([undefined]).groupBy((x) => x === undefined), {"true": [undefined]});
shouldBeObject(toObject([undefined]).group((x) => x === undefined ? "a" : "b"), {"a": [undefined]});
shouldBeObject(toObject([undefined]).group((x) => x === undefined), {"true": [undefined]});

shouldBeObject(toObject(new Array(4)).groupBy((x) => x === undefined ? "a" : "b"), {"a": [undefined, undefined, undefined, undefined]});
shouldBeObject(toObject(new Array(4)).groupBy((x) => x === undefined), {"true": [undefined, undefined, undefined, undefined]});
shouldBeObject(toObject(new Array(4)).group((x) => x === undefined ? "a" : "b"), {"a": [undefined, undefined, undefined, undefined]});
shouldBeObject(toObject(new Array(4)).group((x) => x === undefined), {"true": [undefined, undefined, undefined, undefined]});

shouldBeObject(toObject([0, 1, 2, 3]).groupBy((x) => !(x & 1) ? "a" : "b"), {"a": [0, 2], "b": [1, 3]});
shouldBeObject(toObject([0, 1, 2, 3]).groupBy((x) => !(x & 1)), {"true": [0, 2], "false": [1, 3]});
shouldBeObject(toObject([0, 1, 2, 3]).group((x) => !(x & 1) ? "a" : "b"), {"a": [0, 2], "b": [1, 3]});
shouldBeObject(toObject([0, 1, 2, 3]).group((x) => !(x & 1)), {"true": [0, 2], "false": [1, 3]});

shouldBeObject(toObject([0, 1, 2, 3]).groupBy((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, 3]});
shouldBeObject(toObject([0, 1, 2, 3]).groupBy((x, i) => i >= 2), {"false": [0, 1], "true": [2, 3]});
shouldBeObject(toObject([0, 1, 2, 3]).group((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, 3]});
shouldBeObject(toObject([0, 1, 2, 3]).group((x, i) => i >= 2), {"false": [0, 1], "true": [2, 3]});

shouldBeObject(toObject(mixPartialAndFast).groupBy((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, undefined, undefined, sparseArrayLength - 1]});
shouldBeObject(toObject(mixPartialAndFast).groupBy((x, i) => i >= 2), {"false": [0, 1], "true": [2, undefined, undefined, sparseArrayLength - 1]});
shouldBeObject(toObject(mixPartialAndFast).group((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, undefined, undefined, sparseArrayLength - 1]});
shouldBeObject(toObject(mixPartialAndFast).group((x, i) => i >= 2), {"false": [0, 1], "true": [2, undefined, undefined, sparseArrayLength - 1]});


// Array-like object with invalid lengths

shouldBeObject(Array.prototype.groupBy.call({ 0: 0, 1: 1, 2: 2, 3: 3, length: 0 }, notReached), {});
shouldBeObject(Array.prototype.groupBy.call({ 0: 0, 1: 1, 2: 2, 3: 3, length: -0 }, notReached), {});
shouldBeObject(Array.prototype.groupBy.call({ 0: 0, 1: 1, 2: 2, 3: 3, length: -4 }, notReached), {});
shouldBeObject(Array.prototype.group.call({ 0: 0, 1: 1, 2: 2, 3: 3, length: 0 }, notReached), {});
shouldBeObject(Array.prototype.group.call({ 0: 0, 1: 1, 2: 2, 3: 3, length: -0 }, notReached), {});
shouldBeObject(Array.prototype.group.call({ 0: 0, 1: 1, 2: 2, 3: 3, length: -4 }, notReached), {});


// Reversed generic Object

shouldBeObject(reverseInsertionOrder([undefined]).groupBy((x) => x === undefined ? "a" : "b"), {"a": [undefined]});
shouldBeObject(reverseInsertionOrder([undefined]).groupBy((x) => x === undefined), {"true": [undefined]});
shouldBeObject(reverseInsertionOrder([undefined]).group((x) => x === undefined ? "a" : "b"), {"a": [undefined]});
shouldBeObject(reverseInsertionOrder([undefined]).group((x) => x === undefined), {"true": [undefined]});

shouldBeObject(reverseInsertionOrder(new Array(4)).groupBy((x) => x === undefined ? "a" : "b"), {"a": [undefined, undefined, undefined, undefined]});
shouldBeObject(reverseInsertionOrder(new Array(4)).groupBy((x) => x === undefined), {"true": [undefined, undefined, undefined, undefined]});
shouldBeObject(reverseInsertionOrder(new Array(4)).group((x) => x === undefined ? "a" : "b"), {"a": [undefined, undefined, undefined, undefined]});
shouldBeObject(reverseInsertionOrder(new Array(4)).group((x) => x === undefined), {"true": [undefined, undefined, undefined, undefined]});

shouldBeObject(reverseInsertionOrder([0, 1, 2, 3]).groupBy((x) => !(x & 1) ? "a" : "b"), {"a": [0, 2], "b": [1, 3]});
shouldBeObject(reverseInsertionOrder([0, 1, 2, 3]).groupBy((x) => !(x & 1)), {"true": [0, 2], "false": [1, 3]});
shouldBeObject(reverseInsertionOrder([0, 1, 2, 3]).group((x) => !(x & 1) ? "a" : "b"), {"a": [0, 2], "b": [1, 3]});
shouldBeObject(reverseInsertionOrder([0, 1, 2, 3]).group((x) => !(x & 1)), {"true": [0, 2], "false": [1, 3]});

shouldBeObject(reverseInsertionOrder([0, 1, 2, 3]).groupBy((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, 3]});
shouldBeObject(reverseInsertionOrder([0, 1, 2, 3]).groupBy((x, i) => i >= 2), {"false": [0, 1], "true": [2, 3]});
shouldBeObject(reverseInsertionOrder([0, 1, 2, 3]).group((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, 3]});
shouldBeObject(reverseInsertionOrder([0, 1, 2, 3]).group((x, i) => i >= 2), {"false": [0, 1], "true": [2, 3]});

shouldBeObject(reverseInsertionOrder(mixPartialAndFast).groupBy((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, undefined, undefined, sparseArrayLength - 1]});
shouldBeObject(reverseInsertionOrder(mixPartialAndFast).groupBy((x, i) => i >= 2), {"false": [0, 1], "true": [2, undefined, undefined, sparseArrayLength - 1]});
shouldBeObject(reverseInsertionOrder(mixPartialAndFast).group((x, i) => i >= 2 ? "a" : "b"), {"b": [0, 1], "a": [2, undefined, undefined, sparseArrayLength - 1]});
shouldBeObject(reverseInsertionOrder(mixPartialAndFast).group((x, i) => i >= 2), {"false": [0, 1], "true": [2, undefined, undefined, sparseArrayLength - 1]});


// Extra callback parameters

shouldBeObject([0, 1, 2, 3].groupBy((i, j, k, l, m) => m = !m), {"true": [0, 1, 2, 3]});
shouldBeObject([0, 1, 2, 3].group((i, j, k, l, m) => m = !m), {"true": [0, 1, 2, 3]});


// Special keys

shouldBeObject([0, 1, 2, 3].groupBy((x) => "constructor").constructor, [0, 1, 2, 3]);
shouldBeObject([0, 1, 2, 3].group((x) => "constructor").constructor, [0, 1, 2, 3]);

shouldBeObject([0, 1, 2, 3].groupBy((x) => "prototype").prototype, [0, 1, 2, 3]);
shouldBeObject([0, 1, 2, 3].groupBy((x) => "__proto__").__proto__, [0, 1, 2, 3]);
shouldBeObject([0, 1, 2, 3].group((x) => "prototype").prototype, [0, 1, 2, 3]);
shouldBeObject([0, 1, 2, 3].group((x) => "__proto__").__proto__, [0, 1, 2, 3]);

shouldBeObject([0, 1, 2, 3].groupBy((x) => -0)[0], [0, 1, 2, 3]);
shouldBeObject([0, 1, 2, 3].groupBy((x) => 0)[0], [0, 1, 2, 3]);
shouldBeObject([0, 1, 2, 3].group((x) => -0)[0], [0, 1, 2, 3]);
shouldBeObject([0, 1, 2, 3].group((x) => 0)[0], [0, 1, 2, 3]);

let objectWithToStringCounter = {
counter: 0,
toString() { return this.counter++; },
};
shouldBeObject([0, 1, 2, 3].groupBy((x) => objectWithToStringCounter), {"0": [0], "1": [1], "2": [2], "3": [3]});
shouldBeObject([0, 1, 2, 3].group((x) => objectWithToStringCounter), {"0": [0], "1": [1], "2": [2], "3": [3]});
shouldBe(objectWithToStringCounter.counter, 4);

try {
shouldBeObject([0, 1, 2, 3].groupBy((x) => objectWithToStringThatThrows), {});
shouldBeObject([0, 1, 2, 3].group((x) => objectWithToStringThatThrows), {});
notReached();
} catch (e) {
shouldBe(e.message, "should not reach here");
}

shouldBeObject([0, 1, 2, 3].groupBy((x) => objectWithValueOfThatThrows)["[object Object]"], [0, 1, 2, 3]);
shouldBeObject([0, 1, 2, 3].group((x) => objectWithValueOfThatThrows)["[object Object]"], [0, 1, 2, 3]);

shouldBeObject([0, 1, 2, 3].groupBy((x) => symbol)[symbol], [0, 1, 2, 3]);
shouldBeObject([0, 1, 2, 3].group((x) => symbol)[symbol], [0, 1, 2, 3]);

0 comments on commit 317e700

Please sign in to comment.