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

Replaced std::max usage #8

Closed
wants to merge 1 commit into from

Conversation

GlitchedPolygons
Copy link

This commit prevents error C2589: '(': illegal token on right side of '::' at L99 from happening on MSVC.


On MSVC++ 14.22, compiling thread_pool.hpp in my project causes the following compile-time error:

error C2589: '(': illegal token on right side of '::'
note: This diagnostic occurred in the compiler generated function 'void thread_pool::parallelize_loop(T,T,const F &,thread_pool::ui32)'

This is probably due to some macro redefinition from some external windows.h inclusion (uhrg, I know...). Nonetheless, this simple change fixes it and makes it compile fine now.

Thanks for taking the time in making this btw @bshoshany :)

Cheers

This commit prevents `error C2589: '(': illegal token on right side of '::'` at L99 from happening on MSVC.
@bshoshany
Copy link
Owner

Hmm, I never tested it with windows.h included. But I checked now and indeed got the same error. Weird! :O

I release my projects in cumulative updates, so my policy is not to accept pull requests. Instead, I will add this change to the next version, v1.4, once I perform some additional testing on my end, and possibly together with some other changes.

Thanks for your contribution, and I'm glad you found this project useful! :)

@bshoshany bshoshany closed this May 4, 2021
@GlitchedPolygons
Copy link
Author

Ah, thanks for the quick reaction :D

No worries, I'm glad I could help.

All the best :)

@GlitchedPolygons GlitchedPolygons deleted the patch-1 branch May 4, 2021 23:52
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.

2 participants