Windows build not succeeding with the new /WExxxx directives

[juj]

Looking at the git history - awesome work on this project!

I was checking the build today from source on Windows 10 + Visual Studio 2022 Community edition.

On my setup, I got into trouble with these commits:

https://github.com/henricj/dunelegacy/commit/7ffc950618f0bc7b4b41d62a41f741bb975aefc5 https://github.com/henricj/dunelegacy/commit/b591c35634858e789005984925665585b576ee63

where in latest modernize branch I get a ton of errors in the build.

At first I thought I'd just clean the warnings, and started down a path like this:

diff --git a/include/GUI/Widget.h b/include/GUI/Widget.h
index 68178952..eb217740 100644
--- a/include/GUI/Widget.h
+++ b/include/GUI/Widget.h
@@ -271,7 +271,7 @@ public:
         \param  y               y-coordinate (relative to the left top corner of the widget)
         \param  insideOverlay   true, if (x,y) is inside an overlay and this widget may be behind it, false otherwise
     */
-    virtual void handleMouseMovement(int32_t x, int32_t y, bool insideOverlay) { }
+    virtual void handleMouseMovement(int32_t /*x*/, int32_t /*y*/, bool /*insideOverlay*/) { }

     /**
         Handles a left mouse click.
@@ -280,7 +280,7 @@ public:
         \param  pressed true = mouse button pressed, false = mouse button released
         \return true = click was processed by the widget, false = click was not processed by the widget
     */
-    virtual bool handleMouseLeft(int32_t x, int32_t y, bool pressed) { return false; }
+    virtual bool handleMouseLeft(int32_t /*x*/, int32_t /*y*/, bool /*pressed*/) { return false; }

     /**
         Handles a right mouse click.
@@ -289,7 +289,7 @@ public:
         \param  pressed true = mouse button pressed, false = mouse button released
         \return true = click was processed by the widget, false = click was not processed by the widget
     */
-    virtual bool handleMouseRight(int32_t x, int32_t y, bool pressed) { return false; }
+    virtual bool handleMouseRight(int32_t /*x*/, int32_t /*y*/, bool /*pressed*/) { return false; }

     /**
         Handles mouse wheel scrolling.

...

     /**
         This method is called if a child widget is destroyed (see Widget::~Widget).
         \param  pChildWidget    widget to remove
     */
-    virtual void removeChildWidget(Widget* pChildWidget) { }
+    virtual void removeChildWidget(Widget* /*pChildWidget*/) { }

     /**
         This method is called if the widget is removed from a container. If
diff --git a/tests/random/lemire_test.cpp b/tests/random/lemire_test.cpp
index f0b9a982..f8ca6d3a 100644
--- a/tests/random/lemire_test.cpp
+++ b/tests/random/lemire_test.cpp
@@ -20,9 +20,9 @@ TEST(Lemire_uint32, one) {
     generatorN                         g1{1u};
     generatorN                         g2_31{1u << 31};

-    EXPECT_EQ(0, d(g0));
-    EXPECT_EQ(0, d(g1));
-    EXPECT_EQ(0, d(g2_31));
+    EXPECT_EQ(0u, d(g0));
+    EXPECT_EQ(0u, d(g1));
+    EXPECT_EQ(0u, d(g2_31));
 }

 TEST(Lemire_uint32, two) {
@@ -32,10 +32,10 @@ TEST(Lemire_uint32, two) {
     generatorN                         g2_31{1u << 31};
     generatorN                         g_neg0{~0U};

-    EXPECT_EQ(0, d(g0));
-    EXPECT_EQ(0, d(g2_31_minus_1));
-    EXPECT_EQ(1, d(g2_31));
-    EXPECT_EQ(1, d(g_neg0));
+    EXPECT_EQ(0u, d(g0));
+    EXPECT_EQ(0u, d(g2_31_minus_1));
+    EXPECT_EQ(1u, d(g2_31));
+    EXPECT_EQ(1u, d(g_neg0));
 }

@@ -54,7 +54,7 @@ TEST_P(LemireRandomTests, twoRng) {
     for (auto i = 0U; i < tries; ++i) {
         const auto n = d(g);

-        EXPECT_LE(0, n);
+        EXPECT_LE(0u, n);
         EXPECT_LT(n, histogram.size());

         ++histogram.at(n);

but then realized that there are a lot of places where these kinds of warnings turned into errors. So I ended up adjusting the cmake line to

cmake -G "Visual Studio 17 2022" --preset=windows-x64-avx2-release -DDUNE_TARGET_COMPILE_FLAGS="/W4" -B . -S ../../..

to drop the extra /WExxxxs from the build. After that, Visual Studio does produce an error free build it looks like.

Didn't think too much of this at first, wondered this is probably known as maybe you are on a large cleanup spree like git commit history suggests. But then thought to report since the CI button shows green ( https://github.com/henricj/dunelegacy/actions/workflows/windows.yml ), curious how it managed a green build through this.

[henricj]

That is really strange. I just tried rebuilding lemire_test.cpp with a "-v" to make sure the flags are actually there and this was the result:

C:\src\game\dunelegacy\out\build\windows-x64-avx2-release>cmake --build . -v
[1/13] cmd.exe /C "cd /D C:\src\game\dunelegacy && "C:\Program Files\Microsoft Visual Studio\2022\Professional\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -D "GIT=C:/Program Files/Git/cmd/git.exe" -D DUNE_GENERATED_TMP_DIR=C:/src/game/dunelegacy/out/build/windows-x64-avx2-release/generated/tmp -D DUNE_GENERATED_INCLUDE_DIR=C:/src/game/dunelegacy/out/build/windows-x64-avx2-release/generated/include -D DUNE_VERSION_EXECUTE=true -P cmake/GitVersion.cmake"
-- DUNE_GIT_REPO_URL: https://github.com/henricj/dunelegacy.git
-- DUNE_GIT_REPO_BRANCH: modernize
-- DUNE_GIT_DESCRIBE: v0.96.4-0.1.4-452-gd72ae542-dirty
-- DUNE_GIT_MASTER_MERGE_BASE: dd6c791cebd737ed278cba1f21876f60649950a4
-- DUNE_GIT_DESCRIBE_MERGE_BASE: v0.96.4-165-gdd6c791c
-- DUNE_WINDOWS_SDK_VERSION: 10.0.22621.0
[2/3] C:\PROGRA~1\MIB055~1\2022\PROFES~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\cl.exe  /nologo /TP -DNOMINMAX -DNTDDI_VERSION=NTDDI_VISTASP2 -DWIN32_LEAN_AND_MEAN -D_WIN32_WINNT=_WIN32_WINNT_VISTA -IC:\src\game\dunelegacy\tests\random\..\..\include -external:I C:\src\game\dunelegacy\out\build\windows-x64-avx2-release\vcpkg_installed\x64-avx2-windows-ltcg-static\include -external:W0 /DWIN32 /D_WINDOWS /GR /EHsc /Zi   /DNDEBUG -MT /GL /diagnostics:caret /GA /GS /utf-8 /volatile:iso /permissive- /Zc:__cplusplus /Zc:inline /fp:fast /wd4267 /arch:AVX2 /Zi /EHsc /O2 /Ob3 /Gw /W4 /we4018 /we4100 /we4389 /we4456 /we4458 /we5222 -std:c++20 /showIncludes /Fotests\random\CMakeFiles\lemire_test.dir\lemire_test.cpp.obj /Fdtests\random\CMakeFiles\lemire_test.dir\ /FS -c C:\src\game\dunelegacy\tests\random\lemire_test.cpp
[3/3] cmd.exe /C "cd . && "C:\Program Files\Microsoft Visual Studio\2022\Professional\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E vs_link_exe --intdir=tests\random\CMakeFiles\lemire_test.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100226~1.0\x64\mt.exe --manifests  -- C:\PROGRA~1\MIB055~1\2022\PROFES~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\link.exe /nologo tests\random\CMakeFiles\lemire_test.dir\lemire_test.cpp.obj  /out:tests\random\lemire_test.exe /implib:tests\random\lemire_test.lib /pdb:tests\random\lemire_test.pdb /version:0.0 /machine:x64 /debug /INCREMENTAL /subsystem:console  /OPT:REF,ICF=3 /INCREMENTAL:NO /LTCG  vcpkg_installed\x64-avx2-windows-ltcg-static\lib\gtest.lib  vcpkg_installed\x64-avx2-windows-ltcg-static\lib\manual-link\gtest_main.lib  vcpkg_installed\x64-avx2-windows-ltcg-static\lib\gtest.lib  kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cmd.exe /C "cd /D C:\src\game\dunelegacy\out\build\windows-x64-avx2-release\tests\random && "C:\Program Files\PowerShell\7\pwsh.exe" -noprofile -executionpolicy Bypass -file C:/src/game/dunelegacy/external/vcpkg/vcpkg/scripts/buildsystems/msbuild/applocal.ps1 -targetBinary C:/src/game/dunelegacy/out/build/windows-x64-avx2-release/tests/random/lemire_test.exe -installedDir C:/src/game/dunelegacy/out/build/windows-x64-avx2-release/vcpkg_installed/x64-avx2-windows-ltcg-static/bin -OutVariable out""

That is a head-scratcher. The flags are there, yet the compiler isn't complaining.

Both the Linux and Windows CI builds use Ninja, based on the theory that using fewer build systems mean fewer headaches—and and since Ninja is usually noticeably faster than make or MSBuild.

Could you try a build using VS2022's CMake support instead of using the VS generator? I'll give the VS generator a try. It is multi-config and drags in MSBuild, but it should be producing pretty much the same cl.exe command line as above.

[henricj]

It's the vcpkg integration. The warnings are from lines in headers in the vcpkg directory and the -external:W0 is suppressing the warning since that is where the warnings are manifesting, despite the fact that the cause is in project's code.

Running the above command without the "-external" stuff results in the expected errors:

C:\src\game\dunelegacy\out\build\windows-x64-avx2-release>C:\PROGRA~1\MIB055~1\2022\PROFES~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\cl.exe /TP -DNOMINMAX -DNTDDI_VERSION=NTDDI_VISTASP2 -DWIN32_LEAN_AND_MEAN -D_WIN32_WINNT=_WIN32_WINNT_VISTA -IC:\src\game\dunelegacy\tests\random\..\..\include -IC:\src\game\dunelegacy\out\build\windows-x64-avx2-release\vcpkg_installed\x64-avx2-windows-ltcg-static\include  /DWIN32 /D_WINDOWS /GR /EHsc /Zi   /DNDEBUG -MT /GL /diagnostics:caret /GA /GS /utf-8 /volatile:iso /permissive- /Zc:__cplusplus /Zc:inline /fp:fast /wd4267 /arch:AVX2 /Zi /EHsc /O2 /Ob3 /Gw /W4 /we4018 /we4100 /we4389 /we4456 /we4458 /we5222 -std:c++20  /Fotests\random\CMakeFiles\lemire_test.dir\lemire_test.cpp.obj /Fdtests\random\CMakeFiles\lemire_test.dir\ /FS -c C:\src\game\dunelegacy\tests\random\lemire_test.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.32.31329 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

lemire_test.cpp
C:\src\game\dunelegacy\out\build\windows-x64-avx2-release\vcpkg_installed\x64-avx2-windows-ltcg-static\include\gtest/gtest.h(1626,1): error C4018: '<=': signed/unsigned mismatch
GTEST_IMPL_CMP_HELPER_(LE, <=)
^
C:\src\game\dunelegacy\tests\random\lemire_test.cpp(57): note: see reference to function template instantiation 'testing::AssertionResult testing::internal::CmpHelperLE<int,uint32_t>(const char *,const char *,const T1 &,const T2 &)' being compiled
        with
        [
            T1=int,
            T2=uint32_t
        ]
        EXPECT_LE(0, n);
C:\src\game\dunelegacy\out\build\windows-x64-avx2-release\vcpkg_installed\x64-avx2-windows-ltcg-static\include\gtest/gtest.h(1545,11): error C4389: '==': signed/unsigned mismatch
  if (lhs == rhs) {
          ^
C:\src\game\dunelegacy\out\build\windows-x64-avx2-release\vcpkg_installed\x64-avx2-windows-ltcg-static\include\gtest/gtest.h(1563): note: see reference to function template instantiation 'testing::AssertionResult testing::internal::CmpHelperEQ<T1,T2>(const char *,const char *,const T1 &,const T2 &)' being compiled
        with
        [
            T1=int,
            T2=uint32_t
        ]
                                 const T2& rhs) {
C:\src\game\dunelegacy\tests\random\lemire_test.cpp(23): note: see reference to function template instantiation 'testing::AssertionResult testing::internal::EqHelper::Compare<int,uint32_t,0x0>(const char *,const char *,const T1 &,const T2 &)' being compiled
        with
        [
            T1=int,
            T2=uint32_t
        ]
    EXPECT_EQ(0, d(g0));

This should probably be brought up with MS and/or the vcpkg team.