Bug 907958 - Disallow |function function() {}| and similar unreadabilities. r=jorendorff, r=wingo for the yield interactions, r=luke for the asm.js interactions
authorJeff Walden <jwalden@mit.edu>
Thu, 15 Aug 2013 10:07:40 -0700
changeset 144576 43503c7ca48ae62126e63b15553062ab6577a3fd
parent 144575 2643fd47538b266775a5587cc2e292b18fab88d6
child 144577 5991872b4ce3c83784be41cfb1795b6c942f8a19
push id25168
push useremorley@mozilla.com
push dateWed, 28 Aug 2013 11:21:34 +0000
treeherdermozilla-central@6afb1f453688 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff, wingo, luke
bugs907958
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 907958 - Disallow |function function() {}| and similar unreadabilities. r=jorendorff, r=wingo for the yield interactions, r=luke for the asm.js interactions
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
js/src/jit/AsmJS.cpp
js/src/tests/ecma_5/misc/future-reserved-words.js
js/src/tests/js1_5/LexicalConventions/regress-343675.js
js/src/tests/js1_7/lexical/regress-351515.js
js/src/tests/js1_8_1/regress/regress-452498-117.js
services/common/modules-testing/storageserver.js
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -1637,17 +1637,17 @@ Parser<ParseHandler>::functionArguments(
                         return false;
                     *listp = list;
                 }
                 break;
               }
 #endif /* JS_HAS_DESTRUCTURING */
 
               case TOK_YIELD:
-                if (!checkYieldNameValidity(JSMSG_MISSING_FORMAL))
+                if (!checkYieldNameValidity())
                     return false;
                 goto TOK_NAME;
 
               case TOK_TRIPLEDOT:
               {
                 *hasRest = true;
                 tt = tokenStream.getToken();
                 if (tt != TOK_NAME) {
@@ -1715,45 +1715,28 @@ Parser<ParseHandler>::functionArguments(
 
         if (!hasDefaults)
             funbox->length = pc->numArgs() - *hasRest;
     }
 
     return true;
 }
 
-template <typename ParseHandler>
-bool
-Parser<ParseHandler>::checkFunctionName(HandlePropertyName funName)
-{
-    if (pc->isStarGenerator() && funName == context->names().yield) {
-        // The name of a named function expression is specified to be bound in
-        // the outer context as if via "let".  In an ES6 generator, "yield" is
-        // not a valid name for a let-bound variable.
-        report(ParseError, false, null(), JSMSG_RESERVED_ID, "yield");
-        return false;
-    }
-    return true;
-}
-
 template <>
 bool
 Parser<FullParseHandler>::checkFunctionDefinition(HandlePropertyName funName,
                                                   ParseNode **pn_, FunctionSyntaxKind kind,
                                                   bool *pbodyProcessed)
 {
     ParseNode *&pn = *pn_;
     *pbodyProcessed = false;
 
     /* Function statements add a binding to the enclosing scope. */
     bool bodyLevel = pc->atBodyLevel();
 
-    if (!checkFunctionName(funName))
-        return false;
-
     if (kind == Statement) {
         /*
          * Handle redeclaration and optimize cases where we can statically bind the
          * function (thereby avoiding JSOP_DEFFUN and dynamic name lookup).
          */
         if (Definition *dn = pc->decls().lookupFirst(funName)) {
             JS_ASSERT(!dn->isUsed());
             JS_ASSERT(dn->isDefn());
@@ -1935,19 +1918,16 @@ Parser<SyntaxParseHandler>::checkFunctio
                                                     Node *pn, FunctionSyntaxKind kind,
                                                     bool *pbodyProcessed)
 {
     *pbodyProcessed = false;
 
     /* Function statements add a binding to the enclosing scope. */
     bool bodyLevel = pc->atBodyLevel();
 
-    if (!checkFunctionName(funName))
-        return false;
-
     if (kind == Statement) {
         /*
          * Handle redeclaration and optimize cases where we can statically bind the
          * function (thereby avoiding JSOP_DEFFUN and dynamic name lookup).
          */
         if (DefinitionNode dn = pc->decls().lookupFirst(funName)) {
             if (dn == Definition::CONST) {
                 JSAutoByteString name;
@@ -2414,25 +2394,21 @@ SyntaxParseHandler::Node
 Parser<SyntaxParseHandler>::moduleDecl()
 {
     JS_ALWAYS_FALSE(abortIfSyntaxParser());
     return SyntaxParseHandler::NodeFailure;
 }
 
 template <typename ParseHandler>
 bool
-Parser<ParseHandler>::checkYieldNameValidity(unsigned errorNumber)
-{
-    // In star generators and in JS >= 1.7, yield is a keyword.
-    if (pc->isStarGenerator() || versionNumber() >= JSVERSION_1_7) {
-        report(ParseError, false, null(), errorNumber);
-        return false;
-    }
-    // Otherwise in strict mode, yield is a future reserved word.
-    if (pc->sc->strict) {
+Parser<ParseHandler>::checkYieldNameValidity()
+{
+    // In star generators and in JS >= 1.7, yield is a keyword.  Otherwise in
+    // strict mode, yield is a future reserved word.
+    if (pc->isStarGenerator() || versionNumber() >= JSVERSION_1_7 || pc->sc->strict) {
         report(ParseError, false, null(), JSMSG_RESERVED_ID, "yield");
         return false;
     }
     return true;
 }
 
 template <typename ParseHandler>
 typename ParseHandler::Node
@@ -2440,26 +2416,30 @@ Parser<ParseHandler>::functionStmt()
 {
     JS_ASSERT(tokenStream.isCurrentTokenType(TOK_FUNCTION));
 
     TokenStream::Position start(keepAtoms);
     tokenStream.tell(&start);
 
     RootedPropertyName name(context);
     GeneratorKind generatorKind = NotGenerator;
-    TokenKind tt = tokenStream.getToken(TokenStream::KeywordIsName);
+    TokenKind tt = tokenStream.getToken();
 
     if (tt == TOK_MUL) {
         tokenStream.tell(&start);
-        tt = tokenStream.getToken(TokenStream::KeywordIsName);
+        tt = tokenStream.getToken();
         generatorKind = StarGenerator;
     }
 
     if (tt == TOK_NAME) {
         name = tokenStream.currentName();
+    } else if (tt == TOK_YIELD) {
+        if (!checkYieldNameValidity())
+            return null();
+        name = tokenStream.currentName();
     } else {
         /* Unnamed function expressions are forbidden in statement context. */
         report(ParseError, false, null(), JSMSG_UNNAMED_FUNCTION_STMT);
         return null();
     }
 
     /* We forbid function statements in strict mode code. */
     if (!pc->atBodyLevel() && pc->sc->needStrictChecks() &&
@@ -2473,30 +2453,35 @@ template <typename ParseHandler>
 typename ParseHandler::Node
 Parser<ParseHandler>::functionExpr()
 {
     JS_ASSERT(tokenStream.isCurrentTokenType(TOK_FUNCTION));
 
     TokenStream::Position start(keepAtoms);
     tokenStream.tell(&start);
 
-    RootedPropertyName name(context);
     GeneratorKind generatorKind = NotGenerator;
-    TokenKind tt = tokenStream.getToken(TokenStream::KeywordIsName);
+    TokenKind tt = tokenStream.getToken();
 
     if (tt == TOK_MUL) {
         tokenStream.tell(&start);
-        tt = tokenStream.getToken(TokenStream::KeywordIsName);
+        tt = tokenStream.getToken();
         generatorKind = StarGenerator;
     }
 
-    if (tt == TOK_NAME)
+    RootedPropertyName name(context);
+    if (tt == TOK_NAME) {
         name = tokenStream.currentName();
-    else
+    } else if (tt == TOK_YIELD) {
+        if (!checkYieldNameValidity())
+            return null();
+        name = tokenStream.currentName();
+    } else {
         tokenStream.ungetToken();
+    }
 
     return functionDef(name, start, Normal, Expression, generatorKind);
 }
 
 /*
  * Return true if this node, known to be an unparenthesized string literal,
  * could be the string of a directive in a Directive Prologue. Directive
  * strings never contain escape sequences or line continuations.
@@ -3565,17 +3550,17 @@ Parser<ParseHandler>::variables(ParseNod
                 return null();
             handler.addList(pn, pn2);
             continue;
         }
 #endif /* JS_HAS_DESTRUCTURING */
 
         if (tt != TOK_NAME) {
             if (tt == TOK_YIELD) {
-                if (!checkYieldNameValidity(JSMSG_NO_VARIABLE_NAME))
+                if (!checkYieldNameValidity())
                     return null();
             } else {
                 if (tt != TOK_ERROR)
                     report(ParseError, false, null(), JSMSG_NO_VARIABLE_NAME);
                 return null();
             }
         }
 
@@ -4877,17 +4862,17 @@ Parser<ParseHandler>::tryStatement()
               case TOK_LC:
                 catchName = destructuringExpr(&data, tt);
                 if (!catchName)
                     return null();
                 break;
 #endif
 
               case TOK_YIELD:
-                if (!checkYieldNameValidity(JSMSG_CATCH_IDENTIFIER))
+                if (!checkYieldNameValidity())
                     return null();
                 // Fall through.
               case TOK_NAME:
               {
                 RootedPropertyName label(context, tokenStream.currentName());
                 catchName = newBindingNode(label, false);
                 if (!catchName)
                     return null();
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -453,16 +453,21 @@ class Parser : private AutoGCRooter, pub
      * statements; pass ExpressionBody if the body is a single expression.
      */
     enum FunctionBodyType { StatementListBody, ExpressionBody };
     Node functionBody(FunctionSyntaxKind kind, FunctionBodyType type);
 
     bool functionArgsAndBodyGeneric(Node pn, HandleFunction fun, FunctionType type,
                                     FunctionSyntaxKind kind, Directives *newDirectives);
 
+    // Determine whether |yield| is a valid name in the current context, or
+    // whether it's prohibited due to strictness, JS version, or occurrence
+    // inside a star generator.
+    bool checkYieldNameValidity();
+
     virtual bool strictMode() { return pc->sc->strict; }
 
     const CompileOptions &options() const {
         return tokenStream.options();
     }
 
   private:
     /*
@@ -541,17 +546,16 @@ class Parser : private AutoGCRooter, pub
     Node generatorExpr(Node kid);
     bool argumentList(Node listNode);
     Node letBlock(LetContext letContext);
     Node destructuringExpr(BindData<ParseHandler> *data, TokenKind tt);
 
     Node identifierName();
 
     bool matchLabel(MutableHandle<PropertyName*> label);
-    bool checkYieldNameValidity(unsigned errorNumber = JSMSG_SYNTAX_ERROR);
 
     bool allowsForEachIn() {
 #if !JS_HAS_FOR_EACH_IN
         return false;
 #else
         return versionNumber() >= JSVERSION_1_6;
 #endif
     }
@@ -563,17 +567,16 @@ class Parser : private AutoGCRooter, pub
         IncDecAssignment
     };
 
     bool checkAndMarkAsAssignmentLhs(Node pn, AssignmentFlavor flavor);
     bool matchInOrOf(bool *isForOfp);
 
     bool checkFunctionArguments();
     bool makeDefIntoUse(Definition *dn, Node pn, JSAtom *atom);
-    bool checkFunctionName(HandlePropertyName funName);
     bool checkFunctionDefinition(HandlePropertyName funName, Node *pn, FunctionSyntaxKind kind,
                                  bool *pbodyProcessed);
     bool finishFunctionDefinition(Node pn, FunctionBox *funbox, Node prelude, Node body);
     bool addFreeVariablesFromLazyFunction(JSFunction *fun, ParseContext<ParseHandler> *pc);
 
     bool isValidForStatementLHS(Node pn1, JSVersion version,
                                 bool forDecl, bool forEach, bool forOf);
     bool checkAndMarkAsIncOperand(Node kid, TokenKind tt, bool preorder);
--- a/js/src/jit/AsmJS.cpp
+++ b/js/src/jit/AsmJS.cpp
@@ -4583,20 +4583,28 @@ CheckStatement(FunctionCompiler &f, Pars
 static bool
 ParseFunction(ModuleCompiler &m, ParseNode **fnOut)
 {
     TokenStream &tokenStream = m.parser().tokenStream;
 
     DebugOnly<TokenKind> tk = tokenStream.getToken();
     JS_ASSERT(tk == TOK_FUNCTION);
 
-    if (tokenStream.getToken(TokenStream::KeywordIsName) != TOK_NAME)
-        return false;  // This will throw a SyntaxError, no need to m.fail.
-
-    RootedPropertyName name(m.cx(), tokenStream.currentToken().name());
+    RootedPropertyName name(m.cx());
+
+    TokenKind tt = tokenStream.getToken();
+    if (tt == TOK_NAME) {
+        name = tokenStream.currentName();
+    } else if (tt == TOK_YIELD) {
+        if (!m.parser().checkYieldNameValidity())
+            return false;
+        name = m.cx()->names().yield;
+    } else {
+        return false;  // The regular parser will throw a SyntaxError, no need to m.fail.
+    }
 
     ParseNode *fn = m.parser().handler.newFunctionDefinition();
     if (!fn)
         return false;
 
     // This flows into FunctionBox, so must be tenured.
     RootedFunction fun(m.cx(), NewFunction(m.cx(), NullPtr(), NULL, 0, JSFunction::INTERPRETED,
                                            m.cx()->global(), name, JSFunction::FinalizeKind,
--- a/js/src/tests/ecma_5/misc/future-reserved-words.js
+++ b/js/src/tests/ecma_5/misc/future-reserved-words.js
@@ -32,17 +32,17 @@ var strictFutureReservedWords =
    "package",
    "private",
    "protected",
    "public",
    "static",
    "yield", // enabled: this file doesn't execute as JS1.7
   ];
 
-function testWord(word, wordKind, expectNormal, expectStrict)
+function testWord(word, expectNormal, expectStrict)
 {
   var actual, status;
 
   // USE AS LHS FOR ASSIGNMENT
 
   actual = "";
   status = summary + ": " + word + ": normal assignment";
   try
@@ -350,32 +350,29 @@ function testWord(word, wordKind, expect
   {
     actual = e.name;
     status +=  ", " + e.name + ": " + e.message + " ";
   }
   reportCompare(expectStrict, actual, status);
 
   // USE AS FUNCTION NAME IN FUNCTION DECLARATION
 
-  if (wordKind !== "reserved")
+  actual = "";
+  status = summary + ": " + word + ": normal function name";
+  try
   {
-    actual = "";
-    status = summary + ": " + word + ": normal function name";
-    try
-    {
-      eval("function " + word + "() { }");
-      actual = "no error";
-    }
-    catch (e)
-    {
-      actual = e.name;
-      status +=  ", " + e.name + ": " + e.message + " ";
-    }
-    reportCompare(expectNormal, actual, status);
+    eval("function " + word + "() { }");
+    actual = "no error";
   }
+  catch (e)
+  {
+    actual = e.name;
+    status +=  ", " + e.name + ": " + e.message + " ";
+  }
+  reportCompare(expectNormal, actual, status);
 
   actual = "";
   status = summary + ": " + word + ": strict function name";
   try
   {
     eval("'use strict'; function " + word + "() { }");
     actual = "no error";
   }
@@ -397,32 +394,29 @@ function testWord(word, wordKind, expect
   {
     actual = e.name;
     status +=  ", " + e.name + ": " + e.message + " ";
   }
   reportCompare(expectStrict, actual, status);
 
   // USE AS FUNCTION NAME IN FUNCTION EXPRESSION
 
-  if (wordKind !== "reserved")
+  actual = "";
+  status = summary + ": " + word + ": normal function expression name";
+  try
   {
-    actual = "";
-    status = summary + ": " + word + ": normal function expression name";
-    try
-    {
-      eval("var s = (function " + word + "() { });");
-      actual = "no error";
-    }
-    catch (e)
-    {
-      actual = e.name;
-      status +=  ", " + e.name + ": " + e.message + " ";
-    }
-    reportCompare(expectNormal, actual, status);
+    eval("var s = (function " + word + "() { });");
+    actual = "no error";
   }
+  catch (e)
+  {
+    actual = e.name;
+    status +=  ", " + e.name + ": " + e.message + " ";
+  }
+  reportCompare(expectNormal, actual, status);
 
   actual = "";
   status = summary + ": " + word + ": strict function expression name";
   try
   {
     eval("'use strict'; var s = (function " + word + "() { });");
     actual = "no error";
   }
@@ -445,22 +439,22 @@ function testWord(word, wordKind, expect
     actual = e.name;
     status +=  ", " + e.name + ": " + e.message + " ";
   }
   reportCompare(expectStrict, actual, status);
 }
 
 function testFutureReservedWord(word)
 {
-  testWord(word, "reserved", "SyntaxError", "SyntaxError");
+  testWord(word, "SyntaxError", "SyntaxError");
 }
 
 function testStrictFutureReservedWord(word)
 {
-  testWord(word, "strict reserved", "no error", "SyntaxError");
+  testWord(word, "no error", "SyntaxError");
 }
 
 futureReservedWords.forEach(testFutureReservedWord);
 strictFutureReservedWords.forEach(testStrictFutureReservedWord);
 
 /******************************************************************************/
 
 if (typeof reportCompare === "function")
deleted file mode 100644
--- a/js/src/tests/js1_5/LexicalConventions/regress-343675.js
+++ /dev/null
@@ -1,49 +0,0 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
-/* 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/. */
-
-//-----------------------------------------------------------------------------
-var BUGNUMBER = 343675;
-var summary = 'Allow keywords, reserved words as function names';
-var actual = '';
-var expect = 'No Error';
-
-
-//-----------------------------------------------------------------------------
-test();
-//-----------------------------------------------------------------------------
-
-function test()
-{
-  enterFunc ('test');
-  printBugNumber(BUGNUMBER);
-  printStatus (summary);
- 
-  var words = [
-    'break', 'else', 'new', 'var', 'case', 'finally', 'return', 'void',
-    'catch', 'for', 'switch', 'while', 'continue', 'function', 'this',
-    'with', 'default', 'if', 'throw', 'delete', 'in', 'try', 'do',
-    'instanceof', 'typeof',
-    'abstract', 'enum', 'int', 'short', 'boolean', 'export', 'interface',
-    'static', 'byte', 'extends', 'long', 'super', 'char', 'final', 'native',
-    'synchronized', 'class', 'float', 'package', 'throws', 'const', 'goto',
-    'private', 'transient', 'debugger', 'implements', 'protected', 'volatile',
-    'double', 'import', 'public'];
-
-  for (var i = 0; i < words.length; i++)
-  {
-    try
-    {
-      actual = 'No Error';
-      eval('function ' + words[i] + '() {}');
-    }
-    catch(ex)
-    {
-      actual = ex + '';
-    }
-    reportCompare(expect, actual, summary + ': ' + words[i]);
-  }
-
-  exitFunc ('test');
-}
--- a/js/src/tests/js1_7/lexical/regress-351515.js
+++ b/js/src/tests/js1_7/lexical/regress-351515.js
@@ -88,22 +88,10 @@ function test()
     actual = 'No Error';
   }
   catch(ex)
   {
     actual = ex.name;
   }
   reportCompare(expect, actual, summary + ': function () { var let;}');
 
-  try
-  {
-    expect = 'No Error';
-    function yield() {}
-    actual = 'No Error';
-  }
-  catch(ex)
-  {
-    actual = ex + '';
-  }
-  reportCompare(expect, actual, summary + ': function yield()');
-
   exitFunc ('test');
 }
--- a/js/src/tests/js1_8_1/regress/regress-452498-117.js
+++ b/js/src/tests/js1_8_1/regress/regress-452498-117.js
@@ -31,21 +31,17 @@ function test()
     eval('x; function  x(){}; const x;');
   }
   catch(ex)
   {
   }
 
 // Assertion failure: !pn->isPlaceholder(), at ../jsparse.cpp:4876
 // =====
-  (function(){ var x; eval("var x; x = null"); })()
-
-// Assertion failure: regs.sp == StackBase(fp), at ../jsinterp.cpp:2984
-// =====
-    function this ({x}) { function x(){} }
+  (function(){ var x; eval("var x; x = null"); })();
 
 // Assertion failure: !(pnu->pn_dflags & PND_BOUND), at ../jsemit.cpp:1818
 // =====
   (function(){const x = 0, y = delete x;})()
 
 // Assertion failure: JOF_OPTYPE(op) == JOF_ATOM, at ../jsemit.cpp:1710
 // =====
     try
--- a/services/common/modules-testing/storageserver.js
+++ b/services/common/modules-testing/storageserver.js
@@ -106,17 +106,17 @@ ServerBSO.prototype = {
       if (this[key] !== undefined) {
         obj[key] = this[key];
       }
     }
 
     return obj;
   },
 
-  delete: function delete() {
+  delete: function delete_() {
     this.deleted = true;
 
     delete this.payload;
     delete this.modified;
   },
 
   /**
    * Handler for GET requests for this BSO.
@@ -566,17 +566,17 @@ StorageServerCollection.prototype = {
         this._log.info("Exception when processing BSO: " +
                        CommonUtils.exceptionStr(ex));
         failed[record.id] = "Exception when processing.";
       }
     }
     return {success: success, failed: failed};
   },
 
-  delete: function delete(options) {
+  delete: function delete_(options) {
     options = options || {};
 
     // Protocol 2.0 only allows the "ids" query string argument.
     let keys = Object.keys(options).filter(function(k) {
       return k != "ids";
     });
     if (keys.length) {
       this._log.warn("Invalid query string parameter to collection delete: " +