Skip to content
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

Fix for winmindef.h defining min/max macros #748

Merged
merged 1 commit into from
Oct 17, 2016

Conversation

sfinktah
Copy link
Contributor

@sfinktah sfinktah commented Sep 25, 2016

winmindef.h defines macros for min and max which cause several lines in documents.h to be expanded incorrectly whilst only presenting warnings that min and max lacked sufficient arguments.

See: http://stackoverflow.com/questions/22744262/cant-call-stdmax-because-minwindef-h-defines-max for more information.

@coveralls
Copy link

coveralls commented Sep 25, 2016

Coverage Status

Coverage remained the same at 99.937% when pulling 51a31ce on sfinktah:sfinktah-minwindef-fix into 7a39172 on miloyip:master.

@miloyip miloyip merged commit 3b1a037 into Tencent:master Oct 17, 2016
@sfinktah sfinktah deleted the sfinktah-minwindef-fix branch October 21, 2016 14:55
@chwarr
Copy link
Contributor

chwarr commented Sep 6, 2017

There's another approach that works across compilers. Adding an extra set of parenthesis. Would you prefer that instead of pushing/popping macro definitions? I can code it up and submit a PR.

static_cast<double>(std::numeric_limits<uint64_t>::max())

becomes

static_cast<double>((std::numeric_limits<uint64_t>::max)())

If you want me to code this up, there are some uses of min/max in the tests as well. Shall I guard them as well, or just the header files?

@miloyip
Copy link
Collaborator

miloyip commented Sep 6, 2017

@chwarr Can you provide a PR for this? I think all source code is better. Thanks

@chwarr
Copy link
Contributor

chwarr commented Sep 6, 2017

#1057 opened with the paren guard for min/max.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants