Skip to content

Fix nested class scope index#4592

Open
regularkevvv wants to merge 2 commits intoboa-dev:mainfrom
regularkevvv:fix-nested-class-scope-index
Open

Fix nested class scope index#4592
regularkevvv wants to merge 2 commits intoboa-dev:mainfrom
regularkevvv:fix-nested-class-scope-index

Conversation

@regularkevvv
Copy link
Contributor

This Pull Request closes #4555.

How to reproduce:

// Class expression constructor with nested class
new (class{constructor(){class D{}}})();

// Static block with nested class
(class{static{class D{}}});

Both crash with: index out of bounds: the len is 0 but the index is 0

Additional patterns that trigger the bug:

// Constructor variants
[new (class{constructor(){class D{}}})()]
({x: new (class{constructor(){class D{}}})()})
new (class{ m(){} constructor(){class D{}} })()
new (class{ static m(){} constructor(){class D{}} })()
new (class{ get g(){return 1} constructor(){class D{}} })()
new (class{ #m(){} constructor(){class D{}} })()
new (class{ ['p']=1; constructor(){class D{}} })()

// Static block variants
(class{ static{let v=1} static{class D{}} })
(class{static{let v=1;((v)=>{class D{}})(v)}})

It changes the following:

  • Fixed ScopeIndexVisitor::visit_class_expression_mut to call visit_function_like with force_function_scope: true for constructors, matching the behavior of visit_class_declaration_mut. Previously it called visit_function_expression_mut which passes false for force_function_scope.

  • Fixed ScopeIndexVisitor::visit_class_element_mut for StaticBlock to pass true for force_function_scope. Previously it passed contains_direct_eval which is unrelated to whether a function scope is pushed at runtime.

Both class expression constructors and static blocks always have HAS_FUNCTION_SCOPE set during bytecode compilation, so the scope analyzer must account for this when assigning scope indices.

@github-actions
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 52,598 52,598 0
Passed 49,421 49,420 -1
Ignored 2,134 2,134 0
Failed 1,043 1,044 +1
Panics 0 0 0
Conformance 93.96% 93.96% -0.00%
Broken tests (1):
test/staging/Temporal/v8/instant-to-json.js (previously Passed)

@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.50%. Comparing base (6ddc2b4) to head (b6fbad8).
⚠️ Report is 639 commits behind head on main.

Files with missing lines Patch % Lines
core/ast/src/scope_analyzer.rs 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4592      +/-   ##
==========================================
+ Coverage   47.24%   56.50%   +9.25%     
==========================================
  Files         476      547      +71     
  Lines       46892    60060   +13168     
==========================================
+ Hits        22154    33935   +11781     
- Misses      24738    26125    +1387     

☔ View full report in Codecov by Sentry.
📢 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.

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.

Panic index out of bounds: the len is 0 but the index is 0 in core/engine/src/vm/opcode/define/mod.rs

1 participant