OdinMonkey: bad asm.js warning with unsigned form

RESOLVED FIXED in mozilla28

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jlong, Assigned: luke)

Tracking

unspecified
mozilla28
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 826081 [details]
foo.js

I'm probably just being pedantic here, but it took me a while to tackle a bug in LLJS because of this. If you run the attached code under OdinMonkey, you get the following asm.js error:

foo.js:11:9 warning: asm.js type error: non-expression-statement call must be coerced

The problem is the `foo(1) >>> 0` coercion. If you remove the coercion, it works (because foo's return type is void). If you try `foo(1) | 0` instead, you get a proper error messag: "signed incompatible with previous return of type void".

I think I'm hitting some edge cases with LLJS that aren't usually seen, so it's not a huge deal.
(Assignee)

Comment 1

5 years ago
Mm, thanks for reporting this (and updating LLJS)!  It's a bit hard to give a good error message because *any* use of a call in any expression other than f()|0 and +f() is an error, so f()>>>0 isn't special.  Perhaps we could just change the error message message from "non-expression-statement call must be coerced" to explain this better.
(Reporter)

Comment 2

5 years ago
Yeah, maybe "coerced as signed or double".

Although, LLJS treats the unsigned type as first-class and in asm.js it's really second-class, but I wouldn't be surprised if the unsigned treatment is confusing to others over time as well.
(Reporter)

Comment 3

5 years ago
I hate letting bugs like this linger, so I'll just close it and if you think there's anything of worth here you can reopen it.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
(Assignee)

Comment 4

5 years ago
Oh no, I think it's a valid bug.  It's on my TODO list :)
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
(Assignee)

Comment 5

5 years ago
Created attachment 832491 [details] [diff] [review]
patch

This text (and its placement) will need to be modified by the float32 patch.
Assignee: nobody → luke
Status: REOPENED → ASSIGNED
Attachment #832491 - Flags: review?(benj)
Attachment #832491 - Flags: review?(benj) → review+
(Reporter)

Comment 6

5 years ago
(In reply to Luke Wagner [:luke] from comment #5)
> Created attachment 832491 [details] [diff] [review]
> patch
> 
> This text (and its placement) will need to be modified by the float32 patch.

This bug probably isn't the right place for this, but are we getting 32-bit floats somehow?
(In reply to James Long (:jlongster) from comment #6)
> This bug probably isn't the right place for this, but are we getting 32-bit
> floats somehow?

Yes! It's a work in progress though, see also bug 904918 to follow its completion.
https://hg.mozilla.org/mozilla-central/rev/a3ace05661dc
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.