godot
godot copied to clipboard
Core: Support c++20 compilation
Adds a new cpp_standard experimental 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 (with werror=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: All warnings accounted for, no longer a draft! Ordering operators are outside the scope of this PR, now that I'm more familiar with just how much would need to change.
EDIT: Added "support" for c++23 as well, albeit to a much lesser extent because its currently experimental.
I have no opinion on adding c++ 20 support. It depends on what the others think.
Hey, I noticed we are using a special #define to check for c++20 support.
Have we considered using the __cplusplus macro to gate on language feature support?
for c++20, it should be sufficient to use this check instead:
#if __cplusplus >= 202002L
See https://stackoverflow.com/a/53557910
The lack of out-of-the-box MSVC support made me hesitant, but seeing as there's a compiler option to enable it I might revisit the idea. Will have to test if the option in isolation causes any issues first
I have no opinion on adding c++ 20 support. It depends on what the others think.
I think it's nice if someday we are choosing to bump up our standards to C++20. It will compile off-the-bat.
It's possible that the C++20 minimum versions can be more lax than I originally expected, at least for our purposes, so I'm trying that out this rebase. Ideally, C++20 will be fully-functional on ubuntu-22.04
EDIT: Yep, seems good! Should make an eventual transition to C++20 much more reasonable
Should make an eventual transition to C++20 much more reasonable
Especially if we want to be able to use consteval and constinit. Could happen sooner than we think.
I'm not sure I like the new INEQUALITY_OPERATOR macro, we don't have those for other operators. It feels out of place and a little confusing.
It kind of smells like a workaround for something.
The alternative is adding a bunch of cpp20 conditionals everywhere. It wouldn't be dealbreaking, but I wasn't sure if that was desired when starting off
The alternative is adding a bunch of cpp20 conditionals everywhere. It wouldn't be dealbreaking, but I wasn't sure if that was desired when starting off
I think we should just either move to c++20, or not. I don't think that supporting c++17 while also using c++20 features is going to lead to a very pleasant codebase personally.
I think we should just either move to c++20, or not. I don't think that supporting c++17 while also using c++20 features is going to lead to a very pleasant codebase personally.
To clarify, this isn't using C++20 features. This is only introducing fixes to make C++20 compilation work outright. Actually adding c++20 functionality wasn't within the original scope of the PR.
I would love to migrate to C++20 fully. Concepts in particular would be the biggest draw, but consteval & three-way comparison aren't far behind.
To clarify, this isn't using C++20 features. This is only introducing fixes to make C++20 compilation work outright. Actually adding c++20 functionality wasn't within the original scope of the PR.
I'll have to have a deeper look at that macro then, surely there's a way to do this is a way that's less jarring!
Thanks for working on this by the way, I don't mean to sound super negative.
The only two real alternative is giving all inequality operators #if __cplusplus < 202002L wrappers, because C++20 fundamentally changes how equality/comparison operators are parsed. As a whole, the C++20 changes are a net positive, as it means a lot of logic can be defaulted away^1 and don't require multi-scope shenanigans:
class String {
...
bool operator==(const String &p_str) const; // Universal in C++17 & C++20
bool operator==(const char *p_str) const; // One-way in C++17, universal in C++20
...
}
// bool operator==(const String &p_str, const String &p_str); // "Ambiguous" error in C++17 & C++20
bool operator==(const char *p_cstr, const String &p_str); // Workaround in C++17, "ambiguous" error in C++20
The latter bit has to do with C++20 automatically rearranging the == (and <=>) operators in ways that previously required explicit specification^2. So now a == b now implicitly defines b == a, a != b, and b != a. However, part of this automatic setup means that the previously explicit operators can cause ambiguity errors, which is why the majority of explicitly defined comparison operators are being deprecated in the standard library — <=> (and == if non-default) is all you'd need! If I were to no longer utilize the INEQUALITY_OPERATOR macro, instead incorporating the new <compare> library with C++20 code, this would result in:
// RID, defaulted
#if __cplusplus < 202002L
_ALWAYS_INLINE_ bool operator==(const RID &p_rid) const {
return _id == p_rid._id;
}
_ALWAYS_INLINE_ bool operator<(const RID &p_rid) const {
return _id < p_rid._id;
}
_ALWAYS_INLINE_ bool operator<=(const RID &p_rid) const {
return _id <= p_rid._id;
}
_ALWAYS_INLINE_ bool operator>(const RID &p_rid) const {
return _id > p_rid._id;
}
_ALWAYS_INLINE_ bool operator>=(const RID &p_rid) const {
return _id >= p_rid._id;
}
_ALWAYS_INLINE_ bool operator!=(const RID &p_rid) const {
return _id != p_rid._id;
}
#else
_ALWAYS_INLINE_ std::strong_ordering operator<=>(const RID &p_rid) const = default; // Handles literally everything.
#endif
// Vector2, non-default
#if __cplusplus < 202002L
bool operator==(const Vector2 &p_other) const;
bool operator!=(const Vector2 &p_other) const;
bool operator<(const Vector2 &p_other) const { return x == p_other.x ? (y < p_other.y) : (x < p_other.x); }
bool operator>(const Vector2 &p_other) const { return x == p_other.x ? (y > p_other.y) : (x > p_other.x); }
bool operator<=(const Vector2 &p_other) const { return x == p_other.x ? (y <= p_other.y) : (x < p_other.x); }
bool operator>=(const Vector2 &p_other) const { return x == p_other.x ? (y >= p_other.y) : (x > p_other.x); }
#else
std::weak_ordering operator<=>(const Vector2 &p_other) const { // Non-default thanks to anonymous unions.
if (x <=> p_other.x != 0) {
return x <=> p_other.x;
}
return y <=> p_other.y;
}
bool operator==(const Vector2 &p_other) const { return *this <=> p_other == 0; } // Required because non-default.
// Explicit comparison operators *between* types often needed in C++20, so they can be scoped.
std::weak_ordering operator<=>(const Vector2i &p_other) const;
bool operator==(const Vector2i &p_other) const;
#endif
Well, I'm not the final arbiter of this decision. But I think my preference would go to a PR that just sets our minimum compiler to c++20, and makes these changes. I'd support that, I don't think there's any pressing reason at this time to support older compilers. Especially since we have the Godot toolchains for people who want to build on ancient Linux distros.
Again, not my choice, but I am really grossed out by the macro approach, even though I agree with you that it is the "least bad" option.
I'm all aboard the C++20 train. Maybe for 4.5?
Superseded by #100749