[Compilation Error] Error when including <windows.h> before <rfl/Result.hpp>

[NiNeDelixe]

Description

There is a compilation error when including Windows headers before the librarys Result.hpp file. The compilation succeeds only when the include order is reversed.

Steps to Reproduce

Failing case (Windows headers first):

#include <windows.h>
#include <rfl/Result.hpp>

Attempt to compile the code

Observe compilation errors

Workaround (compiles successfully):

#include <rfl/Result.hpp>
#include <windows.h>

Expected Behavior

The code should compile successfully regardless of the include order.

Actual Behavior

Compilation fails with errors when <windows.h> is included before <rfl/Result.hpp>.

Error Messages

[main] Building folder: D:/source/PRD4210/COURSE_WORK/Ulin/out/build/x86-debug 
[build] Starting build
[proc] Executing command: "D:\Program Files\CMake\bin\cmake.EXE" --build D:/source/PRD4210/COURSE_WORK/Ulin/out/build/x86-debug --
[build] [1/2] Building CXX object _exec\backend\CMakeFiles\backend.dir\main.cpp.obj
[build] FAILED: [code=2] _exec/backend/CMakeFiles/backend.dir/main.cpp.obj 
[build] "D:\ProgramFiles\Microsoft VisualStudio\VC\Tools\MSVC\14.44.35207\bin\Hostx64\x86\cl.exe"  /nologo /TP -DYYJSON_IMPORTS -ID:\source\PRD4210\COURSE_WORK\Ulin\thirdparty\cpp-static-block -ID:\source\PRD4210\COURSE_WORK\Ulin\src\backend -ID:\source\PRD4210\COURSE_WORK\Ulin\thirdparty\dotenv-cpp\include\laserpants\dotenv -external:ID:\source\PRD4210\COURSE_WORK\Ulin\out\build\x86-debug\vcpkg_installed\x86-windows\include -external:W0 /DWIN32 /D_WINDOWS /GR /EHsc /Zi /Ob0 /Od /RTC1 -std:c++20 -MDd /wd4251 /wd4275 /showIncludes /Fo_exec\backend\CMakeFiles\backend.dir\main.cpp.obj /Fd_exec\backend\CMakeFiles\backend.dir\ /FS -c D:\source\PRD4210\COURSE_WORK\Ulin\src\backend\main.cpp
[build] D:\source\PRD4210\COURSE_WORK\Ulin\out\build\x86-debug\vcpkg_installed\x86-windows\include\rfl/Result.hpp(83): error C2589: (: invalid lexeme to the right of "::"
[build] D:\source\PRD4210\COURSE_WORK\Ulin\out\build\x86-debug\vcpkg_installed\x86-windows\include\rfl/Result.hpp(83): note: template instantiation context (oldest first)
[build] D:\source\PRD4210\COURSE_WORK\Ulin\out\build\x86-debug\vcpkg_installed\x86-windows\include\rfl/Result.hpp(80): note: when compiling the class template "rfl::Result"
[build] D:\source\PRD4210\COURSE_WORK\Ulin\out\build\x86-debug\vcpkg_installed\x86-windows\include\rfl/Result.hpp(83): error C2059: syntax error: )
[build] D:\source\PRD4210\COURSE_WORK\Ulin\out\build\x86-debug\vcpkg_installed\x86-windows\include\rfl/Result.hpp(83): error C2947: required ">" to complete template argument list, detected ">"
[build] ninja: build stopped: subcommand failed.
[proc] The command: "D:\Program Files\CMake\bin\cmake.EXE" --build D:/source/PRD4210/COURSE_WORK/Ulin/out/build/x86-debug -- exited with code: 2
[driver] Build completed: 00:00:01.194
[build] Build finished with exit code 2

Environment

Compiler Version: MSVC 19.44.35217 for x86
Library Version: From current vcpkg version
Operating System: Windows
Build System: Cmake minimum 4.2

CmakeLists.txt

CMakePresets.json

vcpkg.json

main.cpp

[liuzicheng1987]

@NiNeDelixe thanks for reporting!

[saxbophone]

@NiNeDelixe try adding the macro definition NOMINMAX --I have seen this same issue on Windows and adding that fixed the issue. The Windows.h header unfortunately breaks stdlib by defining macros for min() and max() that conflict with std::min() and std::max()! 😦 (Microsoft doing Microsoft things as usual).

[NiNeDelixe]

@NiNeDelixe try adding the macro definition NOMINMAX --I have seen this same issue on Windows and adding that fixed the issue. The Windows.h header unfortunately breaks stdlib by defining macros for min() and max() that conflict with std::min() and std::max()! 😦 (Microsoft doing Microsoft things as usual).

Yeah, with macro is working

[saxbophone]

@liuzicheng1987 I'm interested to get your opinion on whether fixing this in reflect-cpp is in-scope or not? IIRC, the first time I experienced this issue, it was with the spdlog library and my response was to shrug and add the macro definition.

I do think personally the issue is that Windows.h is broken, not these third-party libraries. What do you think?

[liuzicheng1987]

@saxbophone good question. I was thinking about that as well. At the end of the day...it's just one line we need to add, right?

[saxbophone]

@saxbophone good question. I was thinking about that as well. At the end of the day...it's just one line we need to add, right?

Yes, theoretically it's just an addition of something like

target_compile_definitions(reflectcpp PUBLIC NOMINMAX)

in the CMake.

In my opinion, it's ultimately a question of whether you think it's legitimate to take this responsibility on in your library
—ultimately, it's the Windows.h header that breaks C++14, not your library. Theoretically, if the WinAPI was a CMake-based library, we could say that adding -DNOMINMAX is part of its usage requirements 😏.

I think for my own libraries, I would consider this a wontfix for that reason, maybe you have a different opinion. The workaround is low-cost and Microsoft is probably not going to "fix" this anytime soon, alas...