Add STATIC_ASSERT_INCOMPLETE_TYPE to enforce include minimality.

[Ivorforce]

Cross-includes are our main compile time problem right now. Not only does parsing take long, also every small change to a header file leads to hundreds of recompilations of compile units. It's time to do something.

STATIC_ASSERT_INCOMPLETE_TYPE is designed to protect ourselves against unnecessary includes. It lets us enforce certain couplings are not made, such as an include to dictionary.h from ustring.h. The assertion is made only for release build, to avoid interfering with the development cycle.
(This check was originally made for #107492, but I split it off to make it easier to discuss it separately).

I enforce 3 assumptions in this PR that already hold (no Dictionary include from String, and Dictionary and String from Array). There are many others we should be making. This is left to do as follow-up PRs (e.g. #106434).

[lawnjelly]

Is a good idea as it really needs discussion of its own. It looks fine to me (barring the question about the define above).

[Repiteo]

We could expand this assert to headers by making the template reusable. The easiest way would probably be __COUNTER__; while technically non-standard, it's de facto standard like #pragma once and alloca(). Not sure if there's a use-case for that right away, but I can see it being handy as the includes become more fine-grained

[Ivorforce]

We could expand this assert to headers by making the template reusable. The easiest way would probably be __COUNTER__; while technically non-standard, it's de facto standard like #pragma once and alloca(). Not sure if there's a use-case for that right away, but I can see it being handy as the includes become more fine-grained

I'm not sure I understand, could you expand on the idea?

Edit: Ohh, I think I get it. You mean probably something like this, right? (pseudocode)

// <my_header.h>
bool was_already_incomplete = is_incomplete_type_v<Type>();

#include "abc.h"
#include "def.h"
#include "gi.h"

if (!was_already_incomplete && is_incomplete_type_v<Type>()) {
    #error "Class was incomplete etc."
}

[lawnjelly]

Edit: Ohh, I think I get it. You mean probably something like this, right (pseudocode)?

Does that actually achieve anything that can't be done by putting this in the cpp? Apart from making the builds more likely to fail due to change in inclusion order?

[Ivorforce]

Edit: Ohh, I think I get it. You mean probably something like this, right (pseudocode)?

Does that actually achieve anything that can't be done by putting this in the cpp? Apart from making the builds more likely to fail due to change in inclusion order?

I don't think so. But it's clever! 😄

[lawnjelly]

Looks fine to me, but this will need a check by @akien-mga .

[Repiteo]

When suggesting __COUNTER__ for assertion reuse, this is what I was referring to:

// typedefs.h

template <typename T, int Counter, typename = void>
struct is_fully_defined : std::false_type {};

template <typename T, int Counter>
struct is_fully_defined<T, Counter, std::void_t<decltype(sizeof(T))>> : std::true_type {};

template <typename T, int Counter>
constexpr bool is_fully_defined_v = is_fully_defined<T, Counter>::value;

#if !defined(SCU_BUILD_ENABLED) && !defined(DEV_ENABLED) && !defined(DEBUG_ENABLED)
/// Enforces the requirement that a class is not fully defined.
/// This can be used to reduce include coupling and keep compile times low.
/// The check must be made at the top of the corresponding .cpp file of a header.
#define STATIC_ASSERT_INCOMPLETE_CLASS(m_type) \
	class m_type;                              \
	static_assert(!is_fully_defined_v<m_type, __COUNTER__>, #m_type " was unexpectedly fully defined. Please check the include hierarchy of '" __FILE__ "' and remove includes that resolve the class.");
#else
#define STATIC_ASSERT_INCOMPLETE_CLASS(m_type)
#endif

...
// ustring.cpp

#include "ustring.h"

STATIC_ASSERT_INCOMPLETE_CLASS(Dictionary); // Expectation: pass.
// Old: !is_fully_defined_v<Dictionary>. Passes.
// New: !is_fully_defined_v<Dictionary, 0>. Passes.

#include "core/variant/dictionary.h"

STATIC_ASSERT_INCOMPLETE_CLASS(Dictionary); // Expectation: fail.
// Old: !is_fully_defined_v<Dictionary>. Reuses previous template. Passes.
// New: !is_fully_defined_v<Dictionary, 1>. Distinct template. Fails.

[Ivorforce]

When suggesting __COUNTER__ for assertion reuse, this is what I was referring to

Oh, that makes a lot more sense than what I thought you meant.
I don't think we need it right now, but I agree — this is definitely worth keeping in mind for if this problem comes up.