853239 - warn on a statement expression "use asm"; not in a processing directive

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

The idea of "use asm" is to provide good developer feedback.  However, if their mistake is not putting "use asm" in the processing directives region according to the grammar, we won't say anything.  It seems friendly to issue a warning for statement expressions of the form "use asm"; in non-process-directive position.

Comment 1

[Nicholas Nethercote [inactive]]

Yeah.  I hit this with an example like this:

let asmModule = Function("g2", "foreign", "heap2", "{\
    'use asm';\
    let HEAP8 = new g2.Int8Array(heap2);\
    function f() { return 99 }\
    return {f: f};\
}")

The braces were the problem.

Comment 2

[Luke Wagner [:luke]]

Attachment: patch

This patch warns for any expression statement "use asm"; not in the Directive Prologue of a function.  I would've liked to avoid passing canHaveDirectives to statement(); the annoying thing is that maybeParseDirective is called *after* we've seen the TOK_STRING in statement().

I also took the liberty of converting the fallthrough case of TOK_NAME to 'return expressionStatement' so it can be symmetric with the TOK_STRING case and easier to read.

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 744375 [details] [diff] [review]
patch

Review of attachment 744375 [details] [diff] [review]:
-----------------------------------------------------------------

Great tests.

::: js/src/frontend/Parser.cpp
@@ +4468,5 @@
> +      case TOK_STRING:
> +        if (!canHaveDirectives && tokenStream.currentToken().atom() == context->names().useAsm) {
> +            if (!report(ParseWarning, false, null(), JSMSG_USE_ASM_DIRECTIVE_FAIL))
> +                return null();
> +        }

OK, this is a fine place for this. Maybe we should move all directive processing here, someday.

Comment 4

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a45f1dcd603c

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a45f1dcd603c