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

Use of min and max in Gil conflicts with Win32-API on Windows/MSVC #744

Closed
ckormanyos opened this issue Apr 17, 2024 · 4 comments · Fixed by #745
Closed

Use of min and max in Gil conflicts with Win32-API on Windows/MSVC #744

ckormanyos opened this issue Apr 17, 2024 · 4 comments · Fixed by #745

Comments

@ckormanyos
Copy link
Member

ckormanyos commented Apr 17, 2024

Actual behavior

I was recently using Boost.Gil with traditional Win32-API programming on MSVC (in Windows) and discovered a few conflicts with windows.h. Unfortunately windows.h defines min() and max() macros that conflict with the standard C++ ones.

Defining NOMINMAX (or un-defining BOOST_USE_WINDOWS_H) above inclusion of Windows-headers in client code works around this.

I could PR the fix with trivial changes on 9 lines. Should I do this PR?

Or would you just prefer to leave it, sort of forcing clients to definine NOMINMAX?

Expected behavior

It might be preferable, however, to not require defining NOMINMAX.

C++ Minimal Working Example

I found 9 instances in total, one example is here.

Instead of:

static constexpr Tuple min()

use

static constexpr Tuple (min)()

Environment

MSVC 141, 142, 143 solely in combination with Boost.Gil headers and the inclusion of <windows.h>.

  • Version(s) 1.84, 1.85 and on master and develop branches.
@mloskot
Copy link
Member

mloskot commented Apr 17, 2024

Thanks @ckormanyos for reporting this.
Although (min)() is a fine solution, I'm always unclear what is the Boost-wide convention to handle this annoyance. Do you perhaps know? I'd prefer to follow solution that is widely adopted by Boost developers and known to Boost users.

@ckormanyos
Copy link
Member Author

what is the Boost-wide convention to handle this annoyance. Do you perhaps know?

We use (min)() etc. in Math and Multiprecision. When I just started boosting, I learned or thought I learned from @jzmaddock that this parenthesis-based workaround is the Boost way.

@ckormanyos
Copy link
Member Author

I’ll just PR the trivial changes tonight and you can decide at your convenience if it’s a go or no-go.

@mloskot
Copy link
Member

mloskot commented Apr 17, 2024

We use (min)() etc. in Math and Multiprecision...

I'm used to this trick as well.

I’ll just PR the trivial changes tonight

Thank you. I'm sure nobody will object it and your PR will be merged. Especially, that we had accepted similar fixes e.g. #328

BTW, AFAICT, we mix both workarounds, the () and the NOMINMAX e.i. 87b7fdb The former is more portable, of course, as build system agnostic.

@mloskot mloskot linked a pull request Apr 18, 2024 that will close this issue
2 tasks
mloskot pushed a commit that referenced this issue Apr 18, 2024
Co-authored-by: Christopher Kormanyos <[email protected]>
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 a pull request may close this issue.

2 participants