Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[JSC] Revert String.prototype.item
https://bugs.webkit.org/show_bug.cgi?id=217449

Reviewed by Yusuke Suzuki.

JSTests:

* stress/item-method.js:
* test262/config.yaml:

Source/JavaScriptCore:

This patch reverts the String part of r267814, as it has been shown to be web-incompatible:
tc39/proposal-relative-indexing-method#31

Thankfully, this was the inessential part of the proposal; the core parts (Array and %TypedArray%) remain for now.

* builtins/StringPrototype.js:
(item): Deleted.
* runtime/StringPrototype.cpp:

LayoutTests:

* js/Object-getOwnPropertyNames-expected.txt:
* js/script-tests/Object-getOwnPropertyNames.js:


Canonical link: https://commits.webkit.org/230228@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@268165 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rkirsling committed Oct 8, 2020
1 parent 60ee4e3 commit 313b1ff
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 40 deletions.
10 changes: 10 additions & 0 deletions JSTests/ChangeLog
@@ -1,3 +1,13 @@
2020-10-07 Ross Kirsling <ross.kirsling@sony.com>

[JSC] Revert String.prototype.item
https://bugs.webkit.org/show_bug.cgi?id=217449

Reviewed by Yusuke Suzuki.

* stress/item-method.js:
* test262/config.yaml:

2020-10-07 Caio Lima <ticaiolima@gmail.com>

[MIPS] Flaky test stress/array-species-create-should-handle-masquerader.js
Expand Down
18 changes: 0 additions & 18 deletions JSTests/stress/item-method.js
Expand Up @@ -47,21 +47,3 @@ for (const TA of [Int8Array, Uint8Array, Uint8ClampedArray, Int16Array, Uint16Ar
shouldBe(ta.item(null), ta[0]);
shouldBe(ta.item({ valueOf: () => -1 }), ta[ta.length - 1]);
}

shouldBe(String.prototype.item.length, 1);
shouldThrowTypeError(() => String.prototype.item.call(undefined));
shouldThrowTypeError(() => String.prototype.item.call(null));

const string = 'abc';
// intentionally go one too far to ensure that we get undefined instead of wrapping
for (let i = 0; i <= string.length; i++) {
shouldBe(string.item(i), string[i]);
shouldBe(string.item(-i - 1), string[string.length - i - 1]);
}
shouldBe(string.item(), string[0]);
shouldBe(string.item(null), string[0]);
shouldBe(string.item({ valueOf: () => -1 }), string[string.length - 1]);

const emojiPseudoString = { toString: () => '😅' };
shouldBe(String.prototype.item.call(emojiPseudoString, 0), '\u{d83d}');
shouldBe(String.prototype.item.call(emojiPseudoString, -1), '\u{de05}');
3 changes: 3 additions & 0 deletions JSTests/test262/config.yaml
Expand Up @@ -25,6 +25,9 @@ skip:
- regexp-match-indices
- top-level-await
- Intl.ListFormat

# remove once it's no longer in test262
- String.prototype.item
paths:
- test/built-ins/DataView/prototype/getBigInt64
- test/built-ins/DataView/prototype/getBigUint64
Expand Down
10 changes: 10 additions & 0 deletions LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
2020-10-07 Ross Kirsling <ross.kirsling@sony.com>

[JSC] Revert String.prototype.item
https://bugs.webkit.org/show_bug.cgi?id=217449

Reviewed by Yusuke Suzuki.

* js/Object-getOwnPropertyNames-expected.txt:
* js/script-tests/Object-getOwnPropertyNames.js:

2020-10-07 Tim Horton <timothy_horton@apple.com>

REGRESSION: Safari unable to load PDF in <embed> (docs.legalconnect.com)
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/js/Object-getOwnPropertyNames-expected.txt
Expand Up @@ -49,7 +49,7 @@ PASS getSortedOwnPropertyNames(Function.prototype) is ['apply', 'arguments', 'bi
PASS getSortedOwnPropertyNames(Array) is ['from', 'isArray', 'length', 'name', 'of', 'prototype']
PASS getSortedOwnPropertyNames(Array.prototype) is ['concat', 'constructor', 'copyWithin', 'entries', 'every', 'fill', 'filter', 'find', 'findIndex', 'flat', 'flatMap', 'forEach', 'includes', 'indexOf', 'item', 'join', 'keys', 'lastIndexOf', 'length', 'map', 'pop', 'push', 'reduce', 'reduceRight', 'reverse', 'shift', 'slice', 'some', 'sort', 'splice', 'toLocaleString', 'toString', 'unshift', 'values']
PASS getSortedOwnPropertyNames(String) is ['fromCharCode', 'fromCodePoint', 'length', 'name', 'prototype', 'raw']
PASS getSortedOwnPropertyNames(String.prototype) is ['anchor', 'big', 'blink', 'bold', 'charAt', 'charCodeAt', 'codePointAt', 'concat', 'constructor', 'endsWith', 'fixed', 'fontcolor', 'fontsize', 'includes', 'indexOf', 'italics', 'item', 'lastIndexOf', 'length', 'link', 'localeCompare', 'match', 'matchAll', 'normalize', 'padEnd', 'padStart', 'repeat', 'replace', 'replaceAll', 'search', 'slice', 'small', 'split', 'startsWith', 'strike', 'sub', 'substr', 'substring', 'sup', 'toLocaleLowerCase', 'toLocaleUpperCase', 'toLowerCase', 'toString', 'toUpperCase', 'trim', 'trimEnd', 'trimLeft', 'trimRight', 'trimStart', 'valueOf']
PASS getSortedOwnPropertyNames(String.prototype) is ['anchor', 'big', 'blink', 'bold', 'charAt', 'charCodeAt', 'codePointAt', 'concat', 'constructor', 'endsWith', 'fixed', 'fontcolor', 'fontsize', 'includes', 'indexOf', 'italics', 'lastIndexOf', 'length', 'link', 'localeCompare', 'match', 'matchAll', 'normalize', 'padEnd', 'padStart', 'repeat', 'replace', 'replaceAll', 'search', 'slice', 'small', 'split', 'startsWith', 'strike', 'sub', 'substr', 'substring', 'sup', 'toLocaleLowerCase', 'toLocaleUpperCase', 'toLowerCase', 'toString', 'toUpperCase', 'trim', 'trimEnd', 'trimLeft', 'trimRight', 'trimStart', 'valueOf']
PASS getSortedOwnPropertyNames(Boolean) is ['length', 'name', 'prototype']
PASS getSortedOwnPropertyNames(Boolean.prototype) is ['constructor', 'toString', 'valueOf']
PASS getSortedOwnPropertyNames(Number) is ['EPSILON', 'MAX_SAFE_INTEGER', 'MAX_VALUE', 'MIN_SAFE_INTEGER', 'MIN_VALUE', 'NEGATIVE_INFINITY', 'NaN', 'POSITIVE_INFINITY', 'isFinite', 'isInteger', 'isNaN', 'isSafeInteger', 'length', 'name', 'parseFloat', 'parseInt', 'prototype']
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/js/script-tests/Object-getOwnPropertyNames.js
Expand Up @@ -58,7 +58,7 @@ var expectedPropertyNamesSet = {
"Array": "['from', 'isArray', 'length', 'name', 'of', 'prototype']",
"Array.prototype": "['concat', 'constructor', 'copyWithin', 'entries', 'every', 'fill', 'filter', 'find', 'findIndex', 'flat', 'flatMap', 'forEach', 'includes', 'indexOf', 'item', 'join', 'keys', 'lastIndexOf', 'length', 'map', 'pop', 'push', 'reduce', 'reduceRight', 'reverse', 'shift', 'slice', 'some', 'sort', 'splice', 'toLocaleString', 'toString', 'unshift', 'values']",
"String": "['fromCharCode', 'fromCodePoint', 'length', 'name', 'prototype', 'raw']",
"String.prototype": "['anchor', 'big', 'blink', 'bold', 'charAt', 'charCodeAt', 'codePointAt', 'concat', 'constructor', 'endsWith', 'fixed', 'fontcolor', 'fontsize', 'includes', 'indexOf', 'italics', 'item', 'lastIndexOf', 'length', 'link', 'localeCompare', 'match', 'matchAll', 'normalize', 'padEnd', 'padStart', 'repeat', 'replace', 'replaceAll', 'search', 'slice', 'small', 'split', 'startsWith', 'strike', 'sub', 'substr', 'substring', 'sup', 'toLocaleLowerCase', 'toLocaleUpperCase', 'toLowerCase', 'toString', 'toUpperCase', 'trim', 'trimEnd', 'trimLeft', 'trimRight', 'trimStart', 'valueOf']",
"String.prototype": "['anchor', 'big', 'blink', 'bold', 'charAt', 'charCodeAt', 'codePointAt', 'concat', 'constructor', 'endsWith', 'fixed', 'fontcolor', 'fontsize', 'includes', 'indexOf', 'italics', 'lastIndexOf', 'length', 'link', 'localeCompare', 'match', 'matchAll', 'normalize', 'padEnd', 'padStart', 'repeat', 'replace', 'replaceAll', 'search', 'slice', 'small', 'split', 'startsWith', 'strike', 'sub', 'substr', 'substring', 'sup', 'toLocaleLowerCase', 'toLocaleUpperCase', 'toLowerCase', 'toString', 'toUpperCase', 'trim', 'trimEnd', 'trimLeft', 'trimRight', 'trimStart', 'valueOf']",
"Boolean": "['length', 'name', 'prototype']",
"Boolean.prototype": "['constructor', 'toString', 'valueOf']",
"Number": "['EPSILON', 'MAX_SAFE_INTEGER', 'MAX_VALUE', 'MIN_SAFE_INTEGER', 'MIN_VALUE', 'NEGATIVE_INFINITY', 'NaN', 'POSITIVE_INFINITY', 'isFinite', 'isInteger', 'isNaN', 'isSafeInteger', 'length', 'name', 'parseFloat', 'parseInt', 'prototype']",
Expand Down
16 changes: 16 additions & 0 deletions Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,19 @@
2020-10-07 Ross Kirsling <ross.kirsling@sony.com>

[JSC] Revert String.prototype.item
https://bugs.webkit.org/show_bug.cgi?id=217449

Reviewed by Yusuke Suzuki.

This patch reverts the String part of r267814, as it has been shown to be web-incompatible:
https://github.com/tc39/proposal-item-method/issues/31

Thankfully, this was the inessential part of the proposal; the core parts (Array and %TypedArray%) remain for now.

* builtins/StringPrototype.js:
(item): Deleted.
* runtime/StringPrototype.cpp:

2020-10-07 Keith Rollin <krollin@apple.com>

Update post-processing rules for headers to not unnecessarily change timestamps
Expand Down
19 changes: 0 additions & 19 deletions Source/JavaScriptCore/builtins/StringPrototype.js
Expand Up @@ -340,25 +340,6 @@ function concat(arg /* ... */)
return @tailCallForwardArguments(@stringConcatSlowPath, this);
}

// FIXME: This is extremely similar to charAt, so we should optimize it accordingly.
// https://bugs.webkit.org/show_bug.cgi?id=217139
function item(index)
{
"use strict";

if (@isUndefinedOrNull(this))
@throwTypeError("String.prototype.item requires that |this| not be null or undefined");

var string = @toString(this);
var length = string.length;

var k = @toInteger(index);
if (k < 0)
k += length;

return (k >= 0 && k < length) ? string[k] : @undefined;
}

@globalPrivate
function createHTML(func, string, tag, attribute, value)
{
Expand Down
1 change: 0 additions & 1 deletion Source/JavaScriptCore/runtime/StringPrototype.cpp
Expand Up @@ -90,7 +90,6 @@ const ClassInfo StringPrototype::s_info = { "String", &StringObject::s_info, &st
/* Source for StringConstructor.lut.h
@begin stringPrototypeTable
concat JSBuiltin DontEnum|Function 1
item JSBuiltin DontEnum|Function 1
match JSBuiltin DontEnum|Function 1
matchAll JSBuiltin DontEnum|Function 1
padStart JSBuiltin DontEnum|Function 1
Expand Down

0 comments on commit 313b1ff

Please sign in to comment.