Skip to content

fix(engine): correct computed class key evaluation timing - #5641

Open
shreyashsri79 wants to merge 1 commit into
boa-dev:mainfrom
shreyashsri79:gsoc/5635-class-computed-keys
Open

shreyashsri79 wants to merge 1 commit into
boa-dev:mainfrom
shreyashsri79:gsoc/5635-class-computed-keys

Conversation

@shreyashsri79

Copy link
Copy Markdown

Fixes #5635.

Computed static field names were converted to property keys during static initialization, after later instance field keys and earlier static initializers. The class-name binding was also initialized before computed names were evaluated.

This change converts each static computed name to a key at its source position and initializes the class binding after the element loop, before static initializers run, following ClassDefinitionEvaluation. The later redundant key conversion for anonymous static field initializers is removed.

Five tests cover mixed method/static/instance key order, key conversion before initializers, abrupt key conversion, the class-name TDZ in declarations and named expressions, and class-name access from static and instance initializers.

Validation at ace614acc17106f8d8633b0b1071991506583433, based on 329e89748ed34e218fe593b4209b1dbe1d6eb64d, with Rust/Cargo 1.99.1:

  • Four new regression tests failed with unchanged production code; all 17 focused class tests pass with the fix. The TDZ assertion checks the existing C is not defined diagnostic and ReferenceError kind.
  • cargo test --locked --profile ci -p boa_engine --lib -- --nocapture --test-threads=1: 1,144 passed, no failures or ignored tests.
  • cargo test --locked --profile ci -p insta-bytecode -- --nocapture --test-threads=1: bytecode snapshots passed without updates.
  • cargo fmt --all --check: passed.
  • cargo clippy --locked --profile ci -p boa_engine --all-features --all-targets -- -D warnings: passed.
  • cargo run --locked --profile ci --bin boa_tester -- run -vv -d --test262-path <pinned-checkout> -s <suite> -o <results> for test/language/statements/class and test/language/expressions/class: 8,402 passed, 24 ignored, zero failures/panics on both baseline and candidate, with no case-level outcome changes. Test262 revision: d86b2294eb0a17eaa281ff12c73c473ec864c72f. These existing suites also pass on the buggy baseline; the new unit tests establish the regressions.

Full workspace/all-feature test suites and full Test262 were not run locally. Cargo.lock is unchanged.

Prepared with OpenAI Codex assistance.

@shreyashsri79
shreyashsri79 requested a review from a team as a code owner November 5, 2026 21:43
@github-actions github-actions Bot added Waiting On Review Waiting on reviews from the maintainers C-Tests Issues and PRs related to the tests. C-VM Issues and PRs related to the Boa Virtual Machine. labels Nov 5, 2026
@github-actions github-actions Bot added this to the v0.23 milestone Nov 5, 2026
@github-actions

Copy link
Copy Markdown

Test262 conformance changes

Test result main count PR count difference
Total 53,578 53,578 0
Passed 51,450 51,451 +1
Ignored 1,648 1,648 0
Failed 480 479 -1
Panics 0 0 0
Conformance 96.03% 96.03% +0.00%
Fixed tests (1):
test/staging/sm/class/innerBinding.js (previously Failed)

Tested main commit: bfaa15a7d60d8c92370a423d4134ab19b1b1fae0
Tested PR commit: ace614acc17106f8d8633b0b1071991506583433
Compare commits: bfaa15a...ace614a

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Static and instance computed field keys are evaluated in the wrong order, and the class binding is not in TDZ for computed keys

1 participant