Skip to content

Commit

Permalink
Pull Request mozilla#1435: Fixes for Symbol.iterator handling in Nati…
Browse files Browse the repository at this point in the history
…veArray
  • Loading branch information
rbri committed Jan 10, 2024
2 parents ef18cb9 + 2446750 commit 05b3c6d
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 34 deletions.
25 changes: 6 additions & 19 deletions src/org/mozilla/javascript/Arguments.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@

package org.mozilla.javascript;

import org.mozilla.javascript.NativeArrayIterator.ARRAY_ITERATOR_TYPE;

/**
* This class implements the "arguments" object.
*
Expand Down Expand Up @@ -41,7 +39,12 @@ public Arguments(NativeCall activation) {
callerObj = NOT_FOUND;
}

defineProperty(SymbolKey.ITERATOR, iteratorMethod, ScriptableObject.DONTENUM);
defineProperty(
SymbolKey.ITERATOR,
TopLevel.getBuiltinPrototype(
ScriptableObject.getTopLevelScope(parent), TopLevel.Builtins.Array)
.get("values", parent),
ScriptableObject.DONTENUM);
}

@Override
Expand Down Expand Up @@ -402,22 +405,6 @@ void defineAttributesForStrictMode() {
calleeObj = null;
}

private static BaseFunction iteratorMethod =
new BaseFunction() {
private static final long serialVersionUID = 4239122318596177391L;

@Override
public Object call(
Context cx, Scriptable scope, Scriptable thisObj, Object[] args) {
// TODO : call %ArrayProto_values%
// 9.4.4.6 CreateUnmappedArgumentsObject(argumentsList)
// 1. Perform DefinePropertyOrThrow(obj, @@iterator, PropertyDescriptor
// {[[Value]]:%ArrayProto_values%,
// [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true}).
return new NativeArrayIterator(scope, thisObj, ARRAY_ITERATOR_TYPE.VALUES);
}
};

private static class ThrowTypeError extends BaseFunction {
private static final long serialVersionUID = -744615873947395749L;
private String propertyName;
Expand Down
50 changes: 41 additions & 9 deletions src/org/mozilla/javascript/NativeArray.java
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,6 @@ protected void fillConstructorProperties(IdFunctionObject ctor) {

@Override
protected void initPrototypeId(int id) {
if (id == SymbolId_iterator) {
initPrototypeMethod(ARRAY_TAG, id, SymbolKey.ITERATOR, "[Symbol.iterator]", 0);
return;
}

String s, fnName = null;
int arity;
switch (id) {
Expand Down Expand Up @@ -471,7 +466,6 @@ public Object execIdCall(
scope, thisObj, NativeArrayIterator.ARRAY_ITERATOR_TYPE.ENTRIES);

case Id_values:
case SymbolId_iterator:
thisObj = ScriptRuntime.toObject(cx, scope, thisObj);
return new NativeArrayIterator(
scope, thisObj, NativeArrayIterator.ARRAY_ITERATOR_TYPE.VALUES);
Expand All @@ -495,6 +489,42 @@ public boolean has(int index, Scriptable start) {
return super.has(index, start);
}

@Override
public boolean has(Symbol key, Scriptable start) {
if (SymbolKey.ITERATOR.equals(key)) {
return super.has("values", start);
}

return super.has(key, start);
}

@Override
public Object get(Symbol key, Scriptable start) {
if (SymbolKey.ITERATOR.equals(key)) {
return super.get("values", start);
}

return super.get(key, start);
}

@Override
public void put(Symbol key, Scriptable start, Object value) {
if (SymbolKey.ITERATOR.equals(key)) {
super.put("values", start, value);
}

super.put(key, start, value);
}

@Override
public void delete(Symbol key) {
if (SymbolKey.ITERATOR.equals(key)) {
super.delete("values");
}

super.delete(key);
}

private static long toArrayIndex(Object id) {
if (id instanceof String) {
return toArrayIndex((String) id);
Expand Down Expand Up @@ -2519,7 +2549,10 @@ private void checkModCount(int modCount) {
@Override
protected int findPrototypeId(Symbol k) {
if (SymbolKey.ITERATOR.equals(k)) {
return SymbolId_iterator;
// "Symbol.iterator" property of the prototype has the "same value"
// as the "values" property. We implement this by returning the
// ID of "values" when the iterator symbol is accessed.
return Id_values;
}
return 0;
}
Expand Down Expand Up @@ -2729,8 +2762,7 @@ protected int findPrototypeId(String s) {
Id_at = 32,
Id_flat = 33,
Id_flatMap = 34,
SymbolId_iterator = 35,
MAX_PROTOTYPE_ID = SymbolId_iterator;
MAX_PROTOTYPE_ID = Id_flatMap;
private static final int ConstructorId_join = -Id_join,
ConstructorId_reverse = -Id_reverse,
ConstructorId_sort = -Id_sort,
Expand Down
2 changes: 1 addition & 1 deletion testsrc/jstests/harmony/for-of.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ assertThrows('for each (n of [1,2]) {}', SyntaxError);

assertThrows('[n*n for each (n of [1,2])]', SyntaxError);

assertEquals('[Symbol.iterator]', Array.prototype[Symbol.iterator].name);
assertEquals('values', Array.prototype[Symbol.iterator].name);
assertEquals('[Symbol.iterator]', String.prototype[Symbol.iterator].name);

// should have `value` and `done` property.
Expand Down
66 changes: 66 additions & 0 deletions testsrc/org/mozilla/javascript/tests/es6/ArgumentsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

package org.mozilla.javascript.tests.es6;

import static org.junit.Assert.assertEquals;

import org.junit.Test;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.ScriptableObject;
import org.mozilla.javascript.tests.Utils;

/** Tests for Arguments support. */
public class ArgumentsTest {

@Test
public void argumentsSymbolIterator() {
String code =
"function foo() {"
+ " return arguments[Symbol.iterator] === Array.prototype.values;"
+ "}"
+ "foo()";

test(true, code);
}

@Test
public void argumentsSymbolIterator2() {
String code =
"function foo() {"
+ " return arguments[Symbol.iterator] === [][Symbol.iterator];"
+ "}"
+ "foo()";

test(true, code);
}

@Test
public void argumentsForOf() {
String code =
"function foo() {"
+ " var res = '';"
+ " for (arg of arguments) {"
+ " res += arg;"
+ " }"
+ " return res;"
+ "}"
+ "foo(1, 2, 3, 5)";

test("1235", code);
}

private static void test(Object expected, String js) {
Utils.runWithAllOptimizationLevels(
cx -> {
cx.setLanguageVersion(Context.VERSION_ES6);
ScriptableObject scope = cx.initStandardObjects();

Object result = cx.evaluateString(scope, js, "test", 1, null);
assertEquals(expected, result);

return null;
});
}
}
74 changes: 74 additions & 0 deletions testsrc/org/mozilla/javascript/tests/es6/NativeArray3Test.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

package org.mozilla.javascript.tests.es6;

import static org.junit.Assert.assertEquals;

import org.junit.Test;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.ScriptableObject;
import org.mozilla.javascript.tests.Utils;

/** Tests for NativeArray support. */
public class NativeArray3Test {

@Test
public void iteratorPrototype() {
String code = "Array.prototype.values === [][Symbol.iterator]";

test(true, code);
}

@Test
public void iteratorInstances() {
String code = "[1, 2][Symbol.iterator] === [][Symbol.iterator]";

test(true, code);
}

@Test
public void iteratorPrototypeName() {
String code = "Array.prototype.values.name;";

test("values", code);
}

@Test
public void iteratorInstanceName() {
String code = "[][Symbol.iterator].name;";

test("values", code);
}

@Test
public void redefineIterator() {
String code =
"var res = '';\n"
+ "var arr = ['hello', 'world'];\n"
+ "res += arr[Symbol.iterator].toString().includes('return i;');\n"
+ "res += ' - ';\n"
+ "arr[Symbol.iterator] = function () { return i; };\n"
+ "res += arr[Symbol.iterator].toString().includes('return i;');\n"
+ "res += ' - ';\n"
+ "delete arr[Symbol.iterator];\n"
+ "res += arr[Symbol.iterator].toString().includes('return i;');\n"
+ "res;";

test("false - true - false", code);
}

private static void test(Object expected, String js) {
Utils.runWithAllOptimizationLevels(
cx -> {
cx.setLanguageVersion(Context.VERSION_ES6);
ScriptableObject scope = cx.initStandardObjects();

Object result = cx.evaluateString(scope, js, "test", 1, null);
assertEquals(expected, result);

return null;
});
}
}
7 changes: 2 additions & 5 deletions testsrc/test262.properties
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This is a configuration file for Test262SuiteTest.java. See ./README.md for more info about this file

built-ins/Array 177/2670 (6.63%)
built-ins/Array 176/2670 (6.59%)
from/calling-from-valid-1-noStrict.js non-strict Spec pretty clearly says this should be undefined
from/elements-deleted-after.js Checking to see if length changed, but spec says it should not
from/iter-map-fn-this-non-strict.js non-strict Error propagation needs work in general
Expand Down Expand Up @@ -169,7 +169,6 @@ built-ins/Array 177/2670 (6.63%)
prototype/toLocaleString/primitive_this_value_getter.js strict
prototype/unshift/throws-with-string-receiver.js
prototype/methods-called-as-functions.js {unsupported: [Symbol.species]}
prototype/Symbol.iterator.js Expects a particular string value
Symbol.species 4/4 (100.0%)
proto-from-ctor-realm-one.js {unsupported: [Reflect]}
proto-from-ctor-realm-two.js {unsupported: [Reflect]}
Expand Down Expand Up @@ -2447,7 +2446,7 @@ built-ins/WeakMap 1/88 (1.14%)
built-ins/WeakSet 1/75 (1.33%)
proto-from-ctor-realm.js {unsupported: [Reflect]}

language/arguments-object 191/260 (73.46%)
language/arguments-object 189/260 (72.69%)
mapped/mapped-arguments-nonconfigurable-3.js non-strict
mapped/mapped-arguments-nonconfigurable-delete-1.js non-strict
mapped/mapped-arguments-nonconfigurable-delete-2.js non-strict
Expand Down Expand Up @@ -2483,8 +2482,6 @@ language/arguments-object 191/260 (73.46%)
mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-basic.js non-strict
mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-set-by-arguments.js non-strict
mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-set-by-param.js non-strict
mapped/Symbol.iterator.js non-strict
unmapped/Symbol.iterator.js non-strict
unmapped/via-params-dflt.js
unmapped/via-params-dstr.js non-strict
unmapped/via-params-rest.js
Expand Down

0 comments on commit 05b3c6d

Please sign in to comment.