Skip to content

Cleanup BuiltInConstructor constants to ensure no additional allocations#4464

Merged
jedel1043 merged 3 commits intoboa-dev:mainfrom
jedel1043:builtin-constructor-cleanup
Oct 14, 2025
Merged

Cleanup BuiltInConstructor constants to ensure no additional allocations#4464
jedel1043 merged 3 commits intoboa-dev:mainfrom
jedel1043:builtin-constructor-cleanup

Conversation

@jedel1043
Copy link
Member

Cleanup some of the BuiltInConstructor constants to ensure the constants are more descriptive and the pushed properties don't exceeded the defined allocated property slots.

@jedel1043 jedel1043 added A-Enhancement New feature or request A-Internal Changes that don't modify execution behaviour labels Oct 12, 2025
@codecov
Copy link

codecov bot commented Oct 12, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.43%. Comparing base (6ddc2b4) to head (4408008).
⚠️ Report is 555 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/builtins/builder.rs 88.88% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4464      +/-   ##
==========================================
+ Coverage   47.24%   51.43%   +4.19%     
==========================================
  Files         476      504      +28     
  Lines       46892    51411    +4519     
==========================================
+ Hits        22154    26444    +4290     
- Misses      24738    24967     +229     

☔ 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 force-pushed the builtin-constructor-cleanup branch from e773cf2 to cfdd49d Compare October 12, 2025 16:21
@github-actions
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 50,595 50,595 0
Passed 47,620 47,620 0
Ignored 2,056 2,056 0
Failed 919 919 0
Panics 0 0 0
Conformance 94.12% 94.12% 0.00%

Comment on lines +393 to +415
#[cfg(debug_assertions)]
{
assert!(
self.prototype_storage.len()
<= self.prototype_storage_slots_expected + Self::OWN_PROTOTYPE_STORAGE_SLOTS,
"expected to allocate at most {} prototype storage slots, got {}. \
constant {}::PROTOTYPE_STORAGE_SLOTS may need to be adjusted",
self.prototype_storage_slots_expected,
self.prototype_storage.len() - Self::OWN_PROTOTYPE_STORAGE_SLOTS,
self.name.display_escaped(),
);
assert!(
self.constructor_storage.len()
<= self.constructor_storage_slots_expected
+ Self::OWN_CONSTRUCTOR_STORAGE_SLOTS,
"expected to allocate at most {} constructor storage slots, got {}. \
constant {}::CONSTRUCTOR_STORAGE_SLOTS may need to be adjusted",
self.constructor_storage_slots_expected,
self.constructor_storage.len() - Self::OWN_CONSTRUCTOR_STORAGE_SLOTS,
self.name.display_escaped(),
);
}

Copy link
Member

Choose a reason for hiding this comment

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

This is a great idea to have a debug assertion to check

Copy link
Member

@jasonwilliams jasonwilliams left a comment

Choose a reason for hiding this comment

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

LGTM apart from comment

@jedel1043 jedel1043 added this to the next-release milestone Oct 14, 2025
@jedel1043 jedel1043 enabled auto-merge October 14, 2025 00:19
@jedel1043 jedel1043 added this pull request to the merge queue Oct 14, 2025
Merged via the queue into boa-dev:main with commit b33eb6b Oct 14, 2025
16 checks passed
@jedel1043 jedel1043 deleted the builtin-constructor-cleanup branch October 14, 2025 00:43
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