Saltarelle / SaltarelleCompiler

C# to JavaScript compiler – Now http://bridge.net
http://saltarelle-compiler.com
Other
297 stars 74 forks source link

Too many brackets removed from expression #409

Closed sdarnell closed 9 years ago

sdarnell commented 9 years ago

There's some code which generates the wrong value due to order/lack of brackets, here's the original code:

// a is a byte[] containing 254,255,255,255
// purpose is to decode an unsigned int from 4 bytes 
uint v = (uint)( ((uint)a[0] & 0xff) | (((uint)a[1] & 0xff) << 8) | (((uint)a[2] & 0xff) << 16) | (((uint)a[3] & 0xff) << 24) >> 0);

The resulting JS is:

var v = cb[0] & 255 | (cb[1] & 255) << 8 | (cb[2] & 255) << 16 | (cb[3] & 255) << 24 >>> 0;
// This produces -2
var v = (cb[0] & 255 | (cb[1] & 255) << 8 | (cb[2] & 255) << 16 | (cb[3] & 255) << 24) >>> 0;
// Correctly produces 4294967294

I have a temporary work-around which forces the brackets:

uint v = (uint)(( ((uint)a[0] & 0xff) | (((uint)a[1] & 0xff) << 8) | (((uint)a[2] & 0xff) << 16) | (((uint)a[3] & 0xff) << 24) - 0) >> 0);
erik-kallen commented 9 years ago

The problem is that | has a higher priority than << so only the last expression ((((uint)a[3] & 0xff) << 24) will have the right shift applied to it.

Another solution for you is to add another set of parentheses:

uint v = (uint)( ((uint)a[0] & 0xff) | (((uint)a[1] & 0xff) << 8) | (((uint)a[2] & 0xff) << 16) | (((uint)a[3] & 0xff) << 24) >> 0);

which does work.

The solution probably has to be to implement an int to uint conversion that handles negative values correctly. (related to #335)

sdarnell commented 9 years ago

Oh dear I'll have to turn up the volume on my glasses ;) I was convinced that there where brackets around all of the ORs before the >> 0, but there aren't. As you say, the ideal would be for a (uint) cast from int to do a >>> 0, but I appreciate that is part of #335.

So I'm more than happy for this issue to be closed if you'd prefer.