Skip to content

Add completed PropertyDescriptor type - #5284

Open
HalidOdat wants to merge 2 commits into
boa-dev:mainfrom
HalidOdat:feature/complete-property-descriptor
Open

Add completed PropertyDescriptor type#5284
HalidOdat wants to merge 2 commits into
boa-dev:mainfrom
HalidOdat:feature/complete-property-descriptor

Conversation

@HalidOdat

Copy link
Copy Markdown
Member

Refactors property descriptor handling by replacing the builder-style PropertyDescriptor with a more explicit CompletePropertyDescriptor enum. This makes things clearer and more type-safe. The spec always stores complete property descriptors in objects.

Since descriptors are always complete, this allows us to skip some checks and may allow the compiler to better optimize the code. Additionally this also reduces the memory usage for sparse generic arrays, CompletePropertyDescriptor is 24 bytes vs. PropertyDescriptor 40 bytes (64bit arch).

@github-actions github-actions Bot added C-Tests Issues and PRs related to the tests. C-Builtins PRs and Issues related to builtins/intrinsics C-VM Issues and PRs related to the Boa Virtual Machine. Waiting On Review Waiting on reviews from the maintainers labels Mar 30, 2026
@github-actions github-actions Bot added this to the v1.0.0 milestone Mar 30, 2026
@github-actions

github-actions Bot commented Mar 30, 2026

Copy link
Copy Markdown

Test262 conformance changes

Test result main count PR count difference
Total 53,578 53,578 0
Passed 51,426 51,428 +2
Ignored 1,648 1,648 0
Failed 504 502 -2
Panics 0 0 0
Conformance 95.98% 95.99% +0.00%
Fixed tests (2):
test/staging/sm/Promise/promise-species.js (previously Failed)
test/staging/sm/Symbol/species.js (previously Failed)

Tested main commit: 665f03924a54e5162be227e7e909612e36f6e35a
Tested PR commit: 47d60cba56a5b513a72c368fa676c700db0daa32
Compare commits: 665f039...47d60cb

@HalidOdat
HalidOdat force-pushed the feature/complete-property-descriptor branch 2 times, most recently from 38fea6d to 40d9a5e Compare April 1, 2026 16:18
@codecov

codecov Bot commented Apr 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.69565% with 266 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.77%. Comparing base (6ddc2b4) to head (47d60cb).
⚠️ Report is 1050 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/object/internal_methods/mod.rs 66.46% 56 Missing ⚠️
core/engine/src/property/mod.rs 67.58% 47 Missing ⚠️
core/engine/src/object/property_map.rs 54.41% 31 Missing ⚠️
core/engine/src/builtins/proxy/mod.rs 20.00% 28 Missing ⚠️
core/engine/src/object/mod.rs 64.06% 23 Missing ⚠️
core/engine/src/value/display/object.rs 31.81% 15 Missing ⚠️
core/engine/src/builtins/builder.rs 74.35% 10 Missing ⚠️
core/engine/src/module/namespace.rs 0.00% 9 Missing ⚠️
core/engine/src/value/display/array.rs 25.00% 9 Missing ⚠️
core/engine/src/object/internal_methods/string.rs 14.28% 6 Missing ⚠️
... and 12 more
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5284       +/-   ##
===========================================
+ Coverage   47.24%   62.77%   +15.52%     
===========================================
  Files         476      536       +60     
  Lines       46892    60498    +13606     
===========================================
+ Hits        22154    37975    +15821     
+ Misses      24738    22523     -2215     

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

@HalidOdat
HalidOdat force-pushed the feature/complete-property-descriptor branch from 40d9a5e to 1594d05 Compare April 1, 2026 17:01
@HalidOdat
HalidOdat marked this pull request as ready for review April 1, 2026 17:19
@HalidOdat
HalidOdat requested a review from a team as a code owner April 1, 2026 17:19
@HalidOdat
HalidOdat force-pushed the feature/complete-property-descriptor branch from 1594d05 to c9edeab Compare July 5, 2026 14:10

@jedel1043 jedel1043 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mostly looks great! I just have a suggestion to avoid introducing more panics in the engine.

Comment thread core/engine/src/builtins/array/mod.rs Outdated
@jedel1043 jedel1043 added Waiting On Author Waiting on PR changes from the author and removed Waiting On Review Waiting on reviews from the maintainers labels Aug 30, 2026
@HalidOdat
HalidOdat force-pushed the feature/complete-property-descriptor branch 2 times, most recently from e86e5c5 to 5e9220e Compare August 31, 2026 21:52
@jedel1043

jedel1043 commented Aug 31, 2026

Copy link
Copy Markdown
Member

You might need to bump to v0.23 since removing fill_with is technically a breaking change, but it might just be possible to preserve the original public API by restricting the usage of fill_with to only incomplete descriptors, and only allow PropertyDescriptor::fill_with

@HalidOdat
HalidOdat force-pushed the feature/complete-property-descriptor branch from 81a97ad to 47d60cb Compare September 1, 2026 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Builtins PRs and Issues related to builtins/intrinsics C-Tests Issues and PRs related to the tests. C-VM Issues and PRs related to the Boa Virtual Machine. Waiting On Author Waiting on PR changes from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants