Skip to content

fix(builtins/function/mod,function/arguments,function/bound,object/mod): convert panics to EngineError::Panic using js_expect#5001

Merged
jedel1043 merged 3 commits intoboa-dev:mainfrom
KaustubhOG:fix/convert-panics-builtins-function-object
Mar 12, 2026
Merged

fix(builtins/function/mod,function/arguments,function/bound,object/mod): convert panics to EngineError::Panic using js_expect#5001
jedel1043 merged 3 commits intoboa-dev:mainfrom
KaustubhOG:fix/convert-panics-builtins-function-object

Conversation

@KaustubhOG
Copy link
Contributor

Part of #3241.

It changes the following:

  • core/engine/src/builtins/function/arguments.rs: converted 7 panics (7× js_expect on downcast_ref::<MappedArguments>() exotic method calls); binding_indices retains .expect("binding must exist") as a documented
    compiler invariant — binding existence is guaranteed by bound_names(formals)
  • core/engine/src/builtins/function/mod.rs: converted 10 panics (2× downcast_ref::<OrdinaryFunction>(), 3× vm.frames.last() frame existence, 1× to_integer_or_infinity, 1× define_property_or_throw length, 1× define_property_or_throw name, 1× new.target object cast, 1× set_function_name) to use js_expect
  • core/engine/src/builtins/function/bound.rs: converted 2 panics (2× downcast_ref::<BoundFunction>() exotic method calls) to use js_expect
  • core/engine/src/builtins/object/mod.rs: converted 10 panics (6× CreateDataPropertyOrThrow
  • from_property_descriptor, 1× create_data_property_or_throw in group_by,1× create_data_property_or_throw in get_own_property_descriptors,1× toObject in to_string, 1× ToObject in assign) to use js_expect

@KaustubhOG KaustubhOG requested a review from a team as a code owner March 11, 2026 16:27
@KaustubhOG
Copy link
Contributor Author

KaustubhOG commented Mar 11, 2026

Note: set_function_name was changed from -> () to -> JsResult<()>. Call sites in vm/opcode/define/class/ and vm/opcode/set/property.rs were updated only to propagate this cascade ,remaining .expect() calls in those files are left because vm opcodes are a separate batch (after /builtins complete) in #3241

…d): convert panics to EngineError::Panic using js_expect
@KaustubhOG KaustubhOG force-pushed the fix/convert-panics-builtins-function-object branch from 2c65f19 to 6091509 Compare March 11, 2026 17:28
@github-actions
Copy link

github-actions bot commented Mar 11, 2026

Test262 conformance changes

Test result main count PR count difference
Total 52,963 52,963 0
Passed 49,901 49,901 0
Ignored 2,222 2,222 0
Failed 840 840 0
Panics 0 0 0
Conformance 94.22% 94.22% 0.00%

Tested main commit: a49348586d90b790936dc6ab651da6aff3f60e67
Tested PR commit: bef34cd145106537cbd9da7625ff84670b8f72ab
Compare commits: a493485...bef34cd

@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 61.11111% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.29%. Comparing base (6ddc2b4) to head (bef34cd).
⚠️ Report is 819 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/vm/opcode/define/class/setter.rs 25.00% 3 Missing ⚠️
core/engine/src/builtins/function/arguments.rs 33.33% 2 Missing ⚠️
core/engine/src/builtins/function/bound.rs 0.00% 2 Missing ⚠️
core/engine/src/builtins/function/mod.rs 71.42% 2 Missing ⚠️
core/engine/src/vm/opcode/define/class/getter.rs 50.00% 2 Missing ⚠️
core/engine/src/vm/opcode/define/class/method.rs 50.00% 2 Missing ⚠️
core/engine/src/builtins/proxy/mod.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5001       +/-   ##
===========================================
+ Coverage   47.24%   58.29%   +11.05%     
===========================================
  Files         476      559       +83     
  Lines       46892    61408    +14516     
===========================================
+ Hits        22154    35798    +13644     
- Misses      24738    25610      +872     

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

@jedel1043 jedel1043 added this pull request to the merge queue Mar 12, 2026
@jedel1043 jedel1043 added A-Enhancement New feature or request A-Internal Changes that don't modify execution behaviour labels Mar 12, 2026
Merged via the queue into boa-dev:main with commit 5586a04 Mar 12, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Enhancement New feature or request A-Internal Changes that don't modify execution behaviour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants