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

RESOLVED FIXED in mozilla24

Status

()

Core
JavaScript Engine
--
minor
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: bbouvier)

Tracking

({testcase})

Trunk
mozilla24
x86_64
Mac OS X
testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
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
(Assignee)

Comment 1

4 years ago
Created attachment 756786 [details] [diff] [review]
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.
Assignee: general → bbouvier
Status: NEW → ASSIGNED
Attachment #756786 - Flags: review?(luke)
(Reporter)

Updated

4 years ago
Summary: Confusing error message from "use asm" → Confusing error message from "use asm" with "use strict"

Comment 2

4 years ago
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 ...
}
Attachment #756786 - Flags: review?(luke) → review+
(Assignee)

Comment 3

4 years ago
Created attachment 759393 [details] [diff] [review]
same fix addressing comments

Carrying r+ with fixed nit.
Attachment #756786 - Attachment is obsolete: true
Attachment #759393 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Whiteboard: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a027c965a33
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0a027c965a33
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.