Summary: | [MSVC] Catalog warnings | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Don Olmstead <don.olmstead> | ||||||||||||||
Component: | CMake | Assignee: | Don Olmstead <don.olmstead> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | annulen, bfulgham, ews-watchlist, gyuyoung.kim, Hironori.Fujii, mcatanzaro, mmaxfield, ryuan.choi, sergio | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Don Olmstead
2019-06-26 17:12:37 PDT
Created attachment 372974 [details]
Patch
Comment on attachment 372974 [details]
Patch
Oh, nice! r=me (assuming ews is happy).
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) 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. 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. Created attachment 373138 [details]
Patch
Created attachment 419482 [details]
Patch
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. Created attachment 419488 [details]
Patch
(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. Committed r272457: <https://trac.webkit.org/changeset/272457> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419488 [details]. 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. Created attachment 419494 [details]
Build fix
Created attachment 419495 [details]
Build fix
Committed r272459: <https://trac.webkit.org/changeset/272459> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419495 [details]. |