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

Implement Array.prototype.copyWithin #1334

Merged
merged 1 commit into from
Jun 18, 2021

Conversation

jedel1043
Copy link
Member

This Pull Request checks off a method from #36.

It changes the following:

  • Adds the method Array.prototype.copyWithin
  • Adds a test for said method

Had to take a few liberties to make the code more readable, but it should be spec compliant nonetheless. Please tell me if I overlooked something to fix it asap.

@Razican Razican added enhancement New feature or request builtins PRs and Issues related to builtins/intrinsics labels Jun 17, 2021
@Razican Razican added this to the v0.13.0 milestone Jun 17, 2021
@Razican
Copy link
Member

Razican commented Jun 17, 2021

Test262 conformance changes:

Test result master count PR count difference
Total 78,897 78,897 0
Passed 26,865 26,919 +54
Ignored 15,628 15,628 0
Failed 36,404 36,350 -54
Panics 0 0 0
Conformance 34.05% 34.12% +0.07%
Fixed tests:
test/built-ins/Array/prototype/copyWithin/negative-out-of-bounds-start.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-out-of-bounds-start.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-target.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-target.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-start.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-start.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-start-change-start.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-start-change-start.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/undefined-end.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/undefined-end.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/length-near-integer-limit.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/length-near-integer-limit.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-target.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-target.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-start-change-target.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-start-change-target.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-end.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-end.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-target.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-target.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-out-of-bounds-target.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-out-of-bounds-target.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/non-negative-out-of-bounds-target-and-start.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/non-negative-out-of-bounds-target-and-start.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-get-start-value.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-get-start-value.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/name.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/name.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/non-negative-target-and-start.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/non-negative-target-and-start.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-start.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-start.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/call-with-boolean.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/call-with-boolean.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-out-of-bounds-end.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/negative-out-of-bounds-end.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/non-negative-target-start-and-end.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/non-negative-target-start-and-end.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-start.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-start.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-end.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/coerced-values-end.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-end.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-end.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-set-target-value.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-set-target-value.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-this.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-this.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-this-length.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-this-length.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/non-negative-out-of-bounds-end.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/non-negative-out-of-bounds-end.js (previously Failed)
test/built-ins/Array/prototype/copyWithin/prop-desc.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/copyWithin/prop-desc.js (previously Failed)
test/built-ins/Number/S9.3.1_A3_T2.js [strict mode] (previously Failed)
test/built-ins/Number/S9.3.1_A3_T2.js (previously Failed)
test/built-ins/Number/S9.3.1_A3_T1.js [strict mode] (previously Failed)
test/built-ins/Number/S9.3.1_A3_T1.js (previously Failed)
Broken tests:
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-delete-target.js [strict mode] (previously Passed)
test/built-ins/Array/prototype/copyWithin/return-abrupt-from-delete-target.js (previously Passed)
test/built-ins/Number/prototype/toExponential/this-is-0-fractiondigits-is-0.js [strict mode] (previously Passed)
test/built-ins/Number/prototype/toExponential/this-is-0-fractiondigits-is-0.js (previously Passed)

@jedel1043 jedel1043 force-pushed the copy_within_arr branch 4 times, most recently from 95d2f5d to d666f34 Compare June 18, 2021 07:52
Copy link
Contributor

@RageKnify RageKnify 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, looking at the spec, seems like our implementations of has_field and remove_property aren't enough, since it should be possible for them to throw.
We should open an issue for that, but I think this implementation is good right now, maybe add some TODO comments around lines 1595 and 1599.

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.

Let's merge it! Thanks!

@Razican Razican merged commit 9497636 into boa-dev:master Jun 18, 2021
@jedel1043 jedel1043 deleted the copy_within_arr branch June 18, 2021 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants