google / llvm-premerge-checks

CI system for premerge-testing in LLVM project
Apache License 2.0
41 stars 37 forks source link

Windows Precommit CI was not updated for the recent base LLVM language version change? #416

Open AaronBallman opened 2 years ago

AaronBallman commented 2 years ago

Community recently switched our base language version from C++14 to C++17 (in terms of what code we can use in the project): https://discourse.llvm.org/t/c-17-in-llvm-code-base/64120 https://reviews.llvm.org/D130689

However, it looks like at least the Windows precommit CI builder was not updated: https://buildkite.com/llvm-project/premerge-checks/builds/111592#018319f4-801f-4192-825e-c44a2a7a7c81

FAILED: tools/clang/lib/Analysis/CMakeFiles/obj.clangAnalysis.dir/ThreadSafety.cpp.obj
sccache C:\BuildTools\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\cl.exe  /nologo /TP -DBUILD_EXAMPLES -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GNU_SOURCE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\clang\lib\Analysis -IC:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis -IC:\ws\w9\llvm-project\premerge-checks\clang\include -Itools\clang\include -Iinclude -IC:\ws\w9\llvm-project\premerge-checks\llvm\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2  /EHs-c- /GR- -UNDEBUG -std:c++14 /showIncludes /Fotools\clang\lib\Analysis\CMakeFiles\obj.clangAnalysis.dir\ThreadSafety.cpp.obj /Fdtools\clang\lib\Analysis\CMakeFiles\obj.clangAnalysis.dir\ /FS -c C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp
C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2107): error C2429: language feature 'init-statements in if/switch' requires compiler flag '/std:c++17'
C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2120): error C2429: language feature 'init-statements in if/switch' requires compiler flag '/std:c++17'
C:\ws\w9\llvm-project\premerge-checks\clang\lib\Analysis\ThreadSafety.cpp(2418): error C2429: language feature 'init-statements in if/switch' requires compiler flag '/std:c++17'

Note how the command line is still using -std:c++14. I'm a bit surprised that the community's changes to our CMake didn't automatically switch this to C++17 for the precommit builder automatically.

metaflow commented 2 years ago

yes, that's strange it should ran cmake / ninja without much of extra configurations

AaronBallman commented 2 years ago

Yeah, it makes me wonder if that particular review just got a "stale" view of the world somehow (some sort of caching issue maybe)? But the changes to the patch came well after we landed the switch to C++17 in the code base, I believe.