1675783 - Improve implementation of mozilla::Result<T*, JS::OOM>

Status: NEW

(Core :: JavaScript Engine, task, P3)

Description

[Ted Campbell [:tcampbell]]

In the current implementation of mozilla::Result, this construct uses low-bit tagging to handle error cases. This seems to result in slightly poorer code generated than our traditional nullptr-is-failure pattern.

Firstly, the JS::OOM type should be an empty type rather than just default-constructible. We'd also have another empty type for general errors. Having automatic coercion here would help a lot. Distinguishing between pending-exception and pending-exception-or-uncatchable here is probably valuable.

Then, we'd want to add another backed to mozilla::Result that is the reverse of NullIsOk and is instead NullIsErr.

The next ergonomic issue is how to indicate the pointer is non-null. We could either pass a T& or some sort of NonNull<T*> wrapper. Using NonNull would allow MOZ_TRY_VAR to work directly, but we could also consider adding MOZ_TRY_REF (probably the wrong name). References cannot be rebound in C++, so support for local pointer variables is important.

Having this be meaningful is also heavily tied into reworking our allocation systems as well. Careful handling of Result<T, JS::OOM> is meaningless when subtle footguns such as mozilla::Vector::pushBack and js::Vector::pushBack having different defaults about whether an exception is even pending. We should consider if the js::Vector type should instead be more of a wrapper and have support for generating appropriate Result types anyways. If we did this, we could also consider passing cx directly to wrapped methods instead of storing the cx pointer in each Vector.