Bug 1416337 - Limit the function expression-closure extension to apply only to function expressions that constitute an entire AssignmentExpression, so that the next token-get after the function expression closure can safely use Operand. r=arai
authorJeff Walden <jwalden@mit.edu>
Thu, 23 Nov 2017 12:52:41 -0500
changeset 438043 8dd56cab66311abfde3a2465ac1d0182934011ee
parent 438042 00a7aef6d3b22aa15582ffc3e81c0f47c9ada566
child 438044 c6f9187b0b2e9c42f5eeca898bf81640174573fe
push id117
push userfmarier@mozilla.com
push dateTue, 28 Nov 2017 20:17:16 +0000
reviewersarai
bugs1416337
milestone59.0a1
Bug 1416337 - Limit the function expression-closure extension to apply only to function expressions that constitute an entire AssignmentExpression, so that the next token-get after the function expression closure can safely use Operand. r=arai
js/src/frontend/FullParseHandler.h
js/src/frontend/Parser.cpp
js/src/frontend/SyntaxParseHandler.h
js/src/tests/ecma_6/extensions/arrow-as-end-of-expression-closure.js
js/src/tests/js1_8_5/extensions/expression-closure-syntax.js
--- a/js/src/frontend/FullParseHandler.h
+++ b/js/src/frontend/FullParseHandler.h
@@ -629,16 +629,22 @@ class FullParseHandler
     ParseNode* newFunctionExpression(const TokenPos& pos) {
         return new_<CodeNode>(PNK_FUNCTION, JSOP_LAMBDA, pos);
     }
 
     ParseNode* newArrowFunction(const TokenPos& pos) {
         return new_<CodeNode>(PNK_FUNCTION, JSOP_LAMBDA_ARROW, pos);
     }
 
+    bool isExpressionClosure(ParseNode* node) const {
+        return node->isKind(PNK_FUNCTION) &&
+               node->pn_funbox->isExprBody() &&
+               !node->pn_funbox->isArrow();
+    }
+
     void setFunctionFormalParametersAndBody(ParseNode* pn, ParseNode* kid) {
         MOZ_ASSERT_IF(kid, kid->isKind(PNK_PARAMSBODY));
         pn->pn_body = kid;
     }
     void setFunctionBox(ParseNode* pn, FunctionBox* funbox) {
         MOZ_ASSERT(pn->isKind(PNK_FUNCTION));
         pn->pn_funbox = funbox;
         funbox->functionNode = pn;
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -3718,17 +3718,17 @@ Parser<ParseHandler, CharT>::functionFor
     TokenKind tt;
     if (!tokenStream.getToken(&tt, TokenStream::Operand))
         return false;
     uint32_t openedPos = 0;
     if (tt != TOK_LC) {
         if (kind != Arrow) {
             if (funbox->isGenerator() || funbox->isAsync() || kind == Method ||
                 kind == GetterNoExpressionClosure || kind == SetterNoExpressionClosure ||
-                IsConstructorKind(kind))
+                IsConstructorKind(kind) || kind == PrimaryExpression)
             {
                 error(JSMSG_CURLY_BEFORE_BODY);
                 return false;
             }
 
 #if JS_HAS_EXPR_CLOSURES
             addTelemetry(DeprecatedLanguageExtension::ExpressionClosure);
             if (!warnOnceAboutExprClosure())
@@ -3945,19 +3945,19 @@ Parser<ParseHandler, CharT>::functionExp
 
     Node pn = handler.newFunctionExpression(pos());
     if (!pn)
         return null();
 
     if (invoked)
         pn = handler.setLikelyIIFE(pn);
 
-    // This is PrimaryExpression for now because the parser hasn't been changed
-    // to use AssignmentExpression yet.
-    FunctionSyntaxKind kind = PrimaryExpression;
+    FunctionSyntaxKind kind = expressionClosureHandling == ExpressionClosure::Allowed
+                              ? AssignmentExpression
+                              : PrimaryExpression;
 
     return functionDefinition(pn, toStringStart, InAllowed, yieldHandling, name, kind,
                               generatorKind, asyncKind);
 }
 
 /*
  * Return true if this node, known to be an unparenthesized string literal,
  * could be the string of a directive in a Directive Prologue. Directive
@@ -7861,16 +7861,19 @@ Parser<ParseHandler, CharT>::orExpr(InHa
     int depth = 0;
     Node pn;
     for (;;) {
         pn = unaryExpr(yieldHandling, tripledotHandling, expressionClosureHandling, possibleError,
                        invoked);
         if (!pn)
             return null();
 
+        if (handler.isExpressionClosure(pn))
+            return pn;
+
         expressionClosureHandling = ExpressionClosure::Forbidden;
 
         // If a binary operator follows, consume it and compute the
         // corresponding operator.
         TokenKind tok;
         if (!tokenStream.getToken(&tok))
             return null();
 
@@ -7936,16 +7939,19 @@ Parser<ParseHandler, CharT>::condExpr(In
                                       PossibleError* possibleError,
                                       InvokedPrediction invoked /* = PredictUninvoked */)
 {
     Node condition = orExpr(inHandling, yieldHandling, tripledotHandling,
                             expressionClosureHandling, possibleError, invoked);
     if (!condition)
         return null();
 
+    if (handler.isExpressionClosure(condition))
+        return condition;
+
     bool matched;
     if (!tokenStream.matchToken(&matched, TOK_HOOK))
         return null();
     if (!matched)
         return condition;
 
     Node thenExpr = assignExpr(InAllowed, yieldHandling, TripledotProhibited);
     if (!thenExpr)
@@ -8365,16 +8371,19 @@ Parser<ParseHandler, CharT>::unaryExpr(Y
         MOZ_FALLTHROUGH;
 
       default: {
         Node expr = memberExpr(yieldHandling, tripledotHandling, expressionClosureHandling, tt,
                                /* allowCallSyntax = */ true, possibleError, invoked);
         if (!expr)
             return null();
 
+        if (handler.isExpressionClosure(expr))
+            return expr;
+
         /* Don't look across a newline boundary for a postfix incop. */
         if (!tokenStream.peekTokenSameLine(&tt))
             return null();
 
         if (tt != TOK_INC && tt != TOK_DEC)
             return expr;
 
         tokenStream.consumeKnownToken(tt);
@@ -8529,16 +8538,19 @@ Parser<ParseHandler, CharT>::memberExpr(
         lhs = handler.newSuperBase(thisName, pos());
         if (!lhs)
             return null();
     } else {
         lhs = primaryExpr(yieldHandling, tripledotHandling, expressionClosureHandling, tt,
                           possibleError, invoked);
         if (!lhs)
             return null();
+
+        if (handler.isExpressionClosure(lhs))
+            return lhs;
     }
 
     MOZ_ASSERT_IF(handler.isSuperBase(lhs), tokenStream.isCurrentTokenType(TOK_SUPER));
 
     while (true) {
         if (!tokenStream.getToken(&tt))
             return null();
         if (tt == TOK_EOF)
--- a/js/src/frontend/SyntaxParseHandler.h
+++ b/js/src/frontend/SyntaxParseHandler.h
@@ -45,17 +45,26 @@ class SyntaxParseHandler
         NodeReturn,
         NodeBreak,
         NodeThrow,
         NodeEmptyStatement,
 
         NodeVarDeclaration,
         NodeLexicalDeclaration,
 
-        NodeFunctionDefinition,
+        // A non-arrow function expression with block body, from bog-standard
+        // ECMAScript.
+        NodeFunctionExpressionBlockBody,
+
+        // A non-arrow function expression with AssignmentExpression body -- a
+        // proprietary SpiderMonkey extension.
+        NodeFunctionExpressionClosure,
+
+        NodeFunctionArrow,
+        NodeFunctionStatement,
 
         // This is needed for proper assignment-target handling.  ES6 formally
         // requires function calls *not* pass IsValidSimpleAssignmentTarget,
         // but at last check there were still sites with |f() = 5| and similar
         // in code not actually executed (or at least not executed enough to be
         // noticed).
         NodeFunctionCall,
 
@@ -114,16 +123,20 @@ class SyntaxParseHandler
         // We want to reject |-2 ** 3|, but still need to allow |(-2) ** 3|.
         NodeUnparenthesizedUnary,
 
         // This node is necessary to determine if the LHS of a property access is
         // super related.
         NodeSuperBase
     };
 
+    bool isNonArrowFunctionExpression(Node node) const {
+        return node == NodeFunctionExpressionBlockBody || node == NodeFunctionExpressionClosure;
+    }
+
     bool isPropertyAccess(Node node) {
         return node == NodeDottedProperty || node == NodeElement;
     }
 
     bool isFunctionCall(Node node) {
         // Note: super() is a special form, *not* a function call.
         return node == NodeFunctionCall;
     }
@@ -314,19 +327,30 @@ class SyntaxParseHandler
 
     MOZ_MUST_USE bool addCatchBlock(Node catchList, Node letBlock, Node catchName,
                                     Node catchGuard, Node catchBody) { return true; }
 
     MOZ_MUST_USE bool setLastFunctionFormalParameterDefault(Node funcpn, Node pn) { return true; }
 
     void checkAndSetIsDirectRHSAnonFunction(Node pn) {}
 
-    Node newFunctionStatement(const TokenPos& pos) { return NodeFunctionDefinition; }
-    Node newFunctionExpression(const TokenPos& pos) { return NodeFunctionDefinition; }
-    Node newArrowFunction(const TokenPos& pos) { return NodeFunctionDefinition; }
+    Node newFunctionStatement(const TokenPos& pos) { return NodeFunctionStatement; }
+
+    Node newFunctionExpression(const TokenPos& pos) {
+        // All non-arrow function expressions are initially presumed to have
+        // block body.  This will be overridden later *if* the function
+        // expression permissibly has an AssignmentExpression body.
+        return NodeFunctionExpressionBlockBody;
+    }
+
+    Node newArrowFunction(const TokenPos& pos) { return NodeFunctionArrow; }
+
+    bool isExpressionClosure(Node node) const {
+        return node == NodeFunctionExpressionClosure;
+    }
 
     void setFunctionFormalParametersAndBody(Node pn, Node kid) {}
     void setFunctionBody(Node pn, Node kid) {}
     void setFunctionBox(Node pn, FunctionBox* funbox) {}
     void addFunctionFormalParameter(Node pn, Node argpn) {}
 
     Node newForStatement(uint32_t begin, Node forHead, Node body, unsigned iflags) {
         return NodeGeneric;
@@ -415,17 +439,18 @@ class SyntaxParseHandler
         return node == NodeUnparenthesizedUnary;
     }
 
     bool isReturnStatement(Node node) {
         return node == NodeReturn;
     }
 
     bool isStatementPermittedAfterReturnStatement(Node pn) {
-        return pn == NodeFunctionDefinition || pn == NodeVarDeclaration ||
+        return pn == NodeFunctionStatement || isNonArrowFunctionExpression(pn) ||
+               pn == NodeVarDeclaration ||
                pn == NodeBreak ||
                pn == NodeThrow ||
                pn == NodeEmptyStatement;
     }
 
     bool isSuperBase(Node pn) {
         return pn == NodeSuperBase;
     }
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/extensions/arrow-as-end-of-expression-closure.js
@@ -0,0 +1,41 @@
+// |reftest| skip-if(!xulRuntime.shell) -- needs getBuildConfiguration
+/* 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/. */
+
+// ArrowFunctions with block bodies appearing at the end of the
+// AssignmentExpression returned by SpiderMonkey-specific function expression
+// closures, where subsequent token-examination must use the Operand modifier
+// to avoid an assertion.
+
+if (getBuildConfiguration().release_or_beta)
+{
+  eval(`
+  var ec1 = function() 0 ? 1 : a => {};
+  assertEq(typeof ec1, "function");
+  assertEq(typeof ec1(), "function");
+  assertEq(ec1()(), undefined);
+
+  var ec2 = function() 0 ? 1 : a => {} // deliberately exercise ASI here
+  assertEq(typeof ec2, "function");
+  assertEq(typeof ec2(), "function");
+  assertEq(ec2()(), undefined);
+
+  function ec3() 0 ? 1 : a => {} // exercise ASI here
+  assertEq(typeof ec3(), "function");
+
+  function ec4() 0 ? 1 : a => {};
+  assertEq(typeof ec4(), "function");
+
+  var needle = "@";
+  var x = 42;
+  var g = { test() { assertEq(true, false, "shouldn't be called"); } };
+
+  function ec5() 0 ? 1 : a => {} // ASI
+  /x/g.test((needle = "x"));
+  assertEq(needle, "x");
+  `);
+}
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
new file mode 100644
--- /dev/null
+++ b/js/src/tests/js1_8_5/extensions/expression-closure-syntax.js
@@ -0,0 +1,64 @@
+// |reftest| skip-if(!xulRuntime.shell) -- needs getBuildConfiguration
+// Any copyright is dedicated to the Public Domain.
+// http://creativecommons.org/licenses/publicdomain/
+
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 1416337;
+var summary =
+  "Expression closure syntax is only permitted for functions that constitute " +
+  "entire AssignmentExpressions, not PrimaryExpressions that are themselves " +
+  "components of larger binary expressions";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+if (getBuildConfiguration().release_or_beta)
+{
+  function assertThrowsSyntaxError(code)
+  {
+    function testOne(replacement)
+    {
+      var x, rv;
+      try
+      {
+        rv = eval(code.replace("@@@", replacement));
+      }
+      catch (e)
+      {
+        assertEq(e instanceof SyntaxError, true,
+                 "should have thrown a SyntaxError, instead got: " + e);
+        return;
+      }
+
+      assertEq(true, false, "should have thrown, instead returned " + rv);
+    }
+
+    testOne("function");
+    testOne("async function");
+  }
+
+  assertThrowsSyntaxError("x = ++@@@() 1");
+  assertThrowsSyntaxError("x = delete @@@() 1");
+  assertThrowsSyntaxError("x = new @@@() 1");
+  assertThrowsSyntaxError("x = void @@@() 1");
+  assertThrowsSyntaxError("x = +@@@() 1");
+  assertThrowsSyntaxError("x = 1 + @@@() 1");
+  assertThrowsSyntaxError("x = null != @@@() 1");
+  assertThrowsSyntaxError("x = null != @@@() 0 ? 1 : a => {}");
+  assertThrowsSyntaxError("x = @@@() 0 ? 1 : a => {} !== null");
+  assertThrowsSyntaxError("x = @@@() 0 ? 1 : a => {}.toString");
+  assertThrowsSyntaxError("x = @@@() 0 ? 1 : a => {}['toString']");
+  assertThrowsSyntaxError("x = @@@() 0 ? 1 : a => {}``");
+  assertThrowsSyntaxError("x = @@@() 0 ? 1 : a => {}()");
+  assertThrowsSyntaxError("x = @@@() 0 ? 1 : a => {}++");
+  assertThrowsSyntaxError("x = @@@() 0 ? 1 : a => {} || 0");
+  assertThrowsSyntaxError("x = 0 || @@@() 0 ? 1 : a => {}");
+  assertThrowsSyntaxError("x = @@@() 0 ? 1 : a => {} && true");
+  assertThrowsSyntaxError("x = true && @@@() 0 ? 1 : a => {}");
+}
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);