Bug 199248 – [MSVC] Catalog warnings

• Status: RESOLVED
• Resolution: FIXED
• Component: CMake

[Don Olmstead]

Currently we have a big old list of warnings with no context. So its not clear what we're avoiding.

[Don Olmstead]

Created attachment 372974 [details]
Patch

[Brent Fulgham]

Comment on attachment 372974 [details]
Patch

Oh, nice! r=me (assuming ews is happy).

[Fujii Hironori]

Comment on attachment 372974 [details]
Patch

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

> Source/cmake/OptionsMSVC.cmake:8
> +        /external:W2            # External header warning level

You should move below code for warning option here, and local variable _MSVC_WARNING_LEVEL.
    # More warnings. /W4 should be specified before -Wno-* options for clang-cl.
    string(REGEX REPLACE "/W3" "" CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
    string(REGEX REPLACE "/W3" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
    WEBKIT_PREPEND_GLOBAL_COMPILER_FLAGS(/W4)

[Fujii Hironori]

Comment on attachment 372974 [details]
Patch

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

>> Source/cmake/OptionsMSVC.cmake:8
>> +        /external:W2            # External header warning level
> 
> You should move below code for warning option here, and local variable _MSVC_WARNING_LEVEL.
>     # More warnings. /W4 should be specified before -Wno-* options for clang-cl.
>     string(REGEX REPLACE "/W3" "" CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
>     string(REGEX REPLACE "/W3" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
>     WEBKIT_PREPEND_GLOBAL_COMPILER_FLAGS(/W4)

, and remove local variable _MSVC_WARNING_LEVEL.

[Fujii Hironori]

Comment on attachment 372974 [details]
Patch

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

> Source/cmake/OptionsMSVC.cmake:137
>      string(REGEX REPLACE "/W3" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})

BTW, it seems that we don't need to remove "/W3" anymore.
https://cmake.org/cmake/help/latest/release/3.15.html#id15
> With MSVC-like compilers the value of CMAKE_<LANG>_FLAGS no longer contains warning flags like /W3 by default. See policy CMP0092.

[Don Olmstead]

Created attachment 373138 [details]
Patch

[Don Olmstead]

Created attachment 419482 [details]
Patch

[Fujii Hironori]

Comment on attachment 419482 [details]
Patch

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

> Source/cmake/OptionsMSVC.cmake:2
> +add_compile_options(/WX)

Don’t do this yet. I’m maintaining internal WinCairo clang jenkins jobs which report some compilation warnings.

[Don Olmstead]

Created attachment 419488 [details]
Patch

[Don Olmstead]

(In reply to Fujii Hironori from comment #8)
> Comment on attachment 419482 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=419482&action=review
> 
> > Source/cmake/OptionsMSVC.cmake:2
> > +add_compile_options(/WX)
> 
> Don’t do this yet. I’m maintaining internal WinCairo clang jenkins jobs
> which report some compilation warnings.

Patch now just catalogs all the warnings and removes ones I didn't hit with WinCairo.

🤞 that AppleWin comes back happy.

[EWS]

Committed r272457: <https://trac.webkit.org/changeset/272457>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419488 [details].

[Don Olmstead]

Reopening for follow up with Apple Win.

EWS was saying patch wasn't relevant so modifying a file outside of Source/cmake to get things going.

[Don Olmstead]

Created attachment 419494 [details]
Build fix

[Don Olmstead]

Created attachment 419495 [details]
Build fix

[EWS]

Committed r272459: <https://trac.webkit.org/changeset/272459>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 419495 [details].