Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type safe root shape #2940

Merged
merged 2 commits into from May 19, 2023
Merged

Type safe root shape #2940

merged 2 commits into from May 19, 2023

Conversation

HalidOdat
Copy link
Member

Currently when we construct a Realm we take a SharedShape as the root, since users can create realms to ensure that the passed shape is the root shape (otherwise if a non-root shape is passed, will at best cause a panic and at worse will give unexpected behaviour), added the RootShape that ensures that only a root shape is passed, this is applied to many other places where a root shape is required.

It changes the following:

  • Add RootShape wrapper around SharedShape
  • Implement From<UniqueShape> for Shape
  • Implement From<SharedShape> for Shape

@HalidOdat HalidOdat added enhancement New feature or request API labels May 18, 2023
@HalidOdat HalidOdat added this to the v0.17.0 milestone May 18, 2023
@HalidOdat HalidOdat requested a review from a team May 18, 2023 02:09
@HalidOdat HalidOdat marked this pull request as ready for review May 18, 2023 02:10
@github-actions
Copy link

github-actions bot commented May 18, 2023

Test262 conformance changes

Test result main count PR count difference
Total 93,982 93,982 0
Passed 73,783 73,783 0
Ignored 17,489 17,489 0
Failed 2,710 2,710 0
Panics 0 0 0
Conformance 78.51% 78.51% 0.00%

@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #2940 (c3c68d1) into main (4a368a2) will increase coverage by 0.00%.
The diff coverage is 73.46%.

@@           Coverage Diff           @@
##             main    #2940   +/-   ##
=======================================
  Coverage   50.39%   50.39%           
=======================================
  Files         443      444    +1     
  Lines       45337    45349   +12     
=======================================
+ Hits        22846    22854    +8     
- Misses      22491    22495    +4     
Impacted Files Coverage Δ
boa_engine/src/builtins/weak_map/mod.rs 14.11% <0.00%> (-0.35%) ⬇️
boa_engine/src/builtins/weak_set/mod.rs 13.75% <0.00%> (-0.36%) ⬇️
boa_engine/src/object/jsobject.rs 58.64% <ø> (ø)
boa_engine/src/object/shape/unique_shape.rs 26.80% <0.00%> (ø)
boa_engine/src/object/shape/mod.rs 80.95% <68.75%> (ø)
boa_engine/src/builtins/weak/weak_ref.rs 73.80% <100.00%> (+1.30%) ⬆️
boa_engine/src/context/intrinsics.rs 97.54% <100.00%> (+<0.01%) ⬆️
boa_engine/src/context/mod.rs 61.80% <100.00%> (ø)
boa_engine/src/object/property_map.rs 70.15% <100.00%> (+0.11%) ⬆️
boa_engine/src/object/shape/root_shape.rs 100.00% <100.00%> (ø)
... and 3 more

Copy link
Member

@raskad raskad left a comment

Choose a reason for hiding this comment

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

Looks very reasonable to me.

Copy link
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.

Nice improvement!

@jedel1043 jedel1043 enabled auto-merge May 18, 2023 22:51
@HalidOdat HalidOdat force-pushed the feature/type-safe-root-shape branch from e82db13 to c3c68d1 Compare May 19, 2023 03:25
@jedel1043 jedel1043 added this pull request to the merge queue May 19, 2023
Merged via the queue into main with commit e335c54 May 19, 2023
9 checks passed
@HalidOdat HalidOdat deleted the feature/type-safe-root-shape branch May 19, 2023 04:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants