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

[Merged by Bors] - Change ArrayBuffer byteLength to accessor property #2010

Closed
wants to merge 1 commit into from

Conversation

lupd
Copy link
Contributor

@lupd lupd commented Apr 5, 2022

This Pull Request fixes byteLength for ArrayBuffer. It should be an accessor property rather than a method, per the spec.

It changes the following:

  • Removes byteLength method for ArrayBuffer built-in.
  • Add byteLength accessor property for ArrayBuffer.
  • Change byte_length function name to get_byte_length, to match other function names used for accessor properties.

@codecov
Copy link

codecov bot commented Apr 5, 2022

Codecov Report

Merging #2010 (278049a) into main (ffc7a3e) will increase coverage by 0.00%.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##             main    #2010   +/-   ##
=======================================
  Coverage   45.89%   45.90%           
=======================================
  Files         206      206           
  Lines       17150    17152    +2     
=======================================
+ Hits         7871     7873    +2     
  Misses       9279     9279           
Impacted Files Coverage Δ
boa_engine/src/builtins/array_buffer/mod.rs 9.44% <66.66%> (+1.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffc7a3e...278049a. Read the comment docs.

@HalidOdat
Copy link
Member

VM implementation

Test result main count PR count difference
Total 88,650 88,650 0
Passed 44,005 44,047 +42
Ignored 21,711 21,711 0
Failed 22,934 22,892 -42
Panics 0 0 0
Conformance 49.64% 49.69% +0.05%
Fixed tests (42):
test/built-ins/ArrayBuffer/zero-length.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/zero-length.js (previously Failed)
test/built-ins/ArrayBuffer/toindex-length.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/toindex-length.js (previously Failed)
test/built-ins/ArrayBuffer/length-is-absent.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/length-is-absent.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/end-exceeds-length.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/end-exceeds-length.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/species-returns-larger-arraybuffer.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/species-returns-larger-arraybuffer.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/end-default-if-absent.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/end-default-if-absent.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/negative-start.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/negative-start.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/start-default-if-undefined.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/start-default-if-undefined.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/start-default-if-absent.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/start-default-if-absent.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/negative-end.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/negative-end.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/tointeger-conversion-end.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/tointeger-conversion-end.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/tointeger-conversion-start.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/tointeger-conversion-start.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/start-exceeds-length.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/start-exceeds-length.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/end-default-if-undefined.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/end-default-if-undefined.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/start-exceeds-end.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/slice/start-exceeds-end.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/length.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/length.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/detached-buffer.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/detached-buffer.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/prop-desc.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/prop-desc.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/invoked-as-accessor.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/invoked-as-accessor.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/return-bytelength.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/return-bytelength.js (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/name.js [strict mode] (previously Failed)
test/built-ins/ArrayBuffer/prototype/byteLength/name.js (previously Failed)

@HalidOdat HalidOdat added bug Something isn't working builtins PRs and Issues related to builtins/intrinsics labels Apr 6, 2022
@HalidOdat HalidOdat added this to the v0.15.0 milestone Apr 6, 2022
Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good :)

@Razican
Copy link
Member

Razican commented Apr 6, 2022

bors r+

bors bot pushed a commit that referenced this pull request Apr 6, 2022
This Pull Request fixes `byteLength` for `ArrayBuffer`. It should be an accessor property rather than a method, per the spec.

It changes the following:

- Removes `byteLength` method for `ArrayBuffer` built-in.
- Add `byteLength` accessor property for `ArrayBuffer`.
- Change `byte_length` function name to `get_byte_length`, to match other function names used for accessor properties.
@bors
Copy link

bors bot commented Apr 6, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Change ArrayBuffer byteLength to accessor property [Merged by Bors] - Change ArrayBuffer byteLength to accessor property Apr 6, 2022
@bors bors bot closed this Apr 6, 2022
@lupd lupd deleted the array-buffer-length branch April 7, 2022 19:20
Razican pushed a commit that referenced this pull request Jun 8, 2022
This Pull Request fixes `byteLength` for `ArrayBuffer`. It should be an accessor property rather than a method, per the spec.

It changes the following:

- Removes `byteLength` method for `ArrayBuffer` built-in.
- Add `byteLength` accessor property for `ArrayBuffer`.
- Change `byte_length` function name to `get_byte_length`, to match other function names used for accessor properties.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working builtins PRs and Issues related to builtins/intrinsics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants