933932 - OdinMonkey: bad asm.js warning with unsigned form

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[James Long (:jlongster)]

Attachment: 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.

Comment 1

[Luke Wagner [:luke]]

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.

Comment 2

[James Long (:jlongster)]

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.

Comment 3

[James Long (:jlongster)]

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.

Comment 4

[Luke Wagner [:luke]]

Oh no, I think it's a valid bug.  It's on my TODO list :)

Comment 5

[Luke Wagner [:luke]]

Attachment: patch

This text (and its placement) will need to be modified by the float32 patch.

Comment 6

[James Long (:jlongster)]

(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?

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

(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.

Comment 8

[Luke Wagner [:luke]]

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

Comment 9

[Carsten Book [:Tomcat]]

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