8156614: Lazy parsing of ES6 shorthand method syntax is broken
authorhannesw
Mon, 20 Jun 2016 12:21:51 +0200
changeset 39077 c549268fe94c
parent 39076 2fb89709d552
child 39078 2f6a9a1373f7
8156614: Lazy parsing of ES6 shorthand method syntax is broken Reviewed-by: sundar, mhaupt
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/Parser.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java
nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunctionData.java
nashorn/test/script/basic/es6/JDK-8156614.js
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/Parser.java	Mon Jun 20 11:44:29 2016 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/Parser.java	Mon Jun 20 12:21:51 2016 +0200
@@ -143,6 +143,7 @@
 import jdk.nashorn.internal.runtime.ParserException;
 import jdk.nashorn.internal.runtime.RecompilableScriptFunctionData;
 import jdk.nashorn.internal.runtime.ScriptEnvironment;
+import jdk.nashorn.internal.runtime.ScriptFunctionData;
 import jdk.nashorn.internal.runtime.ScriptingFunctions;
 import jdk.nashorn.internal.runtime.Source;
 import jdk.nashorn.internal.runtime.Timing;
@@ -157,6 +158,9 @@
 @Logger(name="parser")
 public class Parser extends AbstractParser implements Loggable {
     private static final String ARGUMENTS_NAME = CompilerConstants.ARGUMENTS_VAR.symbolName();
+    private static final String CONSTRUCTOR_NAME = "constructor";
+    private static final String GET_NAME = "get";
+    private static final String SET_NAME = "set";
 
     /** Current env. */
     private final ScriptEnvironment env;
@@ -277,7 +281,7 @@
      * @return function node resulting from successful parse
      */
     public FunctionNode parse() {
-        return parse(PROGRAM.symbolName(), 0, source.getLength(), false);
+        return parse(PROGRAM.symbolName(), 0, source.getLength(), 0);
     }
 
     /**
@@ -298,13 +302,13 @@
      * @param scriptName name for the script, given to the parsed FunctionNode
      * @param startPos start position in source
      * @param len length of parse
-     * @param allowPropertyFunction if true, "get" and "set" are allowed as first tokens of the program, followed by
-     * a property getter or setter function. This is used when reparsing a function that can potentially be defined as a
-     * property getter or setter in an object literal.
+     * @param reparseFlags flags provided by {@link RecompilableScriptFunctionData} as context for
+     * the code being reparsed. This allows us to recognize special forms of functions such
+     * as property getters and setters or instances of ES6 method shorthand in object literals.
      *
      * @return function node resulting from successful parse
      */
-    public FunctionNode parse(final String scriptName, final int startPos, final int len, final boolean allowPropertyFunction) {
+    public FunctionNode parse(final String scriptName, final int startPos, final int len, final int reparseFlags) {
         final boolean isTimingEnabled = env.isTimingEnabled();
         final long t0 = isTimingEnabled ? System.nanoTime() : 0L;
         log.info(this, " begin for '", scriptName, "'");
@@ -317,7 +321,7 @@
 
             scanFirstToken();
             // Begin parse.
-            return program(scriptName, allowPropertyFunction);
+            return program(scriptName, reparseFlags);
         } catch (final Exception e) {
             handleParseException(e);
 
@@ -421,7 +425,7 @@
             final ParserContextBlockNode body = newBlock();
 
             functionDeclarations = new ArrayList<>();
-            sourceElements(false);
+            sourceElements(0);
             addFunctionDeclarations(function);
             functionDeclarations = null;
 
@@ -646,7 +650,7 @@
         // Set up new block. Captures first token.
         final ParserContextBlockNode newBlock = newBlock();
         try {
-            statement(false, false, true, labelledStatement);
+            statement(false, 0, true, labelledStatement);
         } finally {
             restoreBlock(newBlock);
         }
@@ -897,7 +901,7 @@
      *
      * Parse the top level script.
      */
-    private FunctionNode program(final String scriptName, final boolean allowPropertyFunction) {
+    private FunctionNode program(final String scriptName, final int reparseFlags) {
         // Make a pseudo-token for the script holding its start and length.
         final long functionToken = Token.toDesc(FUNCTION, Token.descPosition(Token.withDelimiter(token)), source.getLength());
         final int  functionLine  = line;
@@ -913,7 +917,7 @@
         final ParserContextBlockNode body = newBlock();
 
         functionDeclarations = new ArrayList<>();
-        sourceElements(allowPropertyFunction);
+        sourceElements(reparseFlags);
         addFunctionDeclarations(script);
         functionDeclarations = null;
 
@@ -961,10 +965,10 @@
      *
      * Parse the elements of the script or function.
      */
-    private void sourceElements(final boolean shouldAllowPropertyFunction) {
+    private void sourceElements(final int reparseFlags) {
         List<Node>    directiveStmts        = null;
         boolean       checkDirective        = true;
-        boolean       allowPropertyFunction = shouldAllowPropertyFunction;
+        int           functionFlags          = reparseFlags;
         final boolean oldStrictMode         = isStrictMode;
 
 
@@ -978,8 +982,8 @@
 
                 try {
                     // Get the next element.
-                    statement(true, allowPropertyFunction, false, false);
-                    allowPropertyFunction = false;
+                    statement(true, functionFlags, false, false);
+                    functionFlags = 0;
 
                     // check for directive prologues
                     if (checkDirective) {
@@ -1097,15 +1101,15 @@
      *     GeneratorDeclaration
      */
     private void statement() {
-        statement(false, false, false, false);
+        statement(false, 0, false, false);
     }
 
     /**
      * @param topLevel does this statement occur at the "top level" of a script or a function?
-     * @param allowPropertyFunction allow property "get" and "set" functions?
+     * @param reparseFlags reparse flags to decide whether to allow property "get" and "set" functions or ES6 methods.
      * @param singleStatement are we in a single statement context?
      */
-    private void statement(final boolean topLevel, final boolean allowPropertyFunction, final boolean singleStatement, final boolean labelledStatement) {
+    private void statement(final boolean topLevel, final int reparseFlags, final boolean singleStatement, final boolean labelledStatement) {
         switch (type) {
         case LBRACE:
             block();
@@ -1194,19 +1198,31 @@
                     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();
                     final long propertyToken = token;
                     final int propertyLine = line;
-                    if ("get".equals(ident)) {
+                    if (GET_NAME.equals(ident)) {
                         next();
                         addPropertyFunctionStatement(propertyGetterFunction(propertyToken, propertyLine));
                         return;
-                    } else if ("set".equals(ident)) {
+                    } else if (SET_NAME.equals(ident)) {
                         next();
                         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;
                 }
             }
 
@@ -1338,7 +1354,7 @@
                 final PropertyNode classElement = methodDefinition(isStatic, classHeritage != null, generator);
                 if (classElement.isComputed()) {
                     classElements.add(classElement);
-                } else if (!classElement.isStatic() && classElement.getKeyName().equals("constructor")) {
+                } else if (!classElement.isStatic() && classElement.getKeyName().equals(CONSTRUCTOR_NAME)) {
                     if (constructor == null) {
                         constructor = classElement;
                     } else {
@@ -1407,7 +1423,7 @@
         }
 
         final Block body = new Block(classToken, ctorFinish, Block.IS_BODY, statements);
-        final IdentNode ctorName = className != null ? className : createIdentNode(identToken, ctorFinish, "constructor");
+        final IdentNode ctorName = className != null ? className : createIdentNode(identToken, ctorFinish, CONSTRUCTOR_NAME);
         final ParserContextFunctionNode function = createParserContextFunctionNode(ctorName, classToken, FunctionNode.Kind.NORMAL, classLineNumber, parameters);
         function.setLastToken(lastToken);
 
@@ -1442,16 +1458,16 @@
         int flags = FunctionNode.ES6_IS_METHOD;
         if (!computed) {
             final String name = ((PropertyKey)propertyName).getPropertyName();
-            if (!generator && isIdent && type != LPAREN && name.equals("get")) {
+            if (!generator && isIdent && type != LPAREN && name.equals(GET_NAME)) {
                 final PropertyFunction methodDefinition = propertyGetterFunction(methodToken, methodLine, flags);
                 verifyAllowedMethodName(methodDefinition.key, isStatic, methodDefinition.computed, generator, true);
                 return new PropertyNode(methodToken, finish, methodDefinition.key, null, methodDefinition.functionNode, null, isStatic, methodDefinition.computed);
-            } else if (!generator && isIdent && type != LPAREN && name.equals("set")) {
+            } else if (!generator && isIdent && type != LPAREN && name.equals(SET_NAME)) {
                 final PropertyFunction methodDefinition = propertySetterFunction(methodToken, methodLine, flags);
                 verifyAllowedMethodName(methodDefinition.key, isStatic, methodDefinition.computed, generator, true);
                 return new PropertyNode(methodToken, finish, methodDefinition.key, null, null, methodDefinition.functionNode, isStatic, methodDefinition.computed);
             } else {
-                if (!isStatic && !generator && name.equals("constructor")) {
+                if (!isStatic && !generator && name.equals(CONSTRUCTOR_NAME)) {
                     flags |= FunctionNode.ES6_IS_CLASS_CONSTRUCTOR;
                     if (subclass) {
                         flags |= FunctionNode.ES6_IS_SUBCLASS_CONSTRUCTOR;
@@ -1469,10 +1485,10 @@
      */
     private void verifyAllowedMethodName(final Expression key, final boolean isStatic, final boolean computed, final boolean generator, final boolean accessor) {
         if (!computed) {
-            if (!isStatic && generator && ((PropertyKey) key).getPropertyName().equals("constructor")) {
+            if (!isStatic && generator && ((PropertyKey) key).getPropertyName().equals(CONSTRUCTOR_NAME)) {
                 throw error(AbstractParser.message("generator.constructor"), key.getToken());
             }
-            if (!isStatic && accessor && ((PropertyKey) key).getPropertyName().equals("constructor")) {
+            if (!isStatic && accessor && ((PropertyKey) key).getPropertyName().equals(CONSTRUCTOR_NAME)) {
                 throw error(AbstractParser.message("accessor.constructor"), key.getToken());
             }
             if (isStatic && ((PropertyKey) key).getPropertyName().equals("prototype")) {
@@ -3133,11 +3149,11 @@
                 final long getSetToken = propertyToken;
 
                 switch (ident) {
-                case "get":
+                case GET_NAME:
                     final PropertyFunction getter = propertyGetterFunction(getSetToken, functionLine);
                     return new PropertyNode(propertyToken, finish, getter.key, null, getter.functionNode, null, false, getter.computed);
 
-                case "set":
+                case SET_NAME:
                     final PropertyFunction setter = propertySetterFunction(getSetToken, functionLine);
                     return new PropertyNode(propertyToken, finish, setter.key, null, null, setter.functionNode, false, setter.computed);
                 default:
@@ -4132,7 +4148,7 @@
                     final List<Statement> prevFunctionDecls = functionDeclarations;
                     functionDeclarations = new ArrayList<>();
                     try {
-                        sourceElements(false);
+                        sourceElements(0);
                         addFunctionDeclarations(functionNode);
                     } finally {
                         functionDeclarations = prevFunctionDecls;
@@ -5111,7 +5127,7 @@
                 break;
             default:
                 // StatementListItem
-                statement(true, false, false, false);
+                statement(true, 0, false, false);
                 break;
             }
         }
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java	Mon Jun 20 11:44:29 2016 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java	Mon Jun 20 12:21:51 2016 +0200
@@ -364,6 +364,9 @@
         if (functionNode.getKind() == FunctionNode.Kind.GETTER || functionNode.getKind() == FunctionNode.Kind.SETTER) {
             flags |= IS_PROPERTY_ACCESSOR;
         }
+        if (functionNode.isMethod() || functionNode.isClassConstructor()) {
+            flags |= IS_ES6_METHOD;
+        }
         return flags;
     }
 
@@ -402,7 +405,7 @@
         parser.setReparsedFunction(this);
 
         final FunctionNode program = parser.parse(CompilerConstants.PROGRAM.symbolName(), descPosition,
-                Token.descLength(token), isPropertyAccessor());
+                Token.descLength(token), flags);
         // Parser generates a program AST even if we're recompiling a single function, so when we are only
         // recompiling a single function, extract it from the program.
         return (isProgram() ? program : extractFunctionFromScript(program)).setName(null, functionName);
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunctionData.java	Mon Jun 20 11:44:29 2016 +0200
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/ScriptFunctionData.java	Mon Jun 20 12:21:51 2016 +0200
@@ -93,6 +93,8 @@
     public static final int IS_VARIABLE_ARITY    = 1 << 5;
     /** Is this a object literal property getter or setter? */
     public static final int IS_PROPERTY_ACCESSOR = 1 << 6;
+    /** Is this an ES6 method? */
+    public static final int IS_ES6_METHOD        = 1 << 7;
 
     /** Flag for strict or built-in functions */
     public static final int IS_STRICT_OR_BUILTIN = IS_STRICT | IS_BUILTIN;
@@ -130,10 +132,6 @@
         return (flags & IS_VARIABLE_ARITY) != 0;
     }
 
-    final boolean isPropertyAccessor() {
-        return (flags & IS_PROPERTY_ACCESSOR) != 0;
-    }
-
     /**
      * Used from e.g. Native*$Constructors as an explicit call. TODO - make arity immutable and final
      * @param arity new arity
@@ -148,7 +146,7 @@
     /**
      * Used from nasgen generated code.
      *
-     * @param doc documentation for this function
+     * @param docKey documentation key for this function
      */
     void setDocumentationKey(final String docKey) {
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/es6/JDK-8156614.js	Mon Jun 20 12:21:51 2016 +0200
@@ -0,0 +1,38 @@
+/*
+ * 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-8156614: Lazy parsing of ES6 shorthand method syntax is broken
+ *
+ * @test
+ * @run
+ * @option --language=es6
+ */
+
+var obj = {
+    foo() { return this.baz(this.bar); },
+    bar: 'hello',
+    baz(s) { return s; }
+};
+
+Assert.assertEquals(obj.foo(), obj.bar);