New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added NOMINMAX and WIN32_LEAN_AND_MEAN defines before including windows.sh #763

Merged
merged 2 commits into from Jan 16, 2017

Conversation

Projects
None yet
2 participants
@horenmar
Copy link
Member

horenmar commented Dec 16, 2016

This stops the windows.h header from defining min and max macros
and including lot of Windows APIs that are not needed by Catch.

Addresses #762.

@philsquared

This comment has been minimized.

Copy link
Collaborator

philsquared commented Jan 9, 2017

Seems like a no-brainer - thanks.
However I have one concern: If NOMINMAX or WIN32_LEAN_AND_MEAN are already #defined then this will fail or warn - then (if compilation continues) leave those identifiers always undefined

@horenmar

This comment has been minimized.

Copy link
Member

horenmar commented Jan 9, 2017

So guarding the define like this

#if !defined(NOMINMAX)
#define NOMINMAX
#define CATCH_DEFINED_NOMINMAX
#endif
... same for WIN32_LEAN_AND_MEAN
#include <windows.h>
#ifdef CATCH_DEFINED_NOMINMAX
#undef NOMINMAX
#endif

would be prefered? (Obviously at that point the logic would be factored out to catch_compiler_capabilities.h)

If we go this way, we could also provide a configuration option to stop Catch from including <windows.h> with these macros defined, but I think that is quite far past the point of diminishing returns and user who wants to include <windows.h> with all symbols in the same file as Catch main/runner, should just include it before including catch.

@philsquared

This comment has been minimized.

Copy link
Collaborator

philsquared commented Jan 9, 2017

That's one way to do it. I did wonder if it worked out easier to do the cross product of possibilities (so you don't need the intermediate identifiers) - but probably not - and certainly not scalable.
So, yes, if you go with your suggestion I think that should cover things sufficiently - not need to overthink it further.

Added NOMINMAX and WIN32_LEAN_AND_MEAN defines before including windo…
…ws.h

This stops the `windows.h` header from defining `min` and `max` macros
and including lot of Windows APIs that are not needed by Catch.
@horenmar

This comment has been minimized.

Copy link
Member

horenmar commented Jan 12, 2017

I rebased and changed the PR, but the results are not pretty.

#  ifdef CATCH_DEFINES_NOMINMAX
#    define NOMINMAX
#  endif
#  ifdef CATCH_DEFINES_WIN32_LEAN_AND_MEAN
#    define WIN32_LEAN_AND_MEAN
#  endif
#include <windows.h>
#  ifdef CATCH_DEFINES_NOMINMAX
#    undef NOMINMAX
#  endif
#  ifdef CATCH_DEFINES_WIN32_LEAN_AND_MEAN
#    undef WIN32_LEAN_AND_MEAN
#  endif

To the best of my knowledge, only way to fix this would be to create a proxy header, that hides away the preprocessor directives.

@horenmar horenmar merged commit afe46ff into catchorg:master Jan 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment