Core: Implement NotNull<T>

[Repiteo]

• Implements Add method hints for non-nullable return types and nullable arguments godot-proposals#2241
• Related Add RequiredParam<T> and RequiredResult<T> to mark Object * arguments and return values as required #86079
• Supersedes Core: Implement Nullable<T> template #90767

An alternative to David's RequiredPtr, implemented as a generic wrapper instead. This is a variation of the gsl-lite implementation of not_null, which diverges from that implementation in one main way: there's now a distinction between fatal & non-fatal nullability. That is:

• NotNull: Non-fatal failure if assigned null, but it will produce an error immediately so it's obvious where the mistake was made. Assignment is generally implicit, making transitioning to it for bound objects easy.
• NeverNull: Will fatally crash if assigned null, removing the need for external null handlers. Assignment is generally explicit, meaning binding changes need to be deliberate & null comparison is outright disabled.

In both cases, this implementation will prevent being assigned pure null/empty values at compile-time. For bound pointers, they can likely be replaced 1-to-1; Ref wrappers will need more work to ensure it functions, which I'll work on finalizing throughout the week. The gsl-lite logic could likely be further reduced in scope, as our needs are more specific, but this gives a functional starting point

[Repiteo]

Hmm, not sure why it started failing all of the sudden; maybe the explicit use of NeverNull threw something off? Gonna mark as a draft while I figure this out

[Bromeon]

Cool to see more people tackling this, thanks a lot! 🙂

It seems the main difference of NotNull vs. RequiredPtr is that this one moves the validation to the call site, thus causing an error earlier?

---

What's very elegant in @dsnopek's PR #86079 is that it's statically impossible to obtain the inner pointer when the value is null (type-state pattern). This provides the confidence that the precondition of a function is truly upheld.

In this PR on the other hand, we have

void Node::add_child(NotNull<Node *> p_child, bool p_force_readable_name, InternalMode p_internal) {
	ERR_FAIL_COND_MSG(data.inside_tree && !Thread::is_main_thread(), "Adding children to a node inside the SceneTree is only allowed from the main thread. Use call_deferred(\"add_child\",node).");

	ERR_THREAD_GUARD
	ERR_FAIL_NULL(p_child);

	... // actual function logic
}

and it's possible to accidentally forget the ERR_FAIL_NULL check, thus running into follow-up logic errors or even undefined behavior.

---

An alternative to David's RequiredPtr, implemented as a generic wrapper instead.

One thing that cannot be done inside a template like NonNull is a descriptive error message: you will get something generic like "non-null value assigned null", rather than "non-null parameter 'p_child' is null".

RequiredPtr also seems quite a bit simpler, without massive SFINAE machinery?

Where do you see the main advantages of this approach vs. RequiredPtr? Maybe it's somehow possible to combine the benefits of both? 🤔

[Repiteo]

The most obvious benefit is when using NeverNull<T>, this check wouldn't be necessary. This would, however, change internal behavior such that Godot would crash if passed a non-null pointer, which seems counter to Godot's goals. However, in many cases, those kinds of errors aren't recoverable in the first place, so maybe that's what we should be doing regardless? If we did that, it would effectively remove the SFINAE hurdles entirely, but would be compatibility-breaking

I'll whip up an alternative PR to give that style a spin, see which we wanna roll with

[Bromeon]

However, in many cases, those kinds of errors aren't recoverable in the first place

But they are recoverable as of today, no? Even if "print error and return" isn't necessarily the best course of action, it's in line with Godot's no-crash philosophy as you say 🙂

The most obvious benefit is when using NeverNull<T>, this check wouldn't be necessary.

But in all other cases (likely the majority), the RequiredPtr pattern would still be safer to use and provide more precise error messages.