Skip to content

fix(runtime): OrdinarySet rejects a receiver-owned accessor instead of firing its setter#6063

Merged
proggeramlug merged 1 commit into
mainfrom
fix/t262-mech-ordinaryset-accessor
Jul 6, 2026
Merged

fix(runtime): OrdinarySet rejects a receiver-owned accessor instead of firing its setter#6063
proggeramlug merged 1 commit into
mainfrom
fix/t262-mech-ordinaryset-accessor

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

OrdinarySetWithOwnDescriptor (sec-ordinarysetwithowndescriptor) step 2 handles the case where the descriptor found during the OrdinarySet walk is a data property (or absent → CreateDataProperty) and the write must land on a Receiver that differs from the object walked. In that tail the Receiver's own descriptor decides the outcome:

Receiver own descriptor result
accessor (2.d.i) return false — setter NOT called
non-writable data (2.d.ii) return false
writable data (2.d.iii) redefine { value }
none (2.e) CreateDataProperty

Perry's create_or_update_receiver_property tail invoked the receiver's setter for the accessor sub-case (2.d.i) and returned true. So a Reflect.set(target, k, v, receiverWithSetter) where target[k] is a data property mis-fired the receiver's setter and reported success. The receiver's setter must only run when it is the descriptor found by the OrdinarySet walk itself — served by the separate Accessor arm in ordinary_set_with_receiver, never through this CreateDataProperty-on-receiver tail. The accessor sub-case now returns false without calling the setter.

var target = { x: 1 };
var setterCalled = 0;
var receiver = { set x(v) { setterCalled++; } };
Reflect.set(target, "x", 2, receiver);  // before: true / setterCalled 1 ; node & now: false / 0

Validation

Built and tested on an internal Linux sweep host (--release).

  • test262 built-ins/Reflect + Proxy + Object (3875 cases): byte-identical pass/fail between baseline and patched — 0 regressions.
  • built-ins/TypedArray + TypedArrayConstructors (2184 cases): 0 regressions; combined with the typed-array [[Set]] fix merged in fix(runtime): typed-array integer-indexed [[Set]] spec rejection + coercion #6059 this flips Set/key-is-valid-index-reflect-set.js to passing (a typed-array element source with an accessor-bearing receiver — the exact data-source / accessor-receiver shape above). On its own the fix touches no other test262 verdict in these dirs but is a prerequisite for that case and is spec-correct.

cargo fmt --all -- --check clean; address-classification audit passes; proxy.rs stays under 2000 lines.

Summary by CodeRabbit

  • Bug Fixes
    • Adjusted property update behavior for receiver-owned accessor properties so they are no longer treated as assignable in this path.
    • Prevents attempts to write through an accessor setter when the receiver already owns that property, returning a failure result instead.

…wned accessor instead of firing its setter

OrdinarySetWithOwnDescriptor step 2.d.i: when the descriptor found during
the OrdinarySet walk is a data property (or absent, meaning
CreateDataProperty) and the write must land on a Receiver that differs
from the object walked, a RECEIVER-owned accessor makes the algorithm
return false WITHOUT invoking the setter. The receiver's setter only fires
when it is the descriptor found by the walk itself (the Accessor arm in
`ordinary_set_with_receiver`), never through this
CreateDataProperty-on-receiver tail.

`create_or_update_receiver_property` previously called the receiver's
setter for that sub-case and returned true, so
`Reflect.set(target, k, v, receiverWithSetter)` where `target[k]` is a
data property mis-fired the receiver's setter and reported success.

Together with the typed-array `[[Set]]` fix in the previous commit this
makes test262
built-ins/TypedArrayConstructors/internals/Set/key-is-valid-index-reflect-set
pass (a typed-array element source, an accessor-bearing receiver). Verified
on an internal Linux sweep host with zero regressions across
built-ins/Reflect + Proxy + Object (3875 cases) and the broad TypedArray
suite.
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b3b7a9c9-3777-4c12-ad3f-0835ff795be0

📥 Commits

Reviewing files that changed from the base of the PR and between ca0e895 and 404ab65.

📒 Files selected for processing (1)
  • crates/perry-runtime/src/proxy.rs

📝 Walkthrough

Walkthrough

In create_or_update_receiver_property, the OwnSetDescriptor::Accessor match arm no longer invokes call_setter_with_receiver; it now returns false immediately, treating a receiver-owned accessor descriptor as non-settable via this path.

Changes

Proxy Set Path Fix

Layer / File(s) Summary
Receiver accessor descriptor handling
crates/perry-runtime/src/proxy.rs
The OwnSetDescriptor::Accessor branch in create_or_update_receiver_property no longer calls the setter and returns false immediately instead.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • PerryTS/perry#5164: Modifies the same OwnSetDescriptor::Accessor handling in create_or_update_receiver_property to reject setter-less accessor paths.
  • PerryTS/perry#5882: Adjusts related Proxy set dispatch and receiver propagation impacted by this same accessor-handling behavior.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main behavioral fix in OrdinarySet for receiver-owned accessors.
Description check ✅ Passed The description clearly explains the bug, the spec correction, and validation results, though some template sections are omitted.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/t262-mech-ordinaryset-accessor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug
proggeramlug merged commit 7306fb6 into main Jul 6, 2026
25 checks passed
@proggeramlug
proggeramlug deleted the fix/t262-mech-ordinaryset-accessor branch July 6, 2026 07:18
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