libassert
libassert copied to clipboard
jeremy-rifkin
Add a full C++20 & C++23 compatibility layer
This PR focuses on building the infrastructure to allow the use of modern C++20 and C++23 features if available. Implementing the library features into the project is outside this PR's scope. Instead, this PR is focused on laying the foundation so that we can gradually add newer features to the library while still maintaining backward support with C++17!
My first initial revision of what this would look like has been finished. I would though like to request permission to add C++20 and C++23 to the CI so we can properly test against this.
Applied requested changes and also added in a few more items that I found were missing.
I've come across a strange error with the current approach to implementing the use of both an internal source_location and std::source_location. It's an issue I've never experienced before. Currently the preprocessor gets mega confused with the project with how one would traditionally switch between two implementations with macros if one target is compiling for C++20 but another target is compiling for C++17. It appears this trips up the preprocessor and causes a link time error. What is happening is essentially as follows:
- libassert internally sets its compile option to C++17.
- The target demo then sets its compile options to C++20.
- The preprocessor sees C++20 and then copy and pastes the correct items for C++20.
- Then the linker trys to perform linking but instead sees C++17 for some reason.
- This causes a linker error and the project fails.
If both targets are using the same C++ version then this issue does not occur.
Some of this is speculation based on what I've observed through debugging and its a strange issue. I'm not 100% sure what would be the best way to approach it without doing some compilation checking through cmake or something to verify what C++ version we are using, but this would be much more work.
What are your thoughts on this issue?
I haven’t read the new changes yet but this sounds expected. If you conditionally enable something based on the standard and that makes changes in the compiled library then it won’t be compatible with code built with a different standard. You’re lucky you got a linker error, this can easily be a nasty ODR violation that doesn’t manifest until it causes a segfault :P
I see no reason to go out of the way to use newer C++ features like this. The C++17 version works and I’m not sure I see the benefit in this. It can only lead to complexity, code duplication, and ODR problems. I’m happy to support newer things if it makes a difference for end-users, though.
I haven’t read the new changes yet but this sounds expected. If you conditionally enable something based on the standard and that makes changes in the compiled library then it won’t be compatible with code built with a different standard. You’re lucky you got a linker error, this can easily be a nasty ODR violation that doesn’t manifest until it causes a segfault :P
I see no reason to go out of the way to use newer C++ features like this. The C++17 version works and I’m not sure I see the benefit in this. It can only lead to complexity, code duplication, and ODR problems. I’m happy to support newer things if it makes a difference for end-users, though.
That is fine. I'll make a final adjustment and get everything together.
For proof of concept on the C++26's user generated static_assert you can see it in action here:
https://godbolt.org/z/hY7fz5rKG
For info on what it is here is the specification:
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2741r1.pdf
MSVC is breaking because of using __VA_OPT__ https://godbolt.org/z/P16d3Gr5c
Since this doesn't appear to be an issue you introduced I'm going to go ahead and merge and fix on dev
Oh, it was introduced here due to #define LIBASSERT_CPLUSPLUS _MSVC_LANG. But we can workaround that.