Skip to content

fix(builtins/typed_array,iterable,generator,error,bigint,eval): convert panics to EngineError::Panic using js_expect#5045

Merged
jedel1043 merged 1 commit intoboa-dev:mainfrom
KaustubhOG:fix/builtins-typed-array-iterable-generator-panics
Mar 16, 2026
Merged

fix(builtins/typed_array,iterable,generator,error,bigint,eval): convert panics to EngineError::Panic using js_expect#5045
jedel1043 merged 1 commit intoboa-dev:mainfrom
KaustubhOG:fix/builtins-typed-array-iterable-generator-panics

Conversation

@KaustubhOG
Copy link
Copy Markdown
Contributor

@KaustubhOG KaustubhOG commented Mar 13, 2026

Part of #3241. It changes the following:

  • builtins/typed_array/builtin.rs: converted 36 panics
  • builtins/typed_array/object.rs: converted 4 panics
  • builtins/iterable/async_from_sync_iterator.rs: converted 8 panics
  • builtins/generator/mod.rs: converted 3 panics
  • builtins/promise/mod.rs: converted 4 panics
  • builtins/json/mod.rs: converted 2 panics
  • builtins/error/aggregate.rs: converted 1 panic
  • builtins/bigint/mod.rs: converted 1 panic
  • builtins/eval/mod.rs: converted 1 panic

After this PR, core/engine/src/builtins/ is 98% panic-free.The remaining .expect() calls all fall into non-convertible categories:

Exhaustive match arms / logic invariants (no JsResult context):

  • atomics/mod.rs — unreachable! on float variants in integer-only ops
  • date/mod.rs, date/utils.rs — exhaustive match after prior validation
  • temporal/ — exhaustive match arms
  • intl/ — exhaustive match arms and logic invariants
  • number/mod.rs, number/globals.rs — pure math helpers, no JsResult propagation path

Builtin initialization infrastructure:

  • builder.rs — init-time invariants, no JsResult context
  • error/type.rs — fn init() returning ()

Low-level buffer ops (intentional panic on corruption):

  • array_buffer/utils.rs — raw slice index ops
  • array_buffer/shared.rs — same
  • atomics/futex.rs — OS threading node invariants

Non-JsResult contexts:

  • promise/mod.rs:2770 — inside fn returning JsObject
  • function/arguments.rs — inside fn returning ThinVec
  • iterator_helper.rs — internal type invariants on self-owned objects

Test-only:

  • uri/mod.rs — test function
  • date/tests.rs — test file

@KaustubhOG KaustubhOG requested a review from a team as a code owner March 13, 2026 20:38
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 13, 2026

Test262 conformance changes

Test result main count PR count difference
Total 52,963 52,963 0
Passed 49,935 49,935 0
Ignored 2,207 2,207 0
Failed 821 821 0
Panics 0 0 0
Conformance 94.28% 94.28% 0.00%

Tested main commit: 40f99905eefb4ac0542e4b7686425ffeb17ff09a
Tested PR commit: 6711953620dae7e1bb962762945e04d023a1caea
Compare commits: 40f9990...6711953

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 32.25806% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.98%. Comparing base (6ddc2b4) to head (6711953).
⚠️ Report is 866 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/builtins/typed_array/builtin.rs 4.16% 23 Missing ⚠️
core/engine/src/builtins/typed_array/object.rs 46.15% 7 Missing ⚠️
core/engine/src/builtins/promise/mod.rs 0.00% 4 Missing ⚠️
.../src/builtins/iterable/async_from_sync_iterator.rs 0.00% 3 Missing ⚠️
core/engine/src/builtins/generator/mod.rs 77.77% 2 Missing ⚠️
core/engine/src/object/builtins/jspromise.rs 0.00% 2 Missing ⚠️
core/engine/src/vm/opcode/await/mod.rs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5045       +/-   ##
===========================================
+ Coverage   47.24%   58.98%   +11.74%     
===========================================
  Files         476      563       +87     
  Lines       46892    62560    +15668     
===========================================
+ Hits        22154    36904    +14750     
- Misses      24738    25656      +918     

☔ 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.

@KaustubhOG KaustubhOG force-pushed the fix/builtins-typed-array-iterable-generator-panics branch from 9f99d23 to e41560d Compare March 13, 2026 22:18
…rt panics to EngineError::Panic using js_expect

Part of boa-dev#3241. It changes the following:
core/engine/src/builtins/typed_array/builtin.rs: converted 30 panics
core/engine/src/builtins/typed_array/object.rs: converted 3 panics
core/engine/src/builtins/iterable/async_from_sync_iterator.rs: converted 8 panics
core/engine/src/builtins/generator/mod.rs: converted 3 panics
core/engine/src/builtins/error/aggregate.rs: converted 1 panic
core/engine/src/builtins/bigint/mod.rs: converted 1 panic
core/engine/src/builtins/eval/mod.rs: converted 1 panic
@KaustubhOG KaustubhOG force-pushed the fix/builtins-typed-array-iterable-generator-panics branch from e41560d to 6711953 Compare March 14, 2026 13:04
@KaustubhOG
Copy link
Copy Markdown
Contributor Author

KaustubhOG commented Mar 14, 2026

Hey @jedel1043, addressed all 3 comments (does the looks good or any furtheree changes requierd). Also while I was at it, done with the remaining builtins/ panics in this same PR ( it got somewhere big but they were tiny (1-5 per file) scattered across promise/mod.rs, json/mod.rs and a couple more typed_array spots, i thought unecessary for a seprate PR to be raised.
.)

@jedel1043 jedel1043 added A-Technical Debt Changes related to technical debt C-Builtins PRs and Issues related to builtins/intrinsics labels Mar 16, 2026
@jedel1043 jedel1043 added this to the v1.0.0 milestone Mar 16, 2026
Copy link
Copy Markdown
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Thank you for this!

@jedel1043 jedel1043 added this pull request to the merge queue Mar 16, 2026
Merged via the queue into boa-dev:main with commit 69ae895 Mar 16, 2026
19 checks passed
@KaustubhOG
Copy link
Copy Markdown
Contributor Author

Remaining builtins/ panics audit — all invariants, zero convertible

File(s) Hits Reason
iterable/iterator_helper.rs 20 .expect("object type already verified") — downcast after explicit type-check gate. Spec-guaranteed unreachable.
atomics/tests.rs, intl/number_format/tests.rs, bigint/tests.rs, intl/locale/tests.rs, date/tests.rs, promise/tests.rs, math/tests.rs, function/tests.rs, array/tests.rs, array_buffer/tests.rs, number/tests.rs 76 All #[test] functions — panicking on failure is correct for tests. Not in scope for #3241.
number/mod.rs, number/globals.rs 15 Buffer iteration, radix validation, string slicing — all preceded by explicit bounds checks in the same function.
intl/locale/utils.rs 6 Inside #[cfg(test)] block — same as test files above.
builder.rs 6 Engine intrinsic bootstrap — failure here means a Boa bug, not a recoverable JS error.
string/mod.rs 5 Symbol-check done, length on new string, surrogate validated, i >= 0 asserted — all invariant-style.
array_buffer/utils.rs, array_buffer/shared.rs 8 Index bounds pre-validated by spec algorithm steps. shared.rs:357 — fixed-length check at caller.
intl/date_time_format/mod.rs 3 try_from_str on hardcoded BCP 47 literals ("gregory", "latn", "h12"). Compile-time constants, cannot fail.
uri/mod.rs 7 Lines 550–570: hardcoded byte literals in test fn. Line 589: test assertion context.
iterable/async_from_sync_iterator.rs 2 Spec-mandated invariants — prototype next and %Promise% return type guaranteed by spec's object model.
intl/segmenter/mod.rs, intl/number_format/options.rs, function/arguments.rs, error/type.rs, regexp/mod.rs, promise/mod.rs, array/mod.rs, atomics/futex.rs 17 Single-site invariants — range pre-checked, spec says "cannot fail", or unreachable-equivalent panics.
Total 165 0 convertible

All remaining panics fall into: test assertions, compile-time constant .expect, spec-guaranteed invariants, or engine bootstrap invariants. None are suitable for EngineError conversion.

@KaustubhOG
Copy link
Copy Markdown
Contributor Author

builtin-top

@KaustubhOG
Copy link
Copy Markdown
Contributor Author

Remaining builtins/ panics audit — all invariants, zero convertible

File(s) Hits Reason
iterable/iterator_helper.rs 20 .expect("object type already verified") — downcast after explicit type-check gate. Spec-guaranteed unreachable.
atomics/tests.rs, intl/number_format/tests.rs, bigint/tests.rs, intl/locale/tests.rs, date/tests.rs, promise/tests.rs, math/tests.rs, function/tests.rs, array/tests.rs, array_buffer/tests.rs, number/tests.rs 76 All #[test] functions — panicking on failure is correct for tests. Not in scope for #3241.
number/mod.rs, number/globals.rs 15 Buffer iteration, radix validation, string slicing — all preceded by explicit bounds checks in the same function.
intl/locale/utils.rs 6 Inside #[cfg(test)] block — same as test files above.
builder.rs 6 Engine intrinsic bootstrap — failure here means a Boa bug, not a recoverable JS error.
string/mod.rs 5 Symbol-check done, length on new string, surrogate validated, i >= 0 asserted — all invariant-style.
array_buffer/utils.rs, array_buffer/shared.rs 8 Index bounds pre-validated by spec algorithm steps. shared.rs:357 — fixed-length check at caller.
intl/date_time_format/mod.rs 3 try_from_str on hardcoded BCP 47 literals ("gregory", "latn", "h12"). Compile-time constants, cannot fail.
uri/mod.rs 7 Lines 550–570: hardcoded byte literals in test fn. Line 589: test assertion context.
iterable/async_from_sync_iterator.rs 2 Spec-mandated invariants — prototype next and %Promise% return type guaranteed by spec's object model.
intl/segmenter/mod.rs, intl/number_format/options.rs, function/arguments.rs, error/type.rs, regexp/mod.rs, promise/mod.rs, array/mod.rs, atomics/futex.rs 17 Single-site invariants — range pre-checked, spec says "cannot fail", or unreachable-equivalent panics.
Total 165 0 convertible
All remaining panics fall into: test assertions, compile-time constant .expect, spec-guaranteed invariants, or engine bootstrap invariants. None are suitable for EngineError conversion.

only for notes purpose will relocate all this (#5063 #5064 #5267) in #3241 and clear everything (that tracking progess bar ).

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

Labels

A-Technical Debt Changes related to technical debt C-Builtins PRs and Issues related to builtins/intrinsics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants