Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (198293 => 198294)
--- trunk/Source/_javascript_Core/ChangeLog 2016-03-16 19:47:48 UTC (rev 198293)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-03-16 19:49:36 UTC (rev 198294)
@@ -1,3 +1,25 @@
+2016-03-16 Saam Barati <[email protected]>
+
+ [ES6] Make Array.prototype.reverse spec compatible.
+ https://bugs.webkit.org/show_bug.cgi?id=155528
+
+ Reviewed by Michael Saboff.
+
+ This patch make Array.prototype.reverse spec compatible.
+ Before, we weren't performing a HasProperty of each index
+ before performing a Get on that index. We now do that on
+ the slow path.
+
+ * runtime/ArrayPrototype.cpp:
+ (JSC::arrayProtoFuncReverse):
+ * tests/stress/array-reverse-proxy.js: Added.
+ (assert):
+ (test):
+ (shallowCopy):
+ (shallowEqual):
+ (let.handler.get getSet):
+ (test.let.handler.get getSet):
+
2016-03-16 Chris Dumez <[email protected]>
Unreviewed, rolling out r198235, r198240, r198241, and
Modified: trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp (198293 => 198294)
--- trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2016-03-16 19:47:48 UTC (rev 198293)
+++ trunk/Source/_javascript_Core/runtime/ArrayPrototype.cpp 2016-03-16 19:49:36 UTC (rev 198294)
@@ -737,8 +737,9 @@
if (!thisObject)
return JSValue::encode(JSValue());
+ VM& vm = exec->vm();
unsigned length = getLength(exec, thisObject);
- if (exec->hadException())
+ if (vm.exception())
return JSValue::encode(jsUndefined());
switch (thisObject->indexingType()) {
@@ -776,31 +777,40 @@
}
unsigned middle = length / 2;
- for (unsigned k = 0; k < middle; k++) {
- unsigned lk1 = length - k - 1;
- JSValue obj2 = getProperty(exec, thisObject, lk1);
- if (exec->hadException())
+ for (unsigned lower = 0; lower < middle; lower++) {
+ unsigned upper = length - lower - 1;
+ bool lowerExists = thisObject->hasProperty(exec, lower);
+ if (vm.exception())
return JSValue::encode(jsUndefined());
- JSValue obj = getProperty(exec, thisObject, k);
- if (exec->hadException())
+ JSValue lowerValue;
+ if (lowerExists)
+ lowerValue = thisObject->get(exec, lower);
+
+ bool upperExists = thisObject->hasProperty(exec, upper);
+ if (vm.exception())
return JSValue::encode(jsUndefined());
+ JSValue upperValue;
+ if (upperExists)
+ upperValue = thisObject->get(exec, upper);
- if (obj2) {
- thisObject->putByIndexInline(exec, k, obj2, true);
- if (exec->hadException())
- return JSValue::encode(jsUndefined());
- } else if (!thisObject->methodTable(exec->vm())->deletePropertyByIndex(thisObject, exec, k)) {
- throwTypeError(exec, ASCIILiteral("Unable to delete property."));
- return JSValue::encode(jsUndefined());
+ if (upperExists) {
+ thisObject->putByIndexInline(exec, lower, upperValue, true);
+ if (vm.exception())
+ return JSValue::encode(JSValue());
+ } else if (!thisObject->methodTable(vm)->deletePropertyByIndex(thisObject, exec, lower)) {
+ if (!vm.exception())
+ throwTypeError(exec, ASCIILiteral("Unable to delete property."));
+ return JSValue::encode(JSValue());
}
- if (obj) {
- thisObject->putByIndexInline(exec, lk1, obj, true);
- if (exec->hadException())
- return JSValue::encode(jsUndefined());
- } else if (!thisObject->methodTable(exec->vm())->deletePropertyByIndex(thisObject, exec, lk1)) {
- throwTypeError(exec, ASCIILiteral("Unable to delete property."));
- return JSValue::encode(jsUndefined());
+ if (lowerExists) {
+ thisObject->putByIndexInline(exec, upper, lowerValue, true);
+ if (vm.exception())
+ return JSValue::encode(JSValue());
+ } else if (!thisObject->methodTable(vm)->deletePropertyByIndex(thisObject, exec, upper)) {
+ if (!vm.exception())
+ throwTypeError(exec, ASCIILiteral("Unable to delete property."));
+ return JSValue::encode(JSValue());
}
}
return JSValue::encode(thisObject);
Modified: trunk/Source/_javascript_Core/tests/es6.yaml (198293 => 198294)
--- trunk/Source/_javascript_Core/tests/es6.yaml 2016-03-16 19:47:48 UTC (rev 198293)
+++ trunk/Source/_javascript_Core/tests/es6.yaml 2016-03-16 19:49:36 UTC (rev 198294)
@@ -941,7 +941,7 @@
- path: es6/Proxy_internal_deleteProperty_calls_Array.prototype.pop.js
cmd: runES6 :normal
- path: es6/Proxy_internal_deleteProperty_calls_Array.prototype.reverse.js
- cmd: runES6 :fail
+ cmd: runES6 :normal
- path: es6/Proxy_internal_deleteProperty_calls_Array.prototype.shift.js
cmd: runES6 :fail
- path: es6/Proxy_internal_deleteProperty_calls_Array.prototype.splice.js
@@ -955,7 +955,7 @@
- path: es6/Proxy_internal_get_calls_Array.prototype.pop.js
cmd: runES6 :normal
- path: es6/Proxy_internal_get_calls_Array.prototype.reverse.js
- cmd: runES6 :fail
+ cmd: runES6 :normal
- path: es6/Proxy_internal_get_calls_Array.prototype.shift.js
cmd: runES6 :normal
- path: es6/Proxy_internal_get_calls_Array.prototype.splice.js
@@ -1047,7 +1047,7 @@
- path: es6/Proxy_internal_set_calls_Array.prototype.push.js
cmd: runES6 :normal
- path: es6/Proxy_internal_set_calls_Array.prototype.reverse.js
- cmd: runES6 :fail
+ cmd: runES6 :normal
- path: es6/Proxy_internal_set_calls_Array.prototype.shift.js
cmd: runES6 :fail
- path: es6/Proxy_internal_set_calls_Array.prototype.splice.js
Added: trunk/Source/_javascript_Core/tests/stress/array-reverse-proxy.js (0 => 198294)
--- trunk/Source/_javascript_Core/tests/stress/array-reverse-proxy.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/array-reverse-proxy.js 2016-03-16 19:49:36 UTC (rev 198294)
@@ -0,0 +1,221 @@
+function assert(b) {
+ if (!b)
+ throw new Error("bad assertion!");
+}
+
+function test(f) {
+ for (let i = 0; i < 1000; i++)
+ f();
+}
+
+function shallowCopy(arr) {
+ let result = [];
+ for (let item of arr)
+ result.push(item);
+ return result;
+}
+
+function shallowEqual(a, b) {
+ if (a.length !== b.length)
+ return false;
+ for (let i = 0; i < a.length; i++) {
+ if (a[i] !== b[i])
+ return false;
+ }
+
+ return true;
+}
+
+test(function() {
+ let target = [10, 20, 30, 40];
+ let copy = shallowCopy(target);
+ let handler = { };
+ let proxy = new Proxy(target, handler);
+ proxy.reverse();
+ copy.reverse();
+ assert(shallowEqual(proxy, target));
+ assert(shallowEqual(target, copy));
+});
+
+test(function() {
+ let target = [10, 20, 30, 40];
+ let copy = shallowCopy(target);
+ let getSet = new Set;
+ let hasSet = new Set;
+ let handler = {
+ get(theTarget, key) {
+ getSet.add(key);
+ return theTarget[key];
+ },
+ has(theTarget, key) {
+ hasSet.add(key);
+ return Reflect.has(theTarget, key);
+ }
+ };
+ let proxy = new Proxy(target, handler);
+ proxy.reverse();
+ copy.reverse();
+ assert(shallowEqual(proxy, target));
+ assert(shallowEqual(target, copy));
+
+ assert(getSet.has("0"));
+ assert(getSet.has("1"));
+ assert(getSet.has("2"));
+ assert(getSet.has("3"));
+ assert(getSet.has("length"));
+
+ assert(hasSet.has("0"));
+ assert(hasSet.has("1"));
+ assert(hasSet.has("2"));
+ assert(hasSet.has("3"));
+});
+
+test(function() {
+ let target = [10, 20, 30, 40];
+ let getSet = new Set;
+ let hasSet = new Set;
+ let deleteSet = new Set;
+ let handler = {
+ get(theTarget, key) {
+ getSet.add(key);
+ return theTarget[key];
+ },
+ has(theTarget, key) {
+ hasSet.add(key);
+ if (key === "0" || key === "1")
+ return true;
+ assert(key === "2" || key === "3");
+ return false;
+ },
+ deleteProperty(theTarget, key) {
+ deleteSet.add(key);
+ return Reflect.deleteProperty(theTarget, key);
+ }
+ };
+
+ let proxy = new Proxy(target, handler);
+ proxy.reverse();
+ assert(shallowEqual(proxy, target));
+
+ assert(getSet.has("0"));
+ assert(getSet.has("1"));
+ assert(getSet.has("2"));
+ assert(getSet.has("3"));
+ assert(getSet.has("length"));
+ assert(getSet.has("reverse"));
+ assert(getSet.size === 6);
+
+ assert(hasSet.has("0"));
+ assert(hasSet.has("1"));
+ assert(hasSet.has("2"));
+ assert(hasSet.has("3"));
+ assert(hasSet.size === 4);
+
+ assert(deleteSet.has("0"));
+ assert(deleteSet.has("1"));
+ assert(!deleteSet.has("2"));
+ assert(!deleteSet.has("3"));
+});
+
+test(function() {
+ let target = [10, 20, 30, 40];
+ let error;
+ let handler = {
+ has(theTarget, key) {
+ return false;
+ },
+ deleteProperty(theTarget, key) {
+ if (key === "0") {
+ error = new Error;
+ throw error;
+ }
+ return true;
+ }
+ };
+
+ let proxy = new Proxy(target, handler);
+ let threw = false;
+ try {
+ proxy.reverse();
+ } catch(e) {
+ threw = true;
+ assert(e === error);
+ }
+ assert(threw);
+});
+
+test(function() {
+ let target = [10, 20, 30, 40];
+ let error;
+ let handler = {
+ has(theTarget, key) {
+ if (key === "0") {
+ error = new Error;
+ throw error;
+ }
+ return false;
+ }
+ };
+
+ let proxy = new Proxy(target, handler);
+ let threw = false;
+ try {
+ proxy.reverse();
+ } catch(e) {
+ threw = true;
+ assert(e === error);
+ }
+ assert(threw);
+});
+
+test(function() {
+ let target = [10, 20, 30, 40];
+ let error;
+ let handler = {
+ has(theTarget, key) {
+ if (key === "3") {
+ error = new Error;
+ throw error;
+ }
+ return false;
+ }
+ };
+
+ let proxy = new Proxy(target, handler);
+ let threw = false;
+ try {
+ proxy.reverse();
+ } catch(e) {
+ threw = true;
+ assert(e === error);
+ }
+ assert(threw);
+});
+
+test(function() {
+ let target = [10, 20, 30, 40];
+ let error;
+ let getSet = new Set;
+ let handler = {
+ get(theTarget, key) {
+ getSet.add(key);
+ return theTarget[key];
+ },
+ has(theTarget, key) {
+ return false;
+ },
+ deleteProperty() {
+ return true;
+ }
+ };
+
+ let proxy = new Proxy(target, handler);
+ proxy.reverse();
+ assert(!getSet.has("0"));
+ assert(!getSet.has("1"));
+ assert(!getSet.has("2"));
+ assert(!getSet.has("3"));
+ assert(getSet.size === 2);
+ assert(getSet.has("reverse"));
+ assert(getSet.has("length"));
+});
