8164467: ES6 computed properties are implemented wrongly
authorhannesw
Mon, 26 Sep 2016 13:27:45 +0200
changeset 41145 b9a1cb9ed1d3
parent 41144 44c67c27002d
child 41146 4de500f4c6bd
8164467: ES6 computed properties are implemented wrongly Reviewed-by: sundar, lagergren
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/PropertyNode.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/Parser.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptObject.java
nashorn/test/script/basic/es6/computed-property-duplicate.js
nashorn/test/script/basic/es6/computed-property-getter.js
nashorn/test/script/basic/es6/computed-property-method.js
nashorn/test/script/basic/es6/computed-property-number.js
nashorn/test/script/basic/es6/computed-property-setter.js
nashorn/test/script/basic/es6/computed-property.js
--- 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);
+}