nvToolsExt.h defines min/max macros on Windows

[upsj]

When including nvToolsExt.h in a MSVC build, the Windows.h include file isn't guarded by NOMINMAX, so it breaks subsequent code that defines min or max functions. I guess we can't decide for users whether this is intended behavior, but maybe it should be documented somewhere, as not everybody is familiar with this issue on Windows.

[bernhardmgruber]

I have a similar problem with the small macro defined in rpcndr.h which is also included by Windowsh and breaks the use of NVTX in CUB. Maybe NVTX can try include only the relevant subparts of the WinAPI it needs.

[jcohen-nvidia]

This one is tricky. In general, Microsoft recommends that you include Windows.h instead of the more specific headers. They define lots of macros in Windows.h that affect the way the inner headers behave, so you can run into trouble if you include one of the inner Windows headers first and then include Windows.h later. For things like NOMINMAX, WIN32_LEAN_AND_MEAN, etc., it seems to be a generally-accepted practice that if you want to use those macros, you should always define them at the very top of your source file, before any #includes, so as to avoid transitively picking up the header before setting the define that affects it how you want. I usually try to set these with a -D option on the compiler commandline. As upsj said, it's not really an option for a header-only library like NVTX to decide and impose these defines on users. I think most developers who have run into the issue when calling std::min(...) or std::max(...) have googled around and found the sage advice to just wrap them in parens like (std::max)(...) to circumvent the preprocessor substitution. But Bernhard's issue with small seems like less of common-knowledge and would annoy CUB users, and I imagine there could be plenty more macro surprises like this, so fixing it in NVTX would certainly be nice.

I will try doing more surgical includes in NVTX instead of Windows.h and see if I can make that work reliably, so NVTX won't force you to do workarounds in your code. But this change could break existing code if it just includes NVTX headers and relies on its transitive includes to pick up Windows.h -- bad practice, of course, but lots of people do it -- so I'm a bit hesitant to make this change without a lot of testing. I'd recommend to both of you (and anyone else finding themselves here) to first try adding a workaround in your code to make it tolerate including the full Windows.h without defines like NOMINMAX, e.g. by using extra parens around min/max/small, and see if that works. It might turn out that even if I fix NVTX, you'd end up needing those workarounds anyway because other libraries you use include Windows.h. For the best user experience, I'm hoping we can fix both ends, i.e. I make NVTX not force its clients to do these workarounds, and the clients do the workarounds anyway so they're hardened against having this problem with other libraries later.