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.

Sign up for GitHub

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

Jump to bottom

feat(boa): adds splice method #1365

Merged
merged 17 commits into from
Sep 6, 2021
Merged

feat(boa): adds splice method #1365

merged 17 commits into from
Sep 6, 2021

Conversation

neeldug
Copy link
Contributor

@neeldug neeldug commented Jun 27, 2021

This Pull Request fixes/closes #36.

It changes the following:

  • adds array.prototype.splice
  • todo: fix stalls at certain testcases

@neeldug
Copy link
Contributor Author

neeldug commented Jun 27, 2021

Actually, worked out why it stalls, and it's been patched in #1351 since it's due to array_species_create not having upper limits setup.

@neeldug
Copy link
Contributor Author

neeldug commented Jun 27, 2021 •
edited
Loading

Test262 conformance changes:

Test result master count PR count difference
Total 80,685 80,685 0
Passed 32,838 32,958 +120
Ignored 15,818 15,818 0
Failed 32,029 31,909 -120
Panics 0 0 0
Conformance 40.70% 40.85% +0.15%
Fixed tests (120): 
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-4-50.js [strict mode] (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-4-50.js (previously Failed)
test/built-ins/Array/prototype/findIndex/array-altered-during-loop.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/findIndex/array-altered-during-loop.js (previously Failed)
test/built-ins/Array/prototype/find/array-altered-during-loop.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/find/array-altered-during-loop.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/set_length_no_args.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/set_length_no_args.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/prop-desc.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/prop-desc.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/length-and-deleteCount-exceeding-integer-limit.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/length-and-deleteCount-exceeding-integer-limit.js (previously Failed)
test/built-ins/Array/prototype/splice/call-with-boolean.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/call-with-boolean.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species-poisoned.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species-poisoned.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species-abrupt.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species-abrupt.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T5.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T5.js (previously Failed)
test/built-ins/Array/prototype/splice/length-exceeding-integer-limit-shrink-array.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/length-exceeding-integer-limit-shrink-array.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T6.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T6.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/15.4.4.12-9-a-1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/15.4.4.12-9-a-1.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A3_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A3_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.3_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.3_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species-undef.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species-undef.js (previously Failed)
test/built-ins/Array/prototype/splice/create-ctor-poisoned.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-ctor-poisoned.js (previously Failed)
test/built-ins/Array/prototype/splice/called_with_one_argument.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/called_with_one_argument.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T6.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T6.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T5.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T5.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T4.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T4.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/name.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/name.js (previously Failed)
test/built-ins/Array/prototype/splice/create-non-array-invalid-len.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-non-array-invalid-len.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T4.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T4.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species-null.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species-null.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T4.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T4.js (previously Failed)

HalidOdat
HalidOdat reviewed Jul 21, 2021
View reviewed changes
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.

This looks good :)

Could we get documentation per step like some other builtin function, makes it much easier to follow the spec.

@HalidOdat HalidOdat added the builtins PRs and Issues related to builtins/intrinsics label Jul 21, 2021
@HalidOdat HalidOdat added this to the v0.13.0 milestone Jul 21, 2021
@neeldug neeldug requested a review from HalidOdat July 21, 2021 23:07
@neeldug neeldug force-pushed the impl-splice branch 2 times, most recently from fce34d7 to bffa4b4 Compare July 21, 2021 23:35
@neeldug
Copy link
Contributor Author

neeldug commented Jul 21, 2021 •
edited
Loading

Some weird behaviour occurs when setting a property as non writable on an object, where for instance on the failing test, test/built-ins/Array/prototype/splice/S15.4.4.12_A6.1_T2.js, it clears the array for some reason when setting length to unwritable. @HalidOdat @Razican any ideas on where this could be going wrong since it executes Object.set_property which seems to be the flawed step.

@neeldug neeldug marked this pull request as ready for review July 25, 2021 12:40
HalidOdat
HalidOdat reviewed Jul 30, 2021
View reviewed changes
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/value/mod.rs Outdated Show resolved Hide resolved
neeldug added 8 commits July 30, 2021 19:01
@neeldug
feat(boa): adds splice method
f44a83e 
- adds array.prototype.splice
- todo: fix stalls at certain testcases

Closes boa-dev#36
@neeldug
fix rust_fmt
02b431e
@neeldug
add docs
272a756
@neeldug
feat(boa): adds splice method
e8e4317 
- adds array.prototype.splice
- todo: fix stalls at certain testcases

Closes boa-dev#36
@neeldug
fix rust_fmt
1b1173c
@neeldug
add docs
316157e
@neeldug
Cleanup due to merge
b517524
@neeldug
Cleanup due to merge
7212db4
@neeldug neeldug requested a review from HalidOdat August 5, 2021 17:24
HalidOdat
HalidOdat reviewed Aug 6, 2021
View reviewed changes
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
@neeldug neeldug requested a review from HalidOdat August 6, 2021 20:49
Razican
Razican approved these changes Aug 25, 2021
View reviewed changes
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.

Looks good to me. I added some recommendations, though. I think it needs a rebase :)

boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
@neeldug neeldug requested a review from Razican September 4, 2021 12:58
@Razican
Copy link
Member

Razican commented Sep 4, 2021 •
edited
Loading

Test262 conformance changes:

Test result master count PR count difference
Total 80,685 80,685 0
Passed 32,838 32,958 +120
Ignored 15,818 15,818 0
Failed 32,029 31,909 -120
Panics 0 0 0
Conformance 40.70% 40.85% +0.15%
Fixed tests (120): 
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-4-50.js [strict mode] (previously Failed)
test/built-ins/Object/getOwnPropertyDescriptor/15.2.3.3-4-50.js (previously Failed)
test/built-ins/Array/prototype/findIndex/array-altered-during-loop.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/findIndex/array-altered-during-loop.js (previously Failed)
test/built-ins/Array/prototype/find/array-altered-during-loop.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/find/array-altered-during-loop.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/set_length_no_args.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/set_length_no_args.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/prop-desc.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/prop-desc.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/length-and-deleteCount-exceeding-integer-limit.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/length-and-deleteCount-exceeding-integer-limit.js (previously Failed)
test/built-ins/Array/prototype/splice/call-with-boolean.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/call-with-boolean.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species-poisoned.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species-poisoned.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species-abrupt.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species-abrupt.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T5.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T5.js (previously Failed)
test/built-ins/Array/prototype/splice/length-exceeding-integer-limit-shrink-array.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/length-exceeding-integer-limit-shrink-array.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T6.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T6.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.5_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.5_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.3_T5.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.3_T5.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.5_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.5_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T5.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T5.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T5.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T5.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/15.4.4.12-9-c-ii-1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/15.4.4.12-9-c-ii-1.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species.js (previously Failed)
test/built-ins/Array/prototype/splice/length-near-integer-limit-grow-array.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/length-near-integer-limit-grow-array.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/create-proto-from-ctor-realm-non-array.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-proto-from-ctor-realm-non-array.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2_T4.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2_T4.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A6.1_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A6.1_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species-neg-zero.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species-neg-zero.js (previously Failed)
test/built-ins/Array/prototype/splice/create-non-array.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-non-array.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T4.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.1_T4.js (previously Failed)
test/built-ins/Array/prototype/splice/clamps-length-to-integer-limit.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/clamps-length-to-integer-limit.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A3_T1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A3_T1.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.3_T4.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.3_T4.js (previously Failed)
test/built-ins/Array/prototype/splice/target-array-with-non-writable-property.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/target-array-with-non-writable-property.js (previously Failed)
test/built-ins/Array/prototype/splice/15.4.4.12-9-a-1.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/15.4.4.12-9-a-1.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A3_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A3_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.3_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.3_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T2.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T2.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species-undef.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species-undef.js (previously Failed)
test/built-ins/Array/prototype/splice/create-ctor-poisoned.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-ctor-poisoned.js (previously Failed)
test/built-ins/Array/prototype/splice/called_with_one_argument.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/called_with_one_argument.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T6.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T6.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T5.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T5.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T4.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.1_T4.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T3.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A4_T3.js (previously Failed)
test/built-ins/Array/prototype/splice/name.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/name.js (previously Failed)
test/built-ins/Array/prototype/splice/create-non-array-invalid-len.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-non-array-invalid-len.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T4.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A1.4_T4.js (previously Failed)
test/built-ins/Array/prototype/splice/create-species-null.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/create-species-null.js (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T4.js [strict mode] (previously Failed)
test/built-ins/Array/prototype/splice/S15.4.4.12_A2.2_T4.js (previously Failed)

@Razican
Copy link
Member

Razican commented Sep 4, 2021

It seems that the test262 submodule is outdated. Could you do "git submodule update"?

@neeldug neeldug requested a review from Razican September 4, 2021 14:05
jedel1043
jedel1043 requested changes Sep 4, 2021
View reviewed changes
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.

Good job! I have some advices to improve the readability. Everything else looks great!

boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
@neeldug neeldug requested a review from jedel1043 September 4, 2021 17:00
jedel1043
jedel1043 approved these changes Sep 5, 2021
View reviewed changes
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.

Great work! Pretty weird how the spec manipulated the reversed indexes. Maybe we will open a PR to improve those loops on the spec.

raskad
raskad approved these changes Sep 6, 2021
View reviewed changes
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 good!

@raskad raskad merged commit e9d7ac9 into boa-dev:master Sep 6, 2021
@neeldug neeldug deleted the impl-splice branch September 6, 2021 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jedel1043 jedel1043 jedel1043 approved these changes

@raskad raskad raskad approved these changes

@HalidOdat HalidOdat Awaiting requested review from HalidOdat

@Razican Razican Awaiting requested review from Razican

Assignees
No one assigned
Labels
builtins PRs and Issues related to builtins/intrinsics
Projects
None yet
Milestone
v0.13.0
Development

Successfully merging this pull request may close these issues.

Implement Array prototype methods
5 participants
@neeldug @Razican @HalidOdat @raskad @jedel1043