Core: Support c++20 compilation #89660
Draft
+349
−357
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Repiteo
force-pushed
the
core/c++20-support
branch
6 times, most recently
from
917f79d
to
2537226
Compare
Sauermann
reviewed
Mar 19, 2024
| @@ -835,17 +835,6 @@ bool Variant::operator==(const Variant &p_variant) const { | |||
| return hash_compare(p_variant); | |||
| } | |||
|
|
|||
| bool Variant::operator!=(const Variant &p_variant) const { | |||
| // Don't use `!hash_compare(p_variant)` given it makes use of OP_EQUAL | |||
There was a problem hiding this comment.
This note matters, so I don't think this is a valid change, is not having an explicit != operator required for C++20? If so that's a problem
There was a problem hiding this comment.
It's not a requirement, no; if need be, this can be the exception to the implicit conversion. With that said, all the existing tests pass, and I don't think there's any Variant operation where x != y and !(x == y) produce different values. Still worth looking into for verification, but I'm skeptical of how much weight this note has.
Repiteo
force-pushed
the
core/c++20-support
branch
from
2537226
to
57d8729
Compare
Repiteo
force-pushed
the
core/c++20-support
branch
3 times, most recently
from
2b315bd
to
eea4fcd
Compare
Repiteo
force-pushed
the
core/c++20-support
branch
4 times, most recently
from
655a1da
to
4e4d1f5
Compare
Repiteo
force-pushed
the
core/c++20-support
branch
4 times, most recently
from
f3044b6
to
1b1499a
Compare
Repiteo
force-pushed
the
core/c++20-support
branch
from
1b1499a
to
23a59f4
Compare
• Technically c++23 as well, albeit to a *much* lesser extent because it's not officially released
Repiteo
force-pushed
the
core/c++20-support
branch
from
23a59f4
to
9ba6de0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a new
cpp_standardexperimental variable to SCons, to allow the user to choose which C++ standard to compile with. The choices are "17" and "20", defaulting to 17. In addition, this makes changes across the repo to allow compilation of c++20 to be possible in the first place, which primarily comes down to more operators to prevent ambiguous conversions. Currently, a c++20 build successfully compiles (withwerror=no).Submitted as a draft, as while the equality operators are accounted for, there's still the ordering operators. Inequality operators could be handled with a wrapper, but ordering operators will be a bit more involved than that; ideally the
<=>operator will be used, so long as parity with the secondary operators is kept between 17 & 20. There's also the matter of making sure the other warnings are accounted for, which are largely secondary but still important so long as they don't negatively impact c++17 (which will remain the repo "default").EDIT: Added "support" for c++23 as well, albeit to a much lesser extent because its currently experimental.