Core: Support c++20 compilation

[Repiteo]

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.

[adamscott]

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.

[Repiteo]

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

[adamscott]

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.

[adamscott]

Do we really want to add two builds though? It would take up much resources just for an edgecase that could be tested (not even needs to be tested regularly) by someone.

[Repiteo]

Hmm, that's fair. I could just assign that value to an existing job instead, because it being accounted for in some form is important

[hpvb]

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.

[Repiteo]

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

[hpvb]

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.

[Repiteo]

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.

[hpvb]

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.

[Repiteo]

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¹ 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². 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

Footnotes

1. https://en.cppreference.com/w/cpp/language/default_comparisons ↩

2. https://en.cppreference.com/w/cpp/language/overload_resolution#Call_to_an_overloaded_operator ↩

[hpvb]

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.

[adamscott]

I'm all aboard the C++20 train. Maybe for 4.5?