Closed Bug 1653567 Opened 1 year ago Closed 1 year ago

Redesign Private Fields implementation to reduce complexity

Categories

(Core :: JavaScript Engine, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
81 Branch
Tracking Status
firefox81 --- fixed

People

(Reporter: mgaudet, Assigned: mgaudet)

References

(Regressed 1 open bug)

Details

Crash Data

Attachments

(1 file)

After looking at implementing the existing Private Fields support for Ion, I came to the conclusion that what I'd built was too complex and didn't share enough functionality with our pre-existing codebase.

Given my increasing level of comfort with the front end, I was able to find a new design that's appreciably simpler, while also easing the (upcoming) implementation of both private methods and ergnomic brand checks.

Adds CheckPrivateField, and uses it to guard private field access before
calling regular element ops, rather than having custom element ops

CheckPrivateField takes two arguments: A ThrowCondition, a message, then using
the top two elements of the stack determines if a throw is required, and if it
is not, places on top of the stack whether or not the field exists.

This design is nice because it means that we'll be able to easily use this
opcode for implementing private methods and ergonomic brand checks for private
fields.

This patch implements the new opcode, deletes the old PrivateElem ops,
and provides Baseline callVM support. It's future work to provide IC, Ion and
Warp support.

Assignee: nobody → mgaudet
Status: NEW → ASSIGNED
Severity: -- → N/A
Priority: -- → P2
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aa11e196e0c7
Redesign Private Fields implementation to reduce complexity r=jorendorff
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch

A non-unified build warns about CanAttachPrivateInitIC being unused. Also \o/

Flags: needinfo?(mgaudet)
Regressions: 1655973
Flags: needinfo?(mgaudet)
Blocks: 1656351
Duplicate of this bug: 1656701
Crash Signature: [@ js::GetOwnPropertyDescriptor]
Regressions: 1708812
You need to log in before you can comment on or make changes to this bug.