Fix #475: Object.keys/values/entries/for-in hide internal slots + respect enumerability#802
Merged
Conversation
…pect enumerability
Boxed primitive wrappers (new String/Number/Boolean) store their internal slots
__primitiveType/__primitiveValue as ordinary fields, and the interpreter's
Object.keys/values/entries and for-in enumerated obj.Fields directly — leaking
those slots and ignoring per-property enumerability.
- Add SharpTSObject.OwnEnumerableKeys() (data fields minus internal slots,
honoring enumerability), shared by Object.keys/values/entries and both for-in
paths so they stay consistent (fixes the keys-vs-for-in mismatch on a boxed
String wrapper). A String exotic's `length` is marked non-enumerable.
- Object.defineProperty now preserves an existing property's writable/enumerable/
configurable attributes when the descriptor omits them (ECMA-262 10.1.6.3),
via PreserveOmittedAttributes — attribute presence is read prototype-aware.
Previously a partial redefine (e.g. {writable:false}) reset the others to
false, dropping the property from enumeration.
Net +96 interpreted Test262 (Object/create, Object/defineProperties,
Object/defineProperty flag-preservation, Object/keys, for-in). One inherent
regression — Object/defineProperty/15.2.3.6-3-40-1 — from a separate gap (a
descriptor object's INHERITED attributes aren't read through its prototype);
tracked as a follow-up. The data-value-preservation half (FromObject defaults an
omitted `value` to the undefined sentinel) is deferred to that same follow-up
since it depends on prototype-aware descriptor reads.
Full suite green (13419).
Enumeration + defineProperty attribute-preservation (#475) flip 96 interpreted Test262 tests to Pass (Object/create, Object/defineProperties, Object/defineProperty flag-preservation, Object/keys, for-in). One inherent regression recorded: Object/defineProperty/15.2.3.6-3-40-1 (Pass->Fail) — descriptor inherited-attribute read gap, tracked in #801. Isolated by regenerating interpreted.txt on main vs this branch; only these 97 lines changed. Compiled baseline unchanged (interp-only fix).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The enumeration half of the boxed-primitive cluster, done with the two companion fixes that were missing when it was pulled from #800.
Problem
Boxed wrappers (
new String/Number/Boolean) store their internal slots__primitiveType/__primitiveValueas ordinary fields, and the interpreter'sObject.keys/values/entriesandfor-inenumeratedobj.Fieldsdirectly — leaking those slots and ignoring per-property enumerability.Fix
SharpTSObject.OwnEnumerableKeys()— own data fields minus the internal slots, honoring enumerability — shared byObject.keys/values/entriesand bothfor-inpaths so they stay consistent (the keys-vs-for-in mismatch on a boxed String wrapper was the original regression that pulled this from Fix #708, #574, #565: boxed-primitive wrapper coercion #800). A String exotic'slengthis now non-enumerable.Object.definePropertypreserves omitted attributes — when a partial descriptor (e.g.{ writable:false }) redefines an existing property, itswritable/enumerable/configurableare preserved per ECMA-262 §10.1.6.3 instead of reset to false (which had been dropping properties from enumeration). Attribute presence is read prototype-aware.Verification
Object/create,Object/defineProperties,Object/definePropertyflag-preservation,Object/keys, andfor-in.Object/defineProperty/15.2.3.6-3-40-1(recorded in the baseline). Makingfor-inrespect enumerability — required to fix Boxed primitive wrappers leak __primitiveType/__primitiveValue into enumeration; interp Object.keys ignores enumerability #475 — collides with a separate pre-existing gap: a descriptor object's inherited attributes aren't read through its prototype (hereRegExp.prototype.enumerable). Filed as Object.defineProperty/ToPropertyDescriptor doesn't read a descriptor object's INHERITED fields through its prototype #801. The data-value-preservation bonus (~120 moreObject/definePropertyvalue tests) depends on the same prototype-aware descriptor reads and is deferred to Object.defineProperty/ToPropertyDescriptor doesn't read a descriptor object's INHERITED fields through its prototype #801.Compiled baseline unchanged (interpreter-only fix).