Set C++ standard based on EMSCRIPTEN condition #1665
Conversation
|
Looks like we are not testing the web build... merging to see if it works |
|
It seems like everything is starting to require C++17. The only place I'm not sure that will work is the automated Windows builds, and I think someone said those can do it now. If so it would be better to just up the C++ version overall instead of for an individual platform. But that's an optimization... |
|
I was about to submit substantially the same PR and noticed this. Thanks @vespakoen.
Merge to master on your fork to test the CD and only push here when it works. Otherwise we just pollute the history.
@phkahler as for moving to C++17 everywhere - I'm strictly against it. It would be better if we keep the core C++11 and build the main targets (Windows, macOS, GTK3) with C++11. We should only use C++17 mode where it is required - currently Emscripten and supposedly QT6 (but it actually builds in C++11 mode, unless I am missing something). |
|
Sorry for polluting, I was over confident that it would work, I should have known better, messing with build systems... It works for me locally, will have to figure out whats going on on github actions, will try on my fork now... |
|
@vespakoen Also looks like nightly isn't getting updated, or is that because the web build failed? |
|
The errors suggest its not building with C++17 |
Emscripten 4.0.20 introduced a breaking change where Embind now requires C++17 or newer.
See: https://github.com/emscripten-core/emscripten/blob/main/ChangeLog.md#4020---111825
Since we use the latest Emscripten version in our CI, it now fails to compile the project.
This is an attempt to fix that by using the CMAKE_CXX_STANDARD property.