877908 - Confusing error message from "use asm" with "use strict"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

js> (function() { "use asm"; "use strict"; function g(){} return g; })

typein:14:0 warning: asm.js type error: asm.js module must end with a return export statement:
typein:14:0 warning: (function() { "use asm"; "use strict"; function g(){} return
typein:14:0 warning: .^

Expected one of the following:
* "use asm" precludes other directives
* unsupported kind of statement in asm.js module

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: proposed fix + test

The asmjs spec is not precise whether the use of "use asm" should preclude other directives, so this patch stays consistent with the rest of the function.

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 756786 [details] [diff] [review]
proposed fix + test

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

Thanks!

::: js/src/ion/AsmJS.cpp
@@ +2574,5 @@
> +    *stmtIter = NextNonEmptyStatement(firstStatement);
> +    if (*stmtIter
> +        && IsExpressionStatement(*stmtIter)
> +        && ExpressionStatementExpr(*stmtIter)->isKind(PNK_STRING))
> +        return m.fail(*stmtIter, "\"use asm\" precludes other directives");

SM style is, when the condition is multi-line, { brace } the then branch, even if it is a single line (with the opening { on a new line).  So,

if (....
    ...)
{
    return ...
}

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: same fix addressing comments

Carrying r+ with fixed nit.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a027c965a33

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0a027c965a33