fix(reflect): correct return values for set/get/define/delete/extensibility - #3402
Merged
Conversation
added 4 commits
May 30, 2026 18:44
…-2756-2762 # Conflicts: # crates/perry-codegen/src/collectors/i32_locals.rs # crates/perry-codegen/src/collectors/refs.rs # crates/perry-runtime/src/object/object_ops.rs
…Escape) + extract object/mod.rs tests to tests.rs (under 2000-cap)
proggeramlug
pushed a commit
that referenced
this pull request
May 30, 2026
RegExpEscape (#3414) and ReflectIsExtensible (#3402) both landed on main with stable-hash tag 12045, tripping stable_hash::tests::expr_variant_stable_hash_tags_are_unique once this branch merged main. Reassign RegExpEscape (the newer variant) to the next free tag 12061 so every Expr variant has a distinct tag without renumbering any other variant.
proggeramlug
added a commit
that referenced
this pull request
May 30, 2026
#3409 (RegExpEscape) and #3402 (ReflectIsExtensible) both merged via a tag race, each assigning stable-hash tag 12045 — making expr_variant_stable_hash_tags_are_unique fail on main and blocking all PR CI. Reassign ReflectIsExtensible to the free tag 12048 (matches the fix already on PR #3445's branch, so no future conflict). Co-authored-by: Ralph Küpper <ralph2@skelpo.com>
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
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.
Closes #2756
Closes #2757
Closes #2758
Closes #2760
Closes #2762
Implementation
Perry's
Reflect.*helpers reported success unconditionally andReflect.getPrototypeOfreturned the target object itself. This corrects the observable return values to match Node v25, covering the ordinary (non-proxy)[[Set]]/[[Delete]]/[[DefineOwnProperty]]/extensibility semantics plus the proxy-trap return-value coercion that already had the dispatch wired.Reflect.set— returns the real boolean[[Set]]result:falsefor a non-writable existing data property or a new key on a non-extensible object; coercedset-trap result for proxies. The ordinary path is computed without throwing (unlike strict-mode assignment, test_gap_object_methods: frozen / sealed / non-extensible writes don't throw in strict mode #615) via a pre-check of recorded attrs + theOBJ_FLAG_NO_EXTENDflag.Reflect.getPrototypeOf— now delegates tojs_object_get_prototype_of(the same lookupObject.getPrototypeOfuses), returning the actual[[Prototype]]includingnullfor null-prototype objects, instead of the target itself. Codegen previously short-circuited tolower_expr(target); it now calls the runtime helper. The=== Class.prototypeconstant fold inlower_expr.rsis preserved (drizzle/effect depend on it).Reflect.defineProperty— returnsfalsewhen defining a new property on a non-extensible object or redefining a non-configurable property; coerceddefineProperty-trap result for proxies;trueon success.Reflect.deleteProperty— returns the real delete result:falsefor a non-configurable property (and the property is left in place); coerceddeleteProperty-trap result for proxies.Reflect.preventExtensions/Reflect.isExtensible— new Reflect-specific HIR variants (ReflectIsExtensible/ReflectPreventExtensions) and runtime helpers that throwTypeErroron non-object targets and return booleans (preventExtensionsreturnstrue, not the object), distinct from theObject.*helpers. ProxypreventExtensions/isExtensibletraps are dispatched and their results coerced.New reflect-support predicates (extensibility flag, own-key presence, writable/configurable attrs) were extracted into
crates/perry-runtime/src/object/reflect_support.rsto keepobject_ops.rsunder the 2000-line lint cap. The two new#[no_mangle]entry points (js_reflect_is_extensible,js_reflect_prevent_extensions) are declared inruntime_decls/objects.rsand pinned with#[used]keepalive anchors so the auto-optimize whole-program bitcode rebuild does not dead-strip them. New HIR variants use unique stable-hash tags 12043/12044.Validation
test-files/test_gap_reflect_2756_2762.tsis byte-identical tonode --experimental-strip-typesunder the default auto-optimize compile:Note on scope
The proxy-trap return paths above are implemented because the trap-dispatch plumbing already existed. The Object-prototype-chain identity case from #2757's example (
Reflect.getPrototypeOf(Object.create(proto)) === proto) is a pre-existingObject.getPrototypeOflimitation (it returns the object for plain object-literal chains) and is out of scope here — the issue explicitly scopes itself to theReflect.getPrototypeOfhelper sharingObject.getPrototypeOfsemantics, which it now does. The gap test coversgetPrototypeOf({}) === Object.prototypeand the null-prototype case, both of which match Node.