Implement the Record and Tuple primitives based on objects
source link: https://bugzilla.mozilla.org/show_bug.cgi?id=1730843
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
Implement the Record and Tuple primitives based on objects
Categories
(Core :: JavaScript Engine, enhancement, P3)
Tracking
(ASSIGNED bug which is in the backlog of work)
People
(Reporter: nicolo.ribaudo, Assigned: nicolo.ribaudo)
Details
Attachments
(22 files, 6 obsolete files)
+++ This bug was initially created as a clone of Bug #1658309 +++
https://github.com/tc39/proposal-record-tuple
I am opening this bug to track an alternative implementation for Bug #1658309 - "Implement the Record and Tuple proposal". Rather than implementing them as three custom new primitive types, they can be implemented as if they were objects but still be exposed to the JS code as if they were primitives.
This is an alternative implementation to D87232. I didn't implement
object wrappers for these new primitive-like-objects yet, sotypeof Object(rec)
is currently "record"
and not "object"
.
They are internally implemented using as dense elements. After that the tuple is fully initialized it's frozen to prevent modifications.
Depends on D125644
This patch adds #{} and #[] support to the parser and to Reflect.parse.
The AST has the same shapre of object and array literals, but the node
names are "RecordExpression" and "TupleExpression".
Since this patch only implements parser support and not bytecode emitter
support, any attempt of evaluating those literals will crash. This is why
I have only used Reflect.parse in the tests.
The ParseNode class sets the hasNonConstInitializerBit
flag for records
and tuples, since this will allow us to use a single record/tuple instance
if it's constant but evaluated multiple times.
This patch is effectively the same as D87586
Depends on D125645
This patch introduces three new op codes:
InitTuple length
preallocates a tuple of the given length: this is the final length if the
tuple doesn't contain spreads, otherwise it's the most pessimisitc choice possible
(i.e. spreads are assumed to introduce no new elements). The preallocated tuple size
can still grow if needed.AddTupleElement
pushes the last value in the stack to the tuple (the second-last stack element)FinishTuple
marks the tuple as initialized. An unfinished tuple should never leak to JavaScript
code, and any attempt to get its element will fail.
The bytecode relative to tuple spread is similar to the bytecode used for array spread, emitting
op codes to call the different steps of the iterator protocol.
Depends on D125646
This patch implements the SameValueZero
(===
) and SameValue
(Object.is
) algorithms for tuples, and adds the equivalent functions for records (they will crash when called).
Depends on D125647
This patch adds support for properties in records, when created using the Record({})
function.
Record properties are stored as normal object properties, but there is
an additional sortedKeys
internal slot containing an array of all the record
keys sorted alphabetically. This will be necessary when comparing two records
for equality, or when enumerating their properties.
Depends on D125649
This patch defines the bytecode for record literals, similarly to how it has been implemented for tuple literals:
InitRecord [length]
pushes a new record to the stack, allocating
memory forlength
propertiesAddRecordProperty
reads the key and the value from the stack,
and defines it on the record which is being initializedFinishRecord
freezes a record and sorts its keys
This patch doesn't implement support for spread in record literals yet.
Depends on D125650
This is done introducing a new opcode, AddRecordSpread
, that
takes a value and defines its enumerable properties on the record
which is currently being initialized.
Depends on D125651
This reuses the same implementation strategy used for tuples
Depends on D125652
This patch implements the third new type introduced by the Records and Tuples proposal: Box
.
It implements:
- the
"box"
primitive type, implemented as an object - the
Box
function and its instance methods - boxes equality
Still missing:
- support for boxes as
Map
andSet
keys Box.containBoxes
(this function will likely change in the proposal)
Depends on D125654
This patch fixes Object.keys
in records to return sorted keys, and adds
tests for the other Object.*
methods.
This patch fixes the missing pieces to make enumeration and object
operations work with tuples (they need to be handled
specially in some places because of the contextual prototype).
Depends on D125656
This patch implements JSON.stringify
as specified in the Record and Tuple proposal.
Due to the Box unwrapping logic, balancing between "don't duplicate code" and "keep the R&T code isolated behind the ENABLE_RECORD_TUPLE
flag" was tricky: I ended up extracting the code to call toJSON
to a separate funcition even
if without the R&T proposal it's used only once.
Depends on D125657
When preparing records, tuples and boxes to be used as a Map/Set key, we
recursively atomize the strings that they contain. This makes it
possible to compute a hash for the whole structure, and to compare them
with an infallible function.
ToDo: Figure out where to store the isAtomized flag for tuples
Depends on D125659
This is in preparation of the next patch which will introduce object wrappers
Depends on D125660
Even if they are still implemented as subclasses of JSObject
, representing
them differently in the Value
wrapper has a few advantages:
- Checking the equality semantics of an object (deep vs pointer) is faster,
since we only need to check theValue
's bits. - It's easier to differentiate between R/T/B and R/T/B object wrappers,
since.isObject()
returnsfalse
for the "primitives". For example,
this patch fixes thetypeof Object(#{})
tests.
This approach's advantages are the same as what we would get implementing them
as primitives from scratch (https://phabricator.services.mozilla.com/D87232),
but it also allows us to re-use the existing objects optimizations.
Depends on D126691
Even if R&T are internally implemented as objects, they are still
primitives and they should be have object wrappers:
typeof Tuple() === "tuple"
typeof Object(Tuple()) === "object"
Depends on D125644
This patch fixes and tests handling of property descriptors for record properties
Depends on D125650
This patch fixes the missing pieces to make object operations work with tuples.
Depends on D129675
The proposal currently specifies R/T/B wrappers as frozen.
This patch makes them look as if they were frozen, while still internally
keeping them marked as extensible. This allows lazily resolving the
wrapper properties (so that we can avoid a full copy every time we
create the wrapper object), but they are exposed as non-extensible to JS
code.
It's still possible that the proposal will switch to make them mutable;
you can follow the discussion (and participate!) at
https://github.com/tc39/proposal-record-tuple/issues/137.
Depends on D125660
Hi Anba,
In this patch, Nicolò has encountered some oddity with make_intl_data.py
and the resulting headers. Would you mind taking a peek to give a hint on what's going on; I don't really understand what's going on here.
Sorry about those bugs. I've filed bug 1741220 and bug 1741222 to fix this.
This is done by not using the NativeGetProperty
and NativeDeleteProperty
functions for R/T/B, but by creating two new "parallel" functions that handle them specifically.
This is because R/T/B don't behave like classic objects, but they get their prototype lexically (like other primitives).
It's not necessary to reimplement NativeSetProperty
, because it always throw since R/T/B are frozen.
Depends on D131124
Since tuples use inline elements, they cannot have internal slots and thus we cannot store the IS_ATOMIZED
flag as we do for records and tuples. However, we can store it in a bit of the ObjectElements
flags.
Depends on D132367
The code for converting records and objects to strings would be almost identical:
to avoid dynamic branches I converted them to if constexpr
statements.
I also found that we didn't have a test for boxed R&T (only for boxes Box).
Ref https://phabricator.services.mozilla.com/D132367#inline-730448
Depends on D132367
This patch fixes a regression introduced by the --enable-record-tuple
build flag, which made
https://searchfox.org/mozilla-central/source/js/src/tests/non262/template-strings/bug1559123.js
crash because the IsExtendedPrimitive
doesn't work with forwarded objects.
This patch introduces a new gc::MaybeForwardedIsExtendedPrimitive
function, similar to
the existing gc::MaybeForwardedObjectIs
one.
Depends on D132393
Comment on attachment 9253022 [details]
Bug 1730843 - Part 20 - Run Records&Tuples tests on ci
Revision D132467 was moved to bug 1744967. Setting attachment 9253022 [details] to obsolete.
After the feedback received at the Dec 2021 TC39 meeting, it's very unlikely
that the proposal will include Box
. This patch removes it so that the
rest of the R&T implementation can be merged.
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK