build: Add NOMINMAX definition for MSVC compatibility

[siposcsaba89]

• Build system improvement:
  • Added #pragma push_macro/#pragma pop_macro to prevent conflicts with the min and max macros defined in Windows headers.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.64%. Comparing base (2a15fa0) to head (0776681).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #478   +/-   ##
=======================================
  Coverage   96.64%   96.64%           
=======================================
  Files          33       33           
  Lines        2622     2622           
=======================================
  Hits         2534     2534           
  Misses         88       88           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[stotko]

Thanks! I'm a bit hesitant to merge this fix since I don't believe that the proposed preprocessor definition should belong to stdgpu. In fact, the culprit is the <windows.h> header which defines non-standard macros for min and max that would clash with the stdgpu's algorithm.h and limits.h headers. However, <windows.h> is not directly used by stdgpu and unconditionally defining NOMINMAX may break cases where this case is already accounted for. Could you provide more details on your use case?

[siposcsaba89]

Thanks! I'm a bit hesitant to merge this fix since I don't believe that the proposed preprocessor definition should belong to stdgpu. In fact, the culprit is the <windows.h> header which defines non-standard macros for min and max that would clash with the stdgpu's algorithm.h and limits.h headers. However, <windows.h> is not directly used by stdgpu and unconditionally defining NOMINMAX may break cases where this case is already accounted for. Could you provide more details on your use case?

Sorry, thad I did not give you more detailed description. I am trying to port our project to compile with cuda 13, and I belive that thrust and cuda in version 13 includes windows.h or some header from windows that defines min, max, which breaks our build. I understand, that you are hesitant, and I tought about that too, that it could break other cases, could give warning from already defined macros, etc., but if I do not set it, it won't compile. Maybe a pragma pop, push solution would be better, that's how nvidia does it, so I think you are right not to merge it, let me see if I can provide a better solution.

[siposcsaba89]

I just made a #pragma push_macro/#pragma pop_macro variation, needed to add to two files. I am not sure, that it is better or not, it solves the problem locally insted of global define, but maybe it would be better to put these in a header file. I am not sure. But for now I think it is the best I can come up with.

[stotko]

Thanks for coming up with a local version. Limiting the scope to only the necessary areas is the right direction 👍 Since this is a Windows-only issue, I suggest to further limit the scope to that particular platform even though it should have no effect on other platforms.

[stotko]

Suggested change

	#pragma push_macro("min")
	#pragma push_macro("max")
	#undef min
	#undef max
	#if defined(_WIN32)
	#pragma push_macro("min")
	#pragma push_macro("max")
	#undef min
	#undef max
	#endif

[stotko]

Suggested change

	#pragma pop_macro("min")
	#pragma pop_macro("max")
	#if defined(_WIN32)
	#pragma pop_macro("min")
	#pragma pop_macro("max")
	#endif

[stotko]

See above

[stotko]

See above

[siposcsaba89]

Thanks for coming up with a local version. Limiting the scope to only the necessary areas is the right direction 👍 Since this is a Windows-only issue, I suggest to further limit the scope to that particular platform even though it should have no effect on other platforms.

With test to compile I also needed to add it to several files, so decided to create a header push/pop pair, but it counts where the user includes them, but clang-format keep fighting the order. I am not sure what is the best solution here.

What do you think about a private NOMINMAX definition to be able to compile stdgpu (for example vcpkg, as we do it), then everyone should handle NOMINMAX in their own project? (Maybe it would be cleaner than putting push/pop everywhere)