Skip to content

Commit 2e13fe2

Browse files
committed
Fix 9 broken tests: hasOwn on class instances, PIE symbol key, RegExp sync
Three independent fixes addressing the failing-test cluster: 1. HasOwnPropertyHelper: add $IHasFields branch for user-defined class instances. Regression from 1866d2f which unified ObjectHasOwn into HasOwnPropertyHelper but lost the $IHasFields → Fields.ContainsKey path. Object.hasOwn(p, "name") on a class instance silently returned false. New branch dispatches through the per-class HasProperty (compile-time backing-field + _fields check) with a PDS fallback for defineProperty-installed descriptors. 2. PropertyIsEnumerableHelper symbol-key path: remove the broken PDSGetPropertyDescriptor call. PDS is string-keyed; the call was passing the Symbol object as the string `name` parameter, which ILVerify flagged as StackUnexpected and would have thrown at runtime if a defineProperty-installed symbol descriptor ever surfaced. Symbol keys go through the per-receiver symbol dict only — spec-default enumerable=true per ECMA-262 §17 for plain `obj[sym] = v` assignments. 3. RuntimeTypeSyncTests SharpTSRegExp ignored list: add DefineAccessor and TryGetAccessor. These are interpreter-only (added in ad935b8 for ECMA-262 §22.2 configurable-accessor parity); compiled $RegExp routes defineProperty accessors through the central PropertyDescriptorStore, so emitting equivalents would be dead code. Test results: 10,321/10,321 passing (was 10,312 with 9 failures).
1 parent a9877cd commit 2e13fe2

2 files changed

Lines changed: 45 additions & 19 deletions

File tree

Compilation/RuntimeEmitter.HasOwnProperty.cs

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ public partial class RuntimeEmitter
1818
/// - $TSFunction: name is "name"/"length" (always cached) or PDS has a
1919
/// user-defined descriptor for it.
2020
/// - $Object: name is in _fields or _getters.
21+
/// - $IHasFields (user-defined class instances): per-class HasProperty
22+
/// knows typed backing fields + the dynamic _fields dict.
2123
/// - Dictionary: ContainsKey(name).
2224
/// - List/$Array: name is "length" or a numeric index in range.
2325
/// - String: numeric index in [0,len) or "length".
@@ -124,6 +126,32 @@ private void EmitHasOwnPropertyHelper(TypeBuilder typeBuilder, EmittedRuntime ru
124126
il.Emit(OpCodes.Br, falseLabel);
125127
il.MarkLabel(notTSObject);
126128

129+
// $IHasFields branch — user-defined class instances. Each compiled
130+
// class implements $IHasFields with a per-class HasProperty that
131+
// checks typed backing field names + the dynamic _fields dict
132+
// (see ILCompiler.Classes.HasFields.EmitHasPropertyBodyCore).
133+
// PDS fallback covers `Object.defineProperty(instance, name, ...)`.
134+
//
135+
// Must run after $TSObject (which also implements $IHasFields but
136+
// has its own richer dispatch via TSObjectHasProperty) and before
137+
// the Dictionary branch (a class instance isn't a Dictionary, but
138+
// ordering here documents the precedence).
139+
var notIHasFieldsLabel = il.DefineLabel();
140+
il.Emit(OpCodes.Ldarg_0);
141+
il.Emit(OpCodes.Isinst, runtime.IHasFieldsInterface);
142+
il.Emit(OpCodes.Brfalse, notIHasFieldsLabel);
143+
il.Emit(OpCodes.Ldarg_0);
144+
il.Emit(OpCodes.Castclass, runtime.IHasFieldsInterface);
145+
il.Emit(OpCodes.Ldloc, nameLocal);
146+
il.Emit(OpCodes.Callvirt, runtime.IHasFieldsHasProperty);
147+
il.Emit(OpCodes.Brtrue, trueLabel);
148+
il.Emit(OpCodes.Ldarg_0);
149+
il.Emit(OpCodes.Ldloc, nameLocal);
150+
il.Emit(OpCodes.Call, runtime.PDSGetPropertyDescriptor);
151+
il.Emit(OpCodes.Brtrue, trueLabel);
152+
il.Emit(OpCodes.Br, falseLabel);
153+
il.MarkLabel(notIHasFieldsLabel);
154+
127155
// Dictionary<string,object> — own property is either a direct key in
128156
// the backing dict (\"foo\" = 1 syntax) OR a PDS-stored descriptor
129157
// (Object.defineProperty path, which doesn't write to _fields). Both
@@ -429,28 +457,20 @@ private void EmitPropertyIsEnumerableHelper(TypeBuilder typeBuilder, EmittedRunt
429457
il.Emit(OpCodes.Throw);
430458
il.MarkLabel(pieAfterNullLabel);
431459

432-
// Symbol-keyed lookup: same routing as HasOwnPropertyHelper.
460+
// Symbol-keyed lookup. PDSGetPropertyDescriptor's name parameter is
461+
// string-typed, so symbol keys can't live there — defineProperty with
462+
// a symbol key writes to the per-receiver symbol dict (GetSymbolDict).
463+
// Presence in that dict is the spec-default enumerable=true for plain
464+
// `obj[sym] = v` assignments; an earlier attempt to honor PDS-installed
465+
// descriptors here passed the symbol as the string `name`, which IL-
466+
// Verify flagged (StackUnexpected) and would have thrown at runtime if
467+
// the path ever ran with a defineProperty-installed symbol descriptor.
468+
// ECMA-262 §17 specifies enumerable=true as the default for own data
469+
// properties of ordinary objects.
433470
var pieNotSymbolLabel = il.DefineLabel();
434471
il.Emit(OpCodes.Ldarg_1);
435472
il.Emit(OpCodes.Call, runtime.IsSymbolMethod);
436473
il.Emit(OpCodes.Brfalse, pieNotSymbolLabel);
437-
// For Symbol keys: lookup descriptor in PDS (Object.defineProperty-
438-
// installed descriptors hold the Enumerable bit). If missing, fall
439-
// back to whether the symbol exists in GetSymbolDict (then default
440-
// enumerable=true for plain `obj[sym] = v` assignments).
441-
var pieSymPdsLocal = il.DeclareLocal(runtime.CompiledPropertyDescriptorType);
442-
var pieSymNoPdsLabel = il.DefineLabel();
443-
il.Emit(OpCodes.Ldarg_0);
444-
il.Emit(OpCodes.Ldarg_1);
445-
il.Emit(OpCodes.Call, runtime.PDSGetPropertyDescriptor);
446-
il.Emit(OpCodes.Stloc, pieSymPdsLocal);
447-
il.Emit(OpCodes.Ldloc, pieSymPdsLocal);
448-
il.Emit(OpCodes.Brfalse, pieSymNoPdsLabel);
449-
il.Emit(OpCodes.Ldloc, pieSymPdsLocal);
450-
il.Emit(OpCodes.Callvirt, runtime.CompiledPropertyDescriptorEnumerable.GetGetMethod()!);
451-
il.Emit(OpCodes.Ret);
452-
il.MarkLabel(pieSymNoPdsLabel);
453-
// No PDS: check symbol dict. Present → enumerable by default; absent → false.
454474
il.Emit(OpCodes.Ldarg_0);
455475
il.Emit(OpCodes.Call, runtime.GetSymbolDictMethod);
456476
il.Emit(OpCodes.Ldarg_1);

SharpTS.Tests/Compilation/RuntimeTypeSyncTests.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,13 @@ public void Dispose()
9797
[.. BaseIgnored, "SetFromEpochMilliseconds", "ToLocalTime"]),
9898

9999
new(typeof(SharpTSRegExp), "RegExp",
100-
[.. BaseIgnored]),
100+
[.. BaseIgnored,
101+
// Per-instance accessor map (interpreter-only path). Compiled $RegExp
102+
// routes defineProperty accessors through the central PropertyDescriptor
103+
// Store (PDS), so emitting equivalent instance methods would just be
104+
// dead code. See SharpTSRegExp.DefineAccessor / TryGetAccessor
105+
// (added in ad935b84 for ECMA-262 §22.2 configurable-accessor parity).
106+
"DefineAccessor", "TryGetAccessor"]),
101107

102108
new(typeof(SharpTSError), "Error",
103109
[.. BaseIgnored, "GetMember"]),

0 commit comments

Comments
 (0)