Skip to content

fix(engine): correct WithBaseObject and HasBinding semantics for with calls - #5507

Open
xcb3d wants to merge 3 commits into
boa-dev:mainfrom
xcb3d:fix/with-object-environment-calls
Open

fix(engine): correct WithBaseObject and HasBinding semantics for with calls#5507
xcb3d wants to merge 3 commits into
boa-dev:mainfrom
xcb3d:fix/with-object-environment-calls

Conversation

@xcb3d

@xcb3d xcb3d commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

This Pull Request fixes the evaluation order and semantics of WithBaseObject and HasBinding for function calls within with statements, as well as property verification in GetBindingValue for object environment records.

It changes the following:

  • In function calls within with blocks, evaluate GetNameAndLocator first to resolve the binding and callee value, pushing the locator onto binding_stack.
  • Derive the this value (WithBaseObject) via ThisForObjectEnvironmentName directly from the resolved binding locator without re-scanning the environment chain, ensuring HasBinding runs only once per spec.
  • Remove unused IndexOperand from ThisForObjectEnvironmentName to save bytecode space.
  • Implement step 2 of GetBindingValue ( N, S ) for ObjectEnvironmentRecord to verify property presence before Get, returning undefined in sloppy mode or throwing ReferenceError in strict mode when deleted via @@unscopables.
  • Add engine-level unit tests for proxy traps and @@unscopables deletion under with.

… calls

- In function calls within with blocks, evaluate GetNameAndLocator first to resolve the binding and callee value, pushing the locator onto binding_stack.
- Derive the this value (WithBaseObject) via ThisForObjectEnvironmentName directly from the resolved binding locator without re-scanning the environment chain, ensuring HasBinding runs only once per spec.
- Remove unused IndexOperand from ThisForObjectEnvironmentName to save bytecode space.
- Implement step 2 of GetBindingValue (N, S) for ObjectEnvironmentRecord to verify property presence before Get, returning undefined in sloppy mode or throwing ReferenceError in strict mode when deleted via @@unscopables.
- Add engine-level unit tests for proxy traps and @@unscopables deletion under with.
@xcb3d
xcb3d requested a review from a team as a code owner September 5, 2026 04:35
@github-actions github-actions Bot added the Waiting On Review Waiting on reviews from the maintainers label Sep 5, 2026
@github-actions github-actions Bot added this to the v0.23 milestone Sep 5, 2026
@github-actions github-actions Bot added C-Tests Issues and PRs related to the tests. C-VM Issues and PRs related to the Boa Virtual Machine. labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Test262 conformance changes

Test result main count PR count difference
Total 53,578 53,578 0
Passed 51,426 51,431 +5
Ignored 1,648 1,648 0
Failed 504 499 -5
Panics 0 0 0
Conformance 95.98% 95.99% +0.01%
Fixed tests (5):
test/language/statements/with/get-binding-value-idref-with-proxy-env.js (previously Failed)
test/language/statements/with/get-binding-value-call-with-proxy-env.js (previously Failed)
test/language/statements/with/has-binding-call-with-proxy-env.js (previously Failed)
test/language/statements/with/set-mutable-binding-idref-compound-assign-with-proxy-env.js (previously Failed)
test/language/statements/with/get-mutable-binding-binding-deleted-in-get-unscopables-strict-mode.js (previously Failed)

Tested main commit: 665f03924a54e5162be227e7e909612e36f6e35a
Tested PR commit: 5743f45137bed9e057856bf10b27ba02c6b6ea59
Compare commits: 665f039...5743f45

@codecov

codecov Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.06%. Comparing base (6ddc2b4) to head (5743f45).
⚠️ Report is 1050 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5507       +/-   ##
===========================================
+ Coverage   47.24%   63.06%   +15.82%     
===========================================
  Files         476      536       +60     
  Lines       46892    60281    +13389     
===========================================
+ Hits        22154    38019    +15865     
+ Misses      24738    22262     -2476     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nekevss nekevss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an insta-bytecode test for this change as well.

Pins the bytecode for a with-statement call: GetNameAndLocator resolves the binding/locator once, then the dst-only ThisForObjectEnvironmentName derives the call's this (WithBaseObject), per PR boa-dev#5507.
@github-actions github-actions Bot added the C-Javascript Pull requests that update Javascript code label Sep 5, 2026
Prettier reflowed the `with (...)` object literal onto multiple lines.
The reformat only shifted the source position of the call, so the
insta-bytecode snapshot was re-accepted (Source Map offset only); the
compiled opcodes are unchanged and it still asserts GetNameAndLocator
followed by a dst-only ThisForObjectEnvironmentName.
@xcb3d
xcb3d requested a review from nekevss September 5, 2026 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Javascript Pull requests that update Javascript code C-Tests Issues and PRs related to the tests. C-VM Issues and PRs related to the Boa Virtual Machine. Waiting On Review Waiting on reviews from the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants