-
Notifications
You must be signed in to change notification settings - Fork 105
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 name clash with std::byte
#764
Conversation
if(WIN32) | ||
target_compile_definitions(skinManagerGui PUBLIC _HAS_STD_BYTE=0) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently, there's a native problem with some Windows SDK and std::byte
defined in C++17.
This problem impacts the compilation of some skinManagerGui
auto-generated files.
Here's the proposed solution.
We also need to be careful as in the future _HAS_STD_BYTE
will get removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we open an issue on how to fix this in a more permanent way? Perhaps just avoiding a windows include or a using namespaces std
somewhere in skinManagerGui could help in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[rem]: https://github.com/microsoft/STL/issues/20
I guess there was a typo and the correct issues is microsoft/STL#204 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we open an issue on how to fix this in a more permanent way? Perhaps just avoiding a windows include or a
using namespaces std
somewhere in skinManagerGui could help in this case.
Unfortunately, the failure is triggered within files that are generated by Qt, which in turn do include the "problematic" Windows SDK resources.
After a quick F2F alignment, we've decided to go this way.
Fixes #763.