Clang was failing because because it rightfully saw we were attempting
to call a deleted constructor of `MarkedValueList`. If you explicitly
called move(list) then GCC would complain that the move was unnecessary.
For what ever reason both tool chains accept when we construct the
ThrowCompletionOr explicitly that we move the list into and return that.
We decided that we want to move away from throwing exceptions in AOs
and regular functions implicitly and then returning some
default-constructed value (usually an empty JS::Value) - this requires
remembering to check for an exception at the call site, which is
error-prone. It's also awkward for return values that cannot be
default-constructed, e.g. MarkedValueList.
Instead, the thrown value should be part of the function return value.
The solution to this is moving closer to the spec and using something
they call "completion records":
https://tc39.es/ecma262/#sec-completion-record-specification-type
This has various advantages:
- It becomes crystal clear whether some AO can fail or not, and errors
need to be handled and values unwrapped explicitly (for that reason
compatibility with the TRY() macro is already built-in, and a similar
TRY_OR_DISCARD() macro has been added specifically for use in LibJS,
while the majority of functions doesn't return ThrowCompletionOr yet)
- We no longer need to mix "valid" and "invalid" values of various types
for the success and exception outcomes (e.g. null/non-null AK::String,
empty/non-empty JS::Value)
- Subsequently it's no longer possible to accidentally use an exception
outcome return value as a success outcome return value (e.g. any AO
that returns a numeric type would return 0 even after throwing an
exception, at least before we started making use of Optional for that)
- Eventually the VM will no longer need to store an exception, and
temporarily clearing an exception (e.g. to call a function) becomes
obsolete - instead, completions will simply propagate up to the caller
outside of LibJS, which then can deal with it in any way
- Similar to throw we'll be able to implement the functionality of
break, continue, and return using completions, which will lead to
easier to understand code and fewer workarounds - the current
unwinding mechanism is not even remotely similar to the spec's
approach
The spec's NormalCompletion and ThrowCompletion AOs have been
implemented as simple wrappers around the JS::Completion constructor.
UpdateEmpty has been implemented as a JS::Completion method.
There's also a new VM::throw_completion<T>() helper, which basically
works like VM::throw_exception<T>() - it creates a T object (usually a
JS::Error), and returns it wrapped in a JS::Completion of Type::Throw.
Two temporary usage patterns have emerged:
1. Callee already returns ThrowCompletionOr, but caller doesn't:
auto foo = TRY_OR_DISCARD(bar());
2. Caller already returns ThrowCompletionOr, but callee doesn't:
auto foo = bar();
if (auto* exception = vm.exception())
return throw_completion(exception->value());
Eventually all error handling and unwrapping can be done with just TRY()
or possibly even operator? in the future :^)
Co-authored-by: Andreas Kling <kling@serenityos.org>
This is much more common across the whole codebase and even these two
files. The same is used to return an empty JS::Value in an exception
check, for example.
'bindings' is the spec-compliant version of 'variables', but we were
simply not even looking at them, which made things using bindings (such
as named function expressions) break in unexpected ways after the move
to using references in call expressions.
Co-Authored-By: davidot <david.tuin@gmail.com>
The save/load of parser state performed by lookahead parsing is quite
expensive so let's try to avoid it in the most common case.
This is a 15-20% speedup on various chunks of JS I've tested. :^)
Before this change, Lexer::is_identifier_{start,middle}() would do a
Unicode property lookup via Unicode::code_point_has_property() quite
frequently, especially for common characters like .,;{}[]() etc.
Since these and any other ASCII characters not covered by the alpha /
alphanumeric check are known to not have the ID_Start / ID_Continue
(except '_', which is special-cased now) properties, we can easily
avoid this function call.
In the spec, this happens in the EvaluateCall abstract operation
(https://tc39.es/ecma262/#sec-evaluatecall), and the order is defined
as:
3. Let argList be ? ArgumentListEvaluation of arguments.
4. If Type(func) is not Object, throw a TypeError exception.
5. If IsCallable(func) is false, throw a TypeError exception.
In LibJS this is handled by CallExpression::execute(), which had the
callee function check first and would therefore never evaluate the
arguments for a non-function callee.
...instead of relying on the VM having a current execution context. This
was an incorrect assumption I made, and it caused onfoo attribute
handler construction in LibWeb to crash.
Just use the same mechanism as NativeFunction in the meantime.
There's currently a fallback at the call site where the Realm is needed
(due to a slightly incorrect implementation of [[Call]] / [[Construct]])
so this is better than crashing (in LibWeb, currently).
We need both a GlobalObject and Realm now, but can get the former from
the latter (once initialized).
This also fixes JS execution in LibWeb, as we failed to set the Realm of
the newly created Interpreter in this function.
