Bug 853239 - warn on a statement expression use (r=jorendorff)

--HG--
extra : rebase_source : dcf55a0ac3312f5d8750c9a0ccbdd1968925dbd5

js/src/frontend/BytecodeCompiler.cpp

@@ -268,7 +268,7 @@ frontend::CompileScript(JSContext *cx, HandleObject scopeChain,
         TokenStream::Position pos(parser.keepAtoms);
         parser.tokenStream.tell(&pos);
 
-        ParseNode *pn = parser.statement();
+        ParseNode *pn = parser.statement(canHaveDirectives);
         if (!pn) {
             if (parser.hadAbortedSyntaxParse()) {
                 // Parsing inner functions lazily may lead the parser into an

js/src/frontend/Parser.cpp

@@ -2521,7 +2521,7 @@ Parser<ParseHandler>::statements()
             }
             break;
         }
-        Node next = statement();
+        Node next = statement(canHaveDirectives);
         if (!next) {
             if (tokenStream.isEOF())
                 tokenStream.setUnexpectedEOF();
@@ -4501,7 +4501,7 @@ Parser<ParseHandler>::expressionStatement()
 
 template <typename ParseHandler>
 typename ParseHandler::Node
-Parser<ParseHandler>::statement()
+Parser<ParseHandler>::statement(bool canHaveDirectives)
 {
     Node pn;
 
@@ -4781,7 +4781,14 @@ Parser<ParseHandler>::statement()
       case TOK_ERROR:
         return null();
 
-      case TOK_NAME: {
+      case TOK_STRING:
+        if (!canHaveDirectives && tokenStream.currentToken().atom() == context->names().useAsm) {
+            if (!report(ParseWarning, false, null(), JSMSG_USE_ASM_DIRECTIVE_FAIL))
+                return null();
+        }
+        return expressionStatement();
+
+      case TOK_NAME:
         if (tokenStream.peekToken() == TOK_COLON)
             return labeledStatement();
         if (tokenStream.currentToken().name() == context->names().module
@@ -4789,8 +4796,7 @@ Parser<ParseHandler>::statement()
         {
             return moduleDecl();
         }
-      }
-        /* FALL THROUGH */
+        return expressionStatement();
 
       default:
         return expressionStatement();

js/src/frontend/Parser.h

@@ -372,7 +372,7 @@ struct Parser : private AutoGCRooter, public StrictModeGetter
   public:
 
     /* Public entry points for parsing. */
-    Node statement();
+    Node statement(bool canHaveDirectives = false);
     bool maybeParseDirective(Node pn, bool *cont);
 
     // Parse a function, given only its body. Used for the Function constructor.

js/src/jit-test/lib/asm.js

@@ -4,6 +4,7 @@
 
 const ASM_OK_STRING = "successfully compiled asm.js code";
 const ASM_TYPE_FAIL_STRING = "asm.js type error:";
+const ASM_DIRECTIVE_FAIL_STRING = "\"use asm\" is only meaningful in the Directive Prologue of a function body";
 
 const USE_ASM = "'use asm';";
 const HEAP_IMPORTS = "var i8=new glob.Int8Array(b);var u8=new glob.Uint8Array(b);"+
@@ -19,6 +20,31 @@ function asmCompile()
     return f;
 }
 
+function assertAsmDirectiveFail(str)
+{
+    if (!isAsmJSCompilationAvailable())
+        return;
+
+    // Turn on warnings-as-errors
+    var oldOpts = options("werror");
+    assertEq(oldOpts.indexOf("werror"), -1);
+
+    // Verify an error is thrown
+    var caught = false;
+    try {
+        eval(str);
+    } catch (e) {
+        if ((''+e).indexOf(ASM_DIRECTIVE_FAIL_STRING) == -1)
+            throw new Error("Didn't catch the expected directive failure error; instead caught: " + e);
+        caught = true;
+    }
+    if (!caught)
+        throw new Error("Didn't catch the directive failure error");
+
+    // Turn warnings-as-errors back off
+    options("werror");
+}
+
 function assertAsmTypeFail()
 {
     if (!isAsmJSCompilationAvailable())

js/src/jit-test/tests/asm.js/testUseAsmWarnings.js

@@ -0,0 +1,14 @@
+load(libdir + "asm.js");
+
+assertAsmDirectiveFail("'use asm'");
+assertAsmDirectiveFail("eval('\"use asm\";');");
+assertAsmDirectiveFail("{ eval('\"use asm\";'); }");
+assertAsmDirectiveFail("if (Math) { 'use asm'; }");
+assertAsmDirectiveFail("function f(){ { 'use asm'; } }");
+assertAsmDirectiveFail("function f(){ ; 'use asm'; } }");
+assertAsmDirectiveFail("function f(){ 1; 'use asm'; } }");
+assertAsmDirectiveFail("function f(){ var x; 'use asm'; } }");
+assertAsmDirectiveFail("function f(){ if (Math) { 'use asm'; } }");
+assertAsmDirectiveFail("(function(){ eval('\"use asm\";') })()");
+assertAsmDirectiveFail("new Function('{\"use asm\";}')");
+assertAsmDirectiveFail("new Function('if (Math){\"use asm\";}')");

js/src/js.msg

@@ -391,7 +391,7 @@ MSG_DEF(JSMSG_DATE_NOT_FINITE,        337, 0, JSEXN_RANGEERR, "date value is not
 MSG_DEF(JSMSG_MODULE_STATEMENT,       338, 0, JSEXN_SYNTAXERR, "module declarations may only appear at the top level of a program or module body")
 MSG_DEF(JSMSG_CURLY_BEFORE_MODULE,    339, 0, JSEXN_SYNTAXERR, "missing { before module body")
 MSG_DEF(JSMSG_CURLY_AFTER_MODULE,     340, 0, JSEXN_SYNTAXERR, "missing } after module body")
-MSG_DEF(JSMSG_USE_ASM_DIRECTIVE_FAIL, 341, 0, JSEXN_SYNTAXERR, "'use asm' directive only works on function code")
+MSG_DEF(JSMSG_USE_ASM_DIRECTIVE_FAIL, 341, 0, JSEXN_SYNTAXERR, "\"use asm\" is only meaningful in the Directive Prologue of a function body")
 MSG_DEF(JSMSG_USE_ASM_TYPE_FAIL,      342, 1, JSEXN_TYPEERR, "asm.js type error: {0}")
 MSG_DEF(JSMSG_USE_ASM_LINK_FAIL,      343, 1, JSEXN_TYPEERR, "asm.js link error: {0}")
 MSG_DEF(JSMSG_USE_ASM_TYPE_OK,        344, 1, JSEXN_ERR,     "successfully compiled asm.js code ({0})")