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

RESOLVED FIXED in Firefox 41

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dholbert, Assigned: bbouvier)

Tracking

(Blocks: 1 bug)

unspecified
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

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

Updated

4 years ago
Flags: needinfo?(sunfish)
(Assignee)

Comment 1

4 years ago
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.
Flags: needinfo?(sunfish)
(Assignee)

Comment 2

4 years ago
Created attachment 8608568 [details] [diff] [review]
p.patch

As explained in the previous comment.
Assignee: nobody → benj
Status: NEW → ASSIGNED
Attachment #8608568 - Flags: review?(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.
Attachment #8608568 - Flags: review?(sunfish) → review+
https://hg.mozilla.org/mozilla-central/rev/b5836f1b10bf
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.