8164467: ES6 computed properties are implemented wrongly
Reviewed-by: sundar, lagergren
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Thu Sep 22 18:32:42 2016 +0000
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java Mon Sep 26 13:27:45 2016 +0200
@@ -2512,7 +2512,8 @@
final List<PropertyNode> elements = objectNode.getElements();
final List<MapTuple<Expression>> tuples = new ArrayList<>();
- final List<PropertyNode> gettersSetters = new ArrayList<>();
+ // List below will contain getter/setter properties and properties with computed keys (ES6)
+ final List<PropertyNode> specialProperties = new ArrayList<>();
final int ccp = getCurrentContinuationEntryPoint();
final List<Splittable.SplitRange> ranges = objectNode.getSplitRanges();
@@ -2522,11 +2523,14 @@
for (final PropertyNode propertyNode : elements) {
final Expression value = propertyNode.getValue();
final String key = propertyNode.getKeyName();
+ final boolean isComputedOrAccessor = propertyNode.isComputed() || value == null;
+
// Just use a pseudo-symbol. We just need something non null; use the name and zero flags.
- final Symbol symbol = value == null ? null : new Symbol(key, 0);
-
- if (value == null) {
- gettersSetters.add(propertyNode);
+ final Symbol symbol = isComputedOrAccessor ? null : new Symbol(key, 0);
+
+ if (isComputedOrAccessor) {
+ // Properties with computed names or getter/setters need special handling.
+ specialProperties.add(propertyNode);
} else if (propertyNode.getKey() instanceof IdentNode &&
key.equals(ScriptObject.PROTO_PROPERTY_NAME)) {
// ES6 draft compliant __proto__ inside object literal
@@ -2542,7 +2546,7 @@
//for literals, a value of null means object type, i.e. the value null or getter setter function
//(I think)
- final Class<?> valueType = (!useDualFields() || value == null || value.getType().isBoolean()) ? Object.class : value.getType().getTypeClass();
+ final Class<?> valueType = (!useDualFields() || isComputedOrAccessor || value.getType().isBoolean()) ? Object.class : value.getType().getTypeClass();
tuples.add(new MapTuple<Expression>(key, symbol, Type.typeFor(valueType), value) {
@Override
public Class<?> getValueType() {
@@ -2590,26 +2594,41 @@
method.invoke(ScriptObject.SET_GLOBAL_OBJECT_PROTO);
}
- for (final PropertyNode propertyNode : gettersSetters) {
- final FunctionNode getter = propertyNode.getGetter();
- final FunctionNode setter = propertyNode.getSetter();
-
- assert getter != null || setter != null;
-
- method.dup().loadKey(propertyNode.getKey());
- if (getter == null) {
- method.loadNull();
+ for (final PropertyNode propertyNode : specialProperties) {
+
+ method.dup();
+
+ if (propertyNode.isComputed()) {
+ assert propertyNode.getKeyName() == null;
+ loadExpressionAsObject(propertyNode.getKey());
+ } else {
+ method.loadKey(propertyNode.getKey());
+ }
+
+ if (propertyNode.getValue() != null) {
+ loadExpressionAsObject(propertyNode.getValue());
+ method.load(0);
+ method.invoke(ScriptObject.GENERIC_SET);
} else {
- getter.accept(this);
- }
-
- if (setter == null) {
- method.loadNull();
- } else {
- setter.accept(this);
- }
-
- method.invoke(ScriptObject.SET_USER_ACCESSORS);
+ final FunctionNode getter = propertyNode.getGetter();
+ final FunctionNode setter = propertyNode.getSetter();
+
+ assert getter != null || setter != null;
+
+ if (getter == null) {
+ method.loadNull();
+ } else {
+ getter.accept(this);
+ }
+
+ if (setter == null) {
+ method.loadNull();
+ } else {
+ setter.accept(this);
+ }
+
+ method.invoke(ScriptObject.SET_USER_ACCESSORS);
+ }
}
}
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/PropertyNode.java Thu Sep 22 18:32:42 2016 +0000
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/PropertyNode.java Mon Sep 26 13:27:45 2016 +0200
@@ -86,11 +86,11 @@
}
/**
- * Get the name of the property key
- * @return key name
+ * Get the name of the property key, or {@code null} if key is a computed name.
+ * @return key name or null
*/
public String getKeyName() {
- return key instanceof PropertyKey ? ((PropertyKey) key).getPropertyName() : null;
+ return !computed && key instanceof PropertyKey ? ((PropertyKey) key).getPropertyName() : null;
}
@Override
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/Parser.java Thu Sep 22 18:32:42 2016 +0000
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/Parser.java Mon Sep 26 13:27:45 2016 +0200
@@ -1176,10 +1176,9 @@
labelStatement();
return;
}
- final boolean allowPropertyFunction = (reparseFlags & ScriptFunctionData.IS_PROPERTY_ACCESSOR) != 0;
- final boolean isES6Method = (reparseFlags & ScriptFunctionData.IS_ES6_METHOD) != 0;
- if(allowPropertyFunction) {
- final String ident = (String)getValue();
+
+ if ((reparseFlags & ScriptFunctionData.IS_PROPERTY_ACCESSOR) != 0) {
+ final String ident = (String) getValue();
final long propertyToken = token;
final int propertyLine = line;
if (GET_NAME.equals(ident)) {
@@ -1191,19 +1190,22 @@
addPropertyFunctionStatement(propertySetterFunction(propertyToken, propertyLine));
return;
}
- } else if (isES6Method) {
- final String ident = (String)getValue();
- IdentNode identNode = createIdentNode(token, finish, ident).setIsPropertyName();
- final long propertyToken = token;
- final int propertyLine = line;
- next();
- // Code below will need refinement once we fully support ES6 class syntax
- final int flags = CONSTRUCTOR_NAME.equals(ident) ? FunctionNode.ES6_IS_CLASS_CONSTRUCTOR : FunctionNode.ES6_IS_METHOD;
- addPropertyFunctionStatement(propertyMethodFunction(identNode, propertyToken, propertyLine, false, flags, false));
- return;
}
}
+ if ((reparseFlags & ScriptFunctionData.IS_ES6_METHOD) != 0
+ && (type == IDENT || type == LBRACKET || isNonStrictModeIdent())) {
+ final String ident = (String)getValue();
+ final long propertyToken = token;
+ final int propertyLine = line;
+ final Expression propertyKey = propertyName();
+
+ // Code below will need refinement once we fully support ES6 class syntax
+ final int flags = CONSTRUCTOR_NAME.equals(ident) ? FunctionNode.ES6_IS_CLASS_CONSTRUCTOR : FunctionNode.ES6_IS_METHOD;
+ addPropertyFunctionStatement(propertyMethodFunction(propertyKey, propertyToken, propertyLine, false, flags, false));
+ return;
+ }
+
expressionStatement();
break;
}
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java Thu Sep 22 18:32:42 2016 +0000
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java Mon Sep 26 13:27:45 2016 +0200
@@ -186,7 +186,10 @@
/** Method handle for setting the user accessors of a ScriptObject */
//TODO fastpath this
- public static final Call SET_USER_ACCESSORS = virtualCall(MethodHandles.lookup(), ScriptObject.class, "setUserAccessors", void.class, String.class, ScriptFunction.class, ScriptFunction.class);
+ public static final Call SET_USER_ACCESSORS = virtualCallNoLookup(ScriptObject.class, "setUserAccessors", void.class, Object.class, ScriptFunction.class, ScriptFunction.class);
+
+ /** Method handle for generic property setter */
+ public static final Call GENERIC_SET = virtualCallNoLookup(ScriptObject.class, "set", void.class, Object.class, Object.class, int.class);
static final MethodHandle[] SET_SLOW = new MethodHandle[] {
findOwnMH_V("set", void.class, Object.class, int.class, int.class),
@@ -1063,12 +1066,13 @@
* @param getter {@link UserAccessorProperty} defined getter, or null if none
* @param setter {@link UserAccessorProperty} defined setter, or null if none
*/
- public final void setUserAccessors(final String key, final ScriptFunction getter, final ScriptFunction setter) {
- final Property oldProperty = getMap().findProperty(key);
+ public final void setUserAccessors(final Object key, final ScriptFunction getter, final ScriptFunction setter) {
+ final Object realKey = JSType.toPropertyKey(key);
+ final Property oldProperty = getMap().findProperty(realKey);
if (oldProperty instanceof UserAccessorProperty) {
modifyOwnProperty(oldProperty, oldProperty.getFlags(), getter, setter);
} else {
- addOwnProperty(newUserAccessors(key, oldProperty != null ? oldProperty.getFlags() : 0, getter, setter));
+ addOwnProperty(newUserAccessors(realKey, oldProperty != null ? oldProperty.getFlags() : 0, getter, setter));
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/es6/computed-property-duplicate.js Mon Sep 26 13:27:45 2016 +0200
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8164467: ES6 computed properties are implemented wrongly
+ *
+ * @test
+ * @run
+ * @option --language=es6
+ */
+
+var obj = {
+ get ['a']() {
+ throw new Error('should not be called');
+ },
+ get ['a']() {
+ throw new Error('should not be called');
+ },
+ get ['a']() {
+ return 'a';
+ }
+};
+
+Assert.assertEquals(obj.a, 'a');
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/es6/computed-property-getter.js Mon Sep 26 13:27:45 2016 +0200
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8164467: ES6 computed properties are implemented wrongly
+ *
+ * @test
+ * @run
+ * @option --language=es6
+ */
+
+function f(x) {
+ return x;
+}
+
+var d = 'd';
+var n = 3;
+var s1 = Symbol();
+var s2 = Symbol();
+
+var object = {
+ get a() { return 'a' },
+ get ['b']() { return 'b' },
+ get [f('c')]() { return 'c' },
+ get [d]() { return d },
+ get [1]() { return 1 },
+ get [f(2)]() { return 2 },
+ get [n]() { return 3 },
+ get [s1]() { return s1 },
+ get [f(s2)]() { return s2 }
+};
+
+
+Assert.assertEquals(object.a, 'a');
+Assert.assertEquals(object.b, 'b');
+Assert.assertEquals(object.c, 'c');
+Assert.assertEquals(object.d, 'd');
+Assert.assertEquals(object[1], 1);
+Assert.assertEquals(object[2], 2);
+Assert.assertEquals(object[3], 3);
+Assert.assertEquals(object[s1], s1);
+Assert.assertEquals(object[s2], s2);
+
+for (var s of ['a', 'b', 'c', 'd', 1, 2, 3, s1, s2]) {
+ Assert.assertEquals(object[s], s);
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/es6/computed-property-method.js Mon Sep 26 13:27:45 2016 +0200
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8164467: ES6 computed properties are implemented wrongly
+ *
+ * @test
+ * @run
+ * @option --language=es6
+ */
+
+function f(x) {
+ return x;
+}
+
+var d = 'd';
+var n = 3;
+var s1 = Symbol();
+var s2 = Symbol();
+
+var object = {
+ a() { return 'a' },
+ ['b']() { return 'b' },
+ [f('c')]() { return 'c' },
+ [d]() { return d },
+ [1]() { return 1 },
+ [f(2)]() { return 2 },
+ [n]() { return 3 },
+ [s1]() { return s1 },
+ [f(s2)]() { return s2 }
+};
+
+
+Assert.assertEquals(object.a(), 'a');
+Assert.assertEquals(object.b(), 'b');
+Assert.assertEquals(object.c(), 'c');
+Assert.assertEquals(object.d(), 'd');
+Assert.assertEquals(object[1](), 1);
+Assert.assertEquals(object[2](), 2);
+Assert.assertEquals(object[3](), 3);
+Assert.assertEquals(object[s1](), s1);
+Assert.assertEquals(object[s2](), s2);
+
+for (var s of ['a', 'b', 'c', 'd', 1, 2, 3, s1, s2]) {
+ Assert.assertEquals(object[s](), s);
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/es6/computed-property-number.js Mon Sep 26 13:27:45 2016 +0200
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8164467: ES6 computed properties are implemented wrongly
+ *
+ * @test
+ * @run
+ * @option --language=es6
+ */
+
+function f(x) {
+ return x;
+}
+
+var object = {
+ [1.5]: 'a',
+ get [f(1e+40)]() { return 'b' },
+ [0.0001]: 'c',
+ [-0]: 'd',
+ [Infinity]: 'e',
+ [-Infinity]: 'f',
+ [NaN]: 'g'
+
+};
+
+
+Assert.assertEquals(object['1.5'], 'a');
+Assert.assertEquals(object['1e+40'], 'b');
+Assert.assertEquals(object['0.0001'], 'c');
+Assert.assertEquals(object[0], 'd');
+Assert.assertEquals(object[Infinity], 'e');
+Assert.assertEquals(object[-Infinity], 'f');
+Assert.assertEquals(object[NaN], 'g');
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/es6/computed-property-setter.js Mon Sep 26 13:27:45 2016 +0200
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8164467: ES6 computed properties are implemented wrongly
+ *
+ * @test
+ * @run
+ * @option --language=es6
+ */
+
+var counter = 0;
+
+var obj = {
+ set ['a'](x) {
+ counter++;
+ }
+};
+
+obj.a = 'a';
+Assert.assertTrue(counter === 1);
+obj.a = 'a';
+Assert.assertTrue(counter === 2);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/es6/computed-property.js Mon Sep 26 13:27:45 2016 +0200
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8164467: ES6 computed properties are implemented wrongly
+ *
+ * @test
+ * @run
+ * @option --language=es6
+ */
+
+function f(x) {
+ return x;
+}
+
+var d = 'd';
+var n = 3;
+var s1 = Symbol();
+var s2 = Symbol();
+
+var object = {
+ a: 'a',
+ ['b']: 'b',
+ [f('c')]: 'c',
+ [d]: d,
+ [1]: 1,
+ [f(2)]: 2,
+ [n]: 3,
+ [s1]: s1,
+ [f(s2)]: s2
+};
+
+
+Assert.assertEquals(object.a, 'a');
+Assert.assertEquals(object.b, 'b');
+Assert.assertEquals(object.c, 'c');
+Assert.assertEquals(object.d, 'd');
+Assert.assertEquals(object[1], 1);
+Assert.assertEquals(object[2], 2);
+Assert.assertEquals(object[3], 3);
+Assert.assertEquals(object[s1], s1);
+Assert.assertEquals(object[s2], s2);
+
+for (var s of ['a', 'b', 'c', 'd', 1, 2, 3, s1, s2]) {
+ Assert.assertEquals(object[s], s);
+}