Skip to content

Host objects: the enumeration hook is the one enumeration uses, and two names say what they do - #3461

Merged
lahma merged 4 commits into
sebastienros:mainfrom
lahma:v5-b5-enumeration-hook-and-unchecked-defines
Aug 27, 2026
Merged

lahma merged 4 commits into
sebastienros:mainfrom
lahma:v5-b5-enumeration-hook-and-unchecked-defines

Conversation

@lahma

@lahma lahma commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Two names that lied, in one theme.

1. GetOwnProperties is not the enumeration hook — so it is not a hook any more

Object.keys / values / entries, for..in, object spread and rest, Object.assign, JSON.stringify and JsonSerializer all list keys through GetOwnPropertyKeys and filter them with ProbeOwnProperty. None of them called GetOwnProperties, whose name is the one that reads like the hook. Its real callers were the CLR conversion path (ToObject under Options.Interop.CreateClrObject), GetSmallestIndex, the debugger's GetAllBindingNames and the debug view.

So the trap ran both ways. A host that overrode GetOwnProperties shipped an object script could not enumerate — a real integrator did exactly that. A host that did the right thing and overrode GetOwnPropertyKeys + ProbeOwnProperty shipped one whose properties ToObject() and the debugger could not see.

GetOwnProperties is now derived and non-virtual: keys from GetOwnPropertyKeys, each descriptor from GetOwnProperty, a key whose descriptor is absent skipped. One pair of overrides answers everything.

Every override audited, and what the derived version answers for each

Thirteen in-box overrides removed. Eleven were already "the same order as GetOwnPropertyKeys, with the descriptors materialized" — several said exactly that in their own doc comments.

Override Derived answer
ArrayInstance identical — indices, length, bag, symbols; GetOwnProperty materializes the same _sparse descriptors the override did
JsRegExp identical — lastIndex, then base
IteratorResult identical — value, done, then base; GetOwnProperty caches the same two descriptors
JsArguments identical — the override was EnsureInitialized() + base, and its GetOwnPropertyKeys override already does that
JsError identical keys, identical descriptor; the derived route no longer materializes the virtual message into the bag as a side effect of being enumerated
ArrayLikeObject, NamedPropertyObject, JsStorage, JsEvent identical — each was literally the key order with descriptors materialized
ObjectWrapper identical — its body was already foreach (key in EnumerateOwnPropertyKeys) yield (key, GetOwnProperty(key))
Function order change — see below
StringInstance content and order change — see below
Function.ObjectInstanceWithConstructor would have lost constructor — fixed on the key side, see below

Test and benchmark overrides removed with them: EnvelopeHostObject, ProjectedHostObject, and HostObjectAccessBenchmark's projected host — all three already agreed with their own GetOwnPropertyKeys.

Order changes, decided deliberately

In every case the derived order is the one Object.getOwnPropertyNames already reported, i.e. the specification's [[OwnPropertyKeys]] order:

  • a function: was prototype, length, name, [arguments, caller], bag → now length, name, prototype, [arguments, caller], bag. That is the order §10.2 creates them in, and the order the key enumeration always used.
  • a String object: was bag, symbols, length → now "0"…"n-1", length, bag, symbols. The character indices are own properties of a String object; the override never reported them.
  • a plain object with integer-like keys: was storage order → now integer indices ascending, then strings in insertion order, then symbols.

One in-box object had the exact defect this change exists to prevent

A lazily created f.prototype (Function.ObjectInstanceWithConstructor) keeps constructor in a field and declared it to GetOwnProperties alone. On main today:

function f() {}
f.prototype.hasOwnProperty("constructor")                       // true
Object.getOwnPropertyDescriptor(f.prototype, "constructor")     // { ... }
Object.getOwnPropertyNames(f.prototype)                         // []   <-- wrong
Reflect.ownKeys(f.prototype)                                    // []   <-- wrong

Deriving GetOwnProperties from the key list would have propagated that omission rather than exposing it, so the fix belongs on the key side: it declares constructor through GetInitialOwnStringPropertyKeys, and both now answer ["constructor"]. It stays non-enumerable, so Object.keys and for..in are unchanged.

Two callers dropped it entirely

GetSmallestIndex and the debugger's GetAllBindingNames only ever wanted keys, so they ask GetOwnPropertyKeys(Types.String). Strictly less work — no descriptor per key, and for a dense array GetSmallestIndex no longer materialized a CEW descriptor per element into _sparse — and it drops symbols from DebugScope.BindingNames, where a symbol was never a binding name.

One more debugger fix the new test forced out

ObjectEnvironment.HasBindings() read the engine's property tables directly, so a with scope over a host object whose properties live outside them was reported as empty and dropped from the scope chain. It now falls back to asking the object, after the two cheap storage tests that answer every in-box case without asking anything.

2. Two names that describe an implementation, and a misleading one

4.16.x 5.x
FastSetProperty(string, PropertyDescriptor) DefineOwnPropertyUnchecked(string, PropertyDescriptor)
FastSetProperty(JsValue, PropertyDescriptor) DefineOwnPropertyUnchecked(JsValue, PropertyDescriptor)
FastSetDataProperty(string, JsValue) DefineOwnDataPropertyUnchecked(string, JsValue)

Same bodies, mechanical rename, and the compiler finds every call site. The old doc comments spent a paragraph explaining that the methods are not fast; the new names say what they are — [[DefineOwnProperty]] with the checks taken out. Always an own property, so it shadows the prototype chain; no inherited setter runs; no validation runs, so the call can never raise a TypeError; and a raw descriptor under a string key deopts a shape-mode receiver to dictionary mode permanently. ObjectInstance.Fast.cs is ObjectInstance.Unchecked.cs.

Performance

GetOwnProperties is on the CLR conversion path, and deriving it does cost something. No measured number here — I did not run a gated benchmark — so, precisely:

  • Added per call: one List<JsValue> and its backing array (about 56 + 8·N bytes on 64-bit), from GetOwnPropertyKeys.
  • Added per key: one virtual GetOwnProperty call plus one hash lookup, where the old dictionary path stepped an enumerator that already held the descriptor. The JsStrings and the PropertyDescriptors are the same objects either way.
  • Removed per call for a shape-mode receiver: the scratch Key[slotCount] the old CollectKeys needed. Roughly a wash there.
  • ObjectWrapper additionally materializes its key list instead of streaming it — one List<JsValue> on ToObject() of a wrapped object.
  • Net negative for the two callers that left: GetSmallestIndex and the debugger's binding names materialize no descriptors at all now, where they used to materialize one per key.

Nothing on a script-visible hot path moved: no script enumeration ever went through this method, and none does now.

Tests

Jint.Tests.PublicInterface/HostEnumerationHookTests.cs — a host declaring only GetOwnPropertyKeys + ProbeOwnProperty is enumerable by Object.keys, for..in, spread, Object.assign and JSON.stringify and through GetOwnProperties, ToObject() and DebugScope.BindingNames; plus the order pin, the absent-descriptor filter, a reflection pin that the method is not virtual, and the f.prototype regression.

Against unmodified main, 8 of its 9 tests fail — the ninth is the control, script enumeration, which already worked:

Failed AFunctionPrototypeReportsTheConstructorItOwns
  Expected engine.Evaluate("function f() {} Object.getOwnPropertyNames(f.prototype).join(',')")
  to be "constructor (String)", but found " (String)".
Failed TheKeyHooksAloneMakeAHostEnumerableThroughGetOwnProperties
  Expected collection to be equal to {"zulu=z", "alpha=a", "7=seven"}, but found empty collection.
Failed TheKeyHooksAloneMakeAHostConvertibleToAClrObject
  Expected converted!.Keys to be equal to {"zulu", "alpha", "7"}, but found empty collection.
Failed TheKeyHooksAloneMakeAHostVisibleToTheDebugger
  System.InvalidOperationException : Sequence contains no matching element
Failed GetOwnPropertiesIsNotAnExtensionPoint
  Expected boolean to be False, but found True.
... and AKeyWhoseDescriptorIsAbsentIsNotReported, AnExpandoWrittenByScriptJoinsTheSameEnumeration,
    GetOwnPropertiesReportsTheOrderGetOwnPropertyKeysReports

Failed!  - Failed: 8, Passed: 1, Skipped: 0, Total: 9   (net472, net8.0 and net10.0 alike)

Jint.Tests.PublicInterface/HostUncheckedDefineTests.cs — pins the four promises the new names make. It is new coverage rather than a regression net: the rename changes no behaviour, so these pass either way.

Verification

  • dotnet build -c Release (solution) and dotnet test -c Release: green, no warnings.
  • Jint.Tests.PublicInterface with JINT_HOST_CONTRACT_VERIFICATION=1: green on all three TFMs.
  • test262: 102,495 passed / 0 failed / 189 skipped — the control figure exactly.
  • Five regenerated public-API baselines, and docs/v5-migration.md §2 (table row plus 2.6), 3.16 and 4.44, all in this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S

@lahma

lahma commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto latest main and renumbered, because three PRs in flight collided on the same sections.

§2.6 and §3.16 were also claimed by #3459 (limits as properties), and §4.44 by #3429 (calendar arithmetic). Assigning a definite merge order — #3459 and #3458, then #3429, then this — puts this PR at:

was now
§2.6 §2.7
§3.16 §3.17
§4.44 §4.45

Anchors and the one cross-reference repointed with them.

Two other things fixed in the rebase:

Build clean, and HostEnumerationHookTests + HostUncheckedDefineTests green (14/14 on net10.0).

@lahma
lahma force-pushed the v5-b5-enumeration-hook-and-unchecked-defines branch 5 times, most recently from e213d62 to c5eecdf Compare August 27, 2026 04:32
lahma and others added 4 commits August 27, 2026 07:52
…wo names say what they do

`ObjectInstance.GetOwnProperties` was a second `virtual` whose name read like the
enumeration hook and which nothing script-visible called. It is derived from
`GetOwnPropertyKeys` + `GetOwnProperty` now, and non-virtual, so a host declares
its keys once and every consumer — script, the CLR conversion behind `ToObject()`,
the debugger, the debug view — reads the same answer.

Thirteen in-box overrides go with it. Two of them disagreed with their own key
enumeration: a function reported `prototype` first where `GetOwnPropertyKeys`
reports `length`, `name`, `prototype`, and a `String` object omitted its character
indices. A lazily created `f.prototype` declared `constructor` to `GetOwnProperties`
alone, so `Object.getOwnPropertyNames(f.prototype)` answered `[]`; it now declares
it through `GetInitialOwnStringPropertyKeys` and answers `["constructor"]`.

`FastSetProperty` / `FastSetDataProperty` are `DefineOwnPropertyUnchecked` /
`DefineOwnDataPropertyUnchecked`. The old names claimed a speed the methods do not
have — a loop of them is the slow way to project host records — and hid what they
do: always an own property, shadowing the prototype chain, no inherited setter, no
`[[DefineOwnProperty]]` validation and therefore no possible `TypeError`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
sebastienros#3429 merged with 4.44 while this branch carried 4.45, and the rebase kept
this section where it had been -- so the file read 4.43, 4.45, 4.44 and the
ordering check added by sebastienros#3454 failed:

    line 2217: 4.44 follows 4.45 at line 2187

The numbers are right; the position was not. Moved this section to the end
of part 4 rather than renumbering, so the section that merged first keeps
the number it merged with. That is the rule the guide states, applied the
way round it is meant: whoever lands second moves.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014W5mbjGhyvgAS4pivXoc4S
@lahma
lahma force-pushed the v5-b5-enumeration-hook-and-unchecked-defines branch from ade36b0 to f061f4f Compare August 27, 2026 04:52
@lahma
lahma merged commit b7fed7a into sebastienros:main Aug 27, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant