Bug 135205 – [Win] Lots of compiler warnings on Windows

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Brent Fulgham]

I see a lot of compiler warnings from JavaScriptCore when building on Windows:

LLIntOffsetsExtractor, JavaScriptCore: Configuration: Debug x64
7>c:\projects\webkit\opensource\webkitbuild\debug\include\private\javascriptcore\MacroAssemblerX86_64.h(206): warning C4307: '*' : integral constant overflow
7>c:\projects\webkit\opensource\webkitbuild\debug\include\private\javascriptcore\MacroAssemblerX86_64.h(210): warning C4307: '*' : integral constant overflow

JavaScriptCore: Configuration: Debug x64
9>c:\projects\webkit\opensource\source\javascriptcore\dfg\DFGEdge.h(208): warning C4805: '|' : unsafe mix of type 'uintptr_t' and type 'bool' in operation (..\dfg\DFGAbstractValue.cpp)
9>c:\projects\webkit\opensource\source\javascriptcore\dfg\DFGCommon.h(251): warning C4805: '==' : unsafe mix of type 'bool' and type 'int' in operation (..\dfg\DFGAbstractValue.cpp)
9>          c:\projects\webkit\opensource\source\javascriptcore\dfg\DFGVariableAccessData.h(74) : see reference to function template instantiation 'bool JSC::DFG::checkAndSet<bool,int>(T &,U)' being compiled
9>          with
9>          [
9>              T=bool
9>  ,            U=int
9>          ]
9>c:\projects\webkit\opensource\source\javascriptcore\dfg\dfgnodeflags.h(112): warning C4701: potentially uninitialized local variable 'mask' used
9>c:\projects\webkit\opensource\source\javascriptcore\dfg\dfgnodeflags.h(129): warning C4701: potentially uninitialized local variable 'mask' used
9>c:\projects\webkit\opensource\source\javascriptcore\runtime\jsdataviewprototype.cpp(136): warning C4701: potentially uninitialized local variable 'u' used

These messages turn up hundreds of times in the build log, making it difficult to find 'real' warnings.

[Radar WebKit Bug Importer]

<rdar://problem/18154531>

[Mark Lam]

Created attachment 239361 [details]
the patch.

[Geoffrey Garen]

Comment on attachment 239361 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=239361&action=review

r=me

> Source/JavaScriptCore/dfg/DFGArgumentPosition.h:67
> +        return checkAndSet(m_shouldNeverUnbox, m_shouldNeverUnbox || shouldNeverUnbox);

Why is this needed?

[Mark Lam]

Comment on attachment 239361 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=239361&action=review

>> Source/JavaScriptCore/dfg/DFGArgumentPosition.h:67
>> +        return checkAndSet(m_shouldNeverUnbox, m_shouldNeverUnbox || shouldNeverUnbox);
> 
> Why is this needed?

Windows MSVC does not like bitwise ORing booleans.  That's what it was warning about, and flooding the build output with warning messages.

[Mark Lam]

Comment on attachment 239361 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=239361&action=review

>>> Source/JavaScriptCore/dfg/DFGArgumentPosition.h:67
>>> +        return checkAndSet(m_shouldNeverUnbox, m_shouldNeverUnbox || shouldNeverUnbox);
>> 
>> Why is this needed?
> 
> Windows MSVC does not like bitwise ORing booleans.  That's what it was warning about, and flooding the build output with warning messages.

To be accurate, the checkAndSet() template function compares the 2 arguments.  Using a bitwise OR results in the second argument becoming type int.  MSVC did not like a comparing a bool (first argument) to an int (second arg) in the reification of the template code.

[Mark Lam]

Thanks.  Landed in r174371: <http://trac.webkit.org/r174371>.

[Geoffrey Garen]

> To be accurate, the checkAndSet() template function compares the 2 arguments.  Using a bitwise OR results in the second argument becoming type int.  MSVC did not like a comparing a bool (first argument) to an int (second arg) in the reification of the template code.

A slightly better option here is probably to static_cast the result to bool. The original programmer did not intend for this code to include a branch, and the build fix introduced a logical branch. (Hopefully, the compiler will eliminate the logical branch, but still, it's nice for build fixes to honor the original programmer's intention.)

[Mark Lam]

*** Bug 125122 has been marked as a duplicate of this bug. ***