Closed Bug 853239 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: luke, Assigned: luke)

References

Details

Attachments

(1 file)

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.
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.
Blocks: 854599
Attached patch patchSplinter Review
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.
Attachment #744375 - Flags: review?(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.
Attachment #744375 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/mozilla-central/rev/a45f1dcd603c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.