1167025 - Assembler-x86-shared.h(2769) : warning C4805: '|' : unsafe mix of type 'bool' and type 'int' in operation

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Daniel Holbert [:dholbert]]

When building on Windows, we get multiple instances of this warning:
{
18:14:39     INFO -  Warning: C4805 in c:\builds\moz2_slave\m-in-w32-d-0000000000000000000\build\src\js\src\jit\x86-shared\Assembler-x86-shared.h: '|' : unsafe mix of type 'bool' and type 'int' in operation
18:14:39     INFO -  c:\builds\moz2_slave\m-in-w32-d-0000000000000000000\build\src\js\src\jit/x86-shared/Assembler-x86-shared.h(2769) : warning C4805: '|' : unsafe mix of type 'bool' and type 'int' in operation
}

(We get multiple instances because this is a .h file included in several places, presumably.)

Sample build log:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32-debug/1432167577/mozilla-inbound-win32-debug-bm86-build1-build1074.txt.gz


This warning is pointing to the following code:
> 2768     unsigned blendpsMask(bool x, bool y, bool z, bool w) {
> 2769         return x | (y << 1) | (z << 2) | (w << 3);
> 2770     }

And sure enough, "x" is a bool, and it's is getting bitwise-or'ed with a series of ints (bit-shifted bools) -- that's why MSVC is warning.

Dan, if you're sure this code is really doing what you want, could you perhaps do something to promote "x" to be an int (or unsigned) here, so that we don't get this build warning?

(strawman suggestion: "x << 0" might do the trick, and have some nice consistency with the other shifts on the same line)

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

It makes sense to keep booleans as part of this interface, as each value could correspond to "do we take the source value?" (true meaning yes, false meaning "no, take the destination value" -- not sure about source/destination order, but you get the idea :))

As a matter of fact, the only two possible values mean "yes" or "no", so boolean seems more adapted than unsigned, and having "x << 0" makes the most sense.

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: p.patch

As explained in the previous comment.

Comment 3

[Dan Gohman [:sunfish]]

Comment on attachment 8608568 [details] [diff] [review]
p.patch

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

If that fixes the warning, it works for me.

Comment 4

[Pulsebot]

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

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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