Boost.Spirit 1.76.0 compilation errors on Windows due to missing NOMINMAX

[phetdam]

Issue

Recently I was interested in experimenting with HPX on Windows so I cloned the GitHub repo and checked the v1.10.0 tag so I could build from source. I was following the Windows build instructions but figured I could build directly using CMake instead of opening Visual Studio, so after building and separately installing Boost 1.76.0, Asio 1.32.0, and hwloc 2.11.2, I ran:

cmake -S . -B build_windows_x64 -A x64 && cmake --build build_windows_x64 --config Release

Although the CMake configuration step succeeded, I noticed that files including Boost.Spirit headers would suffer compilation errors. For example, when compiling libs/core/affinity/src/parse_affinity_options.cpp, I observed errors like:

range_functions.hpp(53, 36): error C2589: '(': illegal token on right side of '::'
range_functions.hpp(53, 36): error C2760: syntax error: ')' was unexpected here, expected 'expression'
range_functions.hpp(53, 36): error C2760: syntax error: ')' was unexpected here, expected ';'
range_functions.hpp(53, 36): error C3878: syntax error: unexpected token ')' following 'expression_statement'

For brevity, only the relevant portions of the first few compiler errors are shown. I saw that similar errors are seen in other files that use Boost.Spirit, in particular libs/core/batch_environments/src/slurm_environment.cpp.

Workaround

Fortunately, I realized this might be due to macro expansion of min and max from a transitive Windows.h include so I set CXXFLAGS in the environment to be /D NOMINMAX which suppresses the min and max macro definitions in Windows.h.

Furthermore, to directly address the problem, I tried the following CMakeLists.txt change:

   hpx_add_config_cond_define(_WINSOCK_DEPRECATED_NO_WARNINGS)
   hpx_add_config_cond_define(_CRT_NON_CONFORMING_SWPRINTFS)
   hpx_add_config_cond_define(_SILENCE_FPOS_SEEKPOS_DEPRECATION_WARNING)
+  # need to unset Windows.h min and max macros as they conflict with
+  # Boost.Spirit usage of limits::min() and limits::max()
+  hpx_add_config_cond_define(NOMINMAX)

   # ############################################################################
   # Boost

Performing a clean build, i.e. deleting the build_windows_x64 build directory and then re-running CMake, worked.

Discussion

Since even the latest HEAD of master does not define NOMINMAX during Windows compilation I think that having this as a formal fix for Windows builds would be helpful. However, after trying to build HPX with Boost 1.79.0 instead, this issue seems to go away. Perhaps since 1.76.0 version of Boost.Spirit, or maybe just Boost in general, this may have been fixed.

I did want to open an issue so that there is a written record of my findings, but I'm not sure if a formal fix is necessary given that simply upgrading the installed version of Boost seems to fix the problem and the CXXFLAGS workaround is easy to do.

[hkaiser]

Is it really a good idea to define NOMINMAX globally? Wouldn't it be better to lift the minimal Boost version required for use with HPX to 1.77 (or whatever version has the fix)? Alternatively, we could define NOMINMAX just while compiling the files causing issues.

I would like to avoid seeping the definition of NOMINMAX into user code.

[phetdam]

Hm, I'm ok with having a higher minimum Boost version, but I haven't checked if 1.77.0 or 1.78.0 are also ok.

But generally for C++ code on Windows, if a Windows.h include is going to leak into a translation unit, defining NOMINMAX as part of the compilation flags should be ok as long as it does not change the behavior of including user-consumable HPX headers. If this is the case, then yes, defining NOMINMAX via add_compile_definitions(NOMINMAX) is a no go.

In the ideal case where HPX headers included by users will not be affected by a NOMINMAX being defined, however, it should be ok to do this in the CMakeLists.txt under an if(WIN32) conditional guard. Or as you mentioned, we can set this as a source file property or define it as part of a particular module's compile flags, whichever is better :)