In the end this is a nicer API than having separate has_{value,target}()
and having to check those first, and then making another Optional from
the unwrapped value:
completion.has_value() ? completion.value() : Optional<Value> {}
// ^^^^^^^^^^^^^^^^^^
// Implicit creation of non-empty Optional<Value>
This way we need to unwrap the optional ourselves, but can easily pass
it to something else as well.
This is in anticipation of the AST using completions :^)
The spec has a note stating that resolve binding will always return a
reference whose [[ReferencedName]] field is name. However this is not
correct as the underlying method GetIdentifierReference may throw on
env.HasBinding(name) thus it can throw. However, there are some
scenarios where it cannot throw because the reference is known to exist
in that case we use MUST with a comment.
Now that it only needs to deal with ECMAScriptFunctionObject via
internal_call() / internal_construct(), we can:
- Remove the generic FunctionObject parameter
- Move it from the VM to ECMAScriptFunctionObject
- Make it private
Now that it only needs to deal with ECMAScriptFunctionObject via
internal_call() / internal_construct(), we can:
- Remove the generic FunctionObject parameter
- Move it from the VM to ECMAScriptFunctionObject
- Make it private
This patch implements:
- Spec compliant [[Call]] and [[Construct]] internal slots, as virtual
FunctionObject::internal_{call,construct}(). These effectively replace
the old virtual FunctionObject::{call,construct}(), but with several
advantages:
- Clear and consistent naming, following the object internal methods
- Use of completions
- internal_construct() returns an Object, and not Value! This has been
a source of confusion for a long time, since in the spec there's
always an Object returned but the Value return type in LibJS meant
that this could not be fully trusted and something could screw you
over.
- Arguments are passed explicitly in form of a MarkedValueList,
allowing manipulation (BoundFunction). We still put them on the
execution context as a lot of code depends on it (VM::arguments()),
but not from the Call() / Construct() AOs anymore, which now allows
for bypassing them and invoking [[Call]] / [[Construct]] directly.
Nothing but Call() / Construct() themselves do that at the moment,
but future additions to ECMA262 or already existing web specs might.
- Spec compliant, standalone Call() and Construct() AOs: currently the
closest we have is VM::{call,construct}(), but those try to cater to
all the different function object subclasses at once, resulting in a
horrible mess and calling AOs with functions they should never be
called with; most prominently PrepareForOrdinaryCall and
OrdinaryCallBindThis, which are only for ECMAScriptFunctionObject.
As a result this also contains an implicit optimization: we no longer
need to create a new function environment for NativeFunctions - which,
worth mentioning, is what started this whole crusade in the first place
:^)
This patch introduces the "environment coordinate" concept, which
encodes the distance from a variable access to the binding it ends up
resolving to.
EnvironmentCoordinate has two fields:
- hops: The number of hops up the lexical environment chain we have
to make before getting to the resolved binding.
- index: The index of the resolved binding within its declarative
environment record.
Whenever a variable lookup resolves somewhere inside a declarative
environment, we now cache the coordinates and reuse them in subsequent
lookups. This is achieved via a coordinate cache in JS::Identifier.
Note that non-strict direct eval() breaks this optimization and so it
will not be performed if the resolved environment has been permanently
screwed by eval().
This makes variable access *significantly* faster. :^)
VM::resolve_binding() can now return a Reference that knows the exact
binding index if it's pointing into a DeclarativeEnvironment.
Reading/writing through the Reference will now use direct environment
access when possible.
This will be used by LibWeb to squirrel away the stack while performing
a microtask checkpoint in some cases. VM will simply consider saved
execution context stacks as GC roots as well.
Before this we used an ad-hoc combination of references and 'variables'
stored in a hashmap. This worked in most cases but is not spec like.
Additionally hoisting, dynamically naming functions and scope analysis
was not done properly.
This patch fixes all of that by:
- Implement BindingInitialization for destructuring assignment.
- Implementing a new ScopePusher which tracks the lexical and var
scoped declarations. This hoists functions to the top level if no
lexical declaration name overlaps. Furthermore we do checking of
redeclarations in the ScopePusher now requiring less checks all over
the place.
- Add methods for parsing the directives and statement lists instead
of having that code duplicated in multiple places. This allows
declarations to pushed to the appropriate scope more easily.
- Remove the non spec way of storing 'variables' in
DeclarativeEnvironment and make Reference follow the spec instead of
checking both the bindings and 'variables'.
- Remove all scoping related things from the Interpreter. And instead
use environments as specified by the spec. This also includes fixing
that NativeFunctions did not produce a valid FunctionEnvironment
which could cause issues with callbacks and eval. All
FunctionObjects now have a valid NewFunctionEnvironment
implementation.
- Remove execute_statements from Interpreter and instead use
ASTNode::execute everywhere this simplifies AST.cpp as you no longer
need to worry about which method to call.
- Make ScopeNodes setup their own environment. This uses four
different methods specified by the spec
{Block, Function, Eval, Global}DeclarationInstantiation with the
annexB extensions.
- Implement and use NamedEvaluation where specified.
Additionally there are fixes to things exposed by these changes to eval,
{for, for-in, for-of} loops and assignment.
Finally it also fixes some tests in test-js which where passing before
but not now that we have correct behavior :^).
Also add the internal slot names as comments, and separate them into
groups of spec and non-spec members.
This will make it easier to compare the implementation code with the
spec, as well as identify internal slots currently missing or only
present on FunctionObject.
The old name is the result of the perhaps somewhat confusingly named
abstract operation OrdinaryFunctionCreate(), which creates an "ordinary
object" (https://tc39.es/ecma262/#ordinary-object) in contrast to an
"exotic object" (https://tc39.es/ecma262/#exotic-object).
However, the term "Ordinary Function" is not used anywhere in the spec,
instead the created object is referred to as an "ECMAScript Function
Object" (https://tc39.es/ecma262/#sec-ecmascript-function-objects), so
let's call it that.
The "ordinary" vs. "exotic" distinction is important because there are
also "Built-in Function Objects", which can be either implemented as
ordinary ECMAScript function objects, or as exotic objects (our
NativeFunction).
More work needs to be done to move a lot of infrastructure to
ECMAScriptFunctionObject in order to make FunctionObject nothing more
than an interface for objects that implement [[Call]] and optionally
[[Construct]].
'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>
This got changed in the spec at some point, replacing the assertion in
step 1 with "... and newTarget (an Object or undefined)" in the
parameter description.
Subsequently, there's now one step less, so the numbers all change.
This is where the spec wants to have it. Requires a couple of hacks as
currently everything that needs a Realm actually has a GlobalObject, so
we need to go via the Interpreter.
Instead of hardcoding the environment's global object as the return
value of GlobalEnvironment::global_this_value(), it now stores an Object
reference which is passed to the constructor for this purpose.
From the spec (https://tc39.es/ecma262/#sec-global-environment-records):
[[GlobalThisValue]] | Object | The value returned by this in global
scope. Hosts may provide any ECMAScript Object value.
The test262 tests under RegExp/property-escapes/generated will invoke
Reflect.apply with up to 10,000 arguments at a time. In LibJS, when the
call stack reached VM::call_internal, we transfer those arguments from
a MarkedValueList to the execution context's arguments Vector.
Because these types differ (MarkedValueList is a Vector<Value, 32>), the
arguments are copied rather than moved. By changing the arguments vector
to a MarkedValueList, we can properly move the passed arguments over.
This shaves about 2 seconds off the following test262 test (from 15sec):
RegExp/property-escapes/generated/General_Category_-_Decimal_Number.js
This removes all usages of the non-standard put helper method and
replaces all of it's usages with the specification required alternative
or with define_direct_property where appropriate.
This is a huge patch, I know. In hindsight this perhaps could've been
done slightly more incremental, but I started and then fixed everything
until it worked, and here we are. I tried splitting of some completely
unrelated changes into separate commits, however. Anyway.
This is a rewrite of most of Object, and by extension large parts of
Array, Proxy, Reflect, String, TypedArray, and some other things.
What we already had worked fine for about 90% of things, but getting the
last 10% right proved to be increasingly difficult with the current code
that sort of grew organically and is only very loosely based on the
spec - this became especially obvious when we started fixing a large
number of test262 failures.
Key changes include:
- 1:1 matching function names and parameters of all object-related
functions, to avoid ambiguity. Previously we had things like put(),
which the spec doesn't have - as a result it wasn't always clear which
need to be used.
- Better separation between object abstract operations and internal
methods - the former are always the same, the latter can be overridden
(and are therefore virtual). The internal methods (i.e. [[Foo]] in the
spec) are now prefixed with 'internal_' for clarity - again, it was
previously not always clear which AO a certain method represents,
get() could've been both Get and [[Get]] (I don't know which one it
was closer to right now).
Note that some of the old names have been kept until all code relying
on them is updated, but they are now simple wrappers around the
closest matching standard abstract operation.
- Simplifications of the storage layer: functions that write values to
storage are now prefixed with 'storage_' to make their purpose clear,
and as they are not part of the spec they should not contain any steps
specified by it. Much functionality is now covered by the layers above
it and was removed (e.g. handling of accessors, attribute checks).
- PropertyAttributes has been greatly simplified, and is being replaced
by PropertyDescriptor - a concept similar to the current
implementation, but more aligned with the actual spec. See the commit
message of the previous commit where it was introduced for details.
- As a bonus, and since I had to look at the spec a whole lot anyway, I
introduced more inline comments with the exact steps from the spec -
this makes it super easy to verify correctness.
- East-const all the things.
As a result of all of this, things are much more correct but a bit
slower now. Retaining speed wasn't a consideration at all, I have done
no profiling of the new code - there might be low hanging fruits, which
we can then harvest separately.
Special thanks to Idan for helping me with this by tracking down bugs,
updating everything outside of LibJS to work with these changes (LibWeb,
Spreadsheet, HackStudio), as well as providing countless patches to fix
regressions I introduced - there still are very few (we got it down to
5), but we also get many new passing test262 tests in return. :^)
Co-authored-by: Idan Horowitz <idan.horowitz@gmail.com>
Specifically, this now explicitly takes the length, adds missing
exceptions checks to calls with user-supplied lengths, takes and uses
the prototype argument, and fixes some spec non-conformance in
ArrayConstructor and its native functions around the use of ArrayCreate
ResolveBinding now matches the spec, while the non-conforming parts
are moved to GetIdentifierReference.
Implementing this properly requires variable bindings.
This is used by VM::call_internal() and VM::construct() which roughly
map to function objects' [[Call]] and [[Construct]] slots in the spec.
Reorganizing this code revealed something weird: NativeFunction gets
its strictness by checking VM::in_strict_mode(). In other words,
it inherits the strict flag from the caller context. This is quite
weird, but many test-js tests rely on it, so let's preserve it until
we can think of something nicer.
This patch adds a new ArgumentsObject class to represent what the spec
calls "Arguments Exotic Objects"
These are constructed by the new CreateMappedArgumentsObject when the
`arguments` identifier is resolved in a callee context.
The implementation is incomplete and doesn't yet support mapping of
the parameter variables to the indexed properties of `arguments`.
This patch implements the IsSimpleParameterList static semantics for
ordinary function objects.
We now also create an unmapped arguments object for callee contexts
with non-simple parameter lists, instead of only doing it in strict
mode. Covered by test262.
Our Reference class now has the same fields as the spec:
- Base (a non-nullish value, an environment record, or `unresolvable`)
- Referenced Name (the name of the binding)
- Strict (whether the reference originated in strict mode code)
- ThisValue (if non-empty, the reference represents a `super` keyword)
The main difference from before is that we now resolve the environment
record that a reference interacts with. Previously we simply resolved
to either "local variable" or "global variable".
The associated abstract operations are still largely non-conforming,
since we don't yet implement proper variable bindings. But this patch
should at least fix a handful of test262 cases. :^)
There's one minor regression: some TypeError message strings get
a little worse due to doing a RequireObjectCoercible earlier in the
evaluation of MemberExpression.
At the moment these environments are always the same as the lexical
ones, so this didn't cause any trouble. Once we start separating them
we have to make sure both environments are protected.
This matches what ECMAScript calls it. Also make it a JS::Function*
instead of a generic Value, since it will always either be a function
object or null.
We had a cached shape for environment records to make instantiating
them fast. Now that environment records don't inherit from JS::Object,
we can just get rid of this. :^)
These represent the outermost scope in the environment record
hierarchy. The spec says they should be a "composite" of two things:
- An ObjectEnvironmentRecord wrapping the global object
- A DeclarativeEnvironmentRecord for other declarations
It's not yet clear to me how this should work, so this patch only
implements the first part, an object record wrapping the global object.
To better follow the spec, we need to distinguish between the current
execution context's lexical environment and variable environment.
This patch moves us to having two record pointers, although both of
them point at the same environment records for now.
This patch adds FunctionEnvironmentRecord as a subclass of the existing
DeclarativeEnvironmentRecord. Things that are specific to function
environment records move into there, simplifying the base.
Most of the abstract operations related to function environment records
are rewritten to match the spec exactly. I also had to implement
GetThisEnvironment() and GetSuperConstructor() to keep tests working
after the changes, so that's nice as well. :^)
This patch makes the following renames:
- get_from_scope() => get_from_environment_record()
- put_to_scope() => put_into_environment_record()
- delete_from_scope() => delete_from_environment_record()
This patch makes the following name changes:
- ScopeObject => EnvironmentRecord
- LexicalEnvironment => DeclarativeEnvironmentRecord
- WithScope => ObjectEnvironmentRecord
