[Merged by Bors] - Implement Array.from #1831

nrabulinski · 2022-02-09T20:20:11Z

This Pull Request fixes/closes #1784.

There're still a few tests failing, notably:

iter-set-elem-prop-non-writable - we don't have generator functions implemented
calling-from-valid-1-noStrict, iter-map-fn-this-non-strict - thisArg in non-strict mode, when undefined, should be inherited (that's what I'm guessing, I haven't confirmed this, but strict counterparts do pass with thisArg being undefined)
source-array-boundary, elements-deleted-after - ~~Not sure yet, still investigating, but they also include thisArg, so perhaps function calling has an underlying issue?~~ Failing because this on the top level evaluates to an empty object instead of containing everything from the top scope

codecov · 2022-02-09T20:29:59Z

Codecov Report

Merging #1831 (d503142) into main (128f836) will decrease coverage by 0.11%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1831      +/-   ##
==========================================
- Coverage   45.98%   45.86%   -0.12%     
==========================================
  Files         206      206              
  Lines       17057    17097      +40     
==========================================
- Hits         7844     7842       -2     
- Misses       9213     9255      +42

Impacted Files	Coverage Δ
boa_engine/src/builtins/array/mod.rs	`74.75% <0.00%> (-5.49%)`	⬇️
boa_engine/src/builtins/iterable/mod.rs	`49.01% <ø> (ø)`

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 128f836...d503142. Read the comment docs.

jedel1043

Thank you for the contribution! I have some suggestions to improve the code, if you don't mind :)

boa/src/builtins/array/mod.rs

raskad

Just got one tiny change, otherwise looks great!

boa/src/builtins/array/mod.rs

Razican

It looks pretty good! Thank you for the contribution :)

I added some comments, feel free to give them a look, but it's pretty good from my side!

boa/src/builtins/array/mod.rs

Razican · 2022-02-23T15:07:54Z

@nrabulinski please, check the comments and see if you can fix what was mentioned, and re-base the branch, so that we can merge this as soon as possible :)

HalidOdat · 2022-03-07T17:47:47Z

VM implementation

Test result	main count	PR count	difference
Total	88,410	88,410	0
Passed	43,293	43,375	+82
Ignored	21,481	21,481	0
Failed	23,636	23,554	-82
Panics	0	0	0
Conformance	48.97%	49.06%	+0.09%

Fixed tests (84):

test/built-ins/Array/from/source-object-length-set-elem-prop-non-writable.js [strict mode] (previously Failed)
test/built-ins/Array/from/source-object-length-set-elem-prop-non-writable.js (previously Failed)
test/built-ins/Array/from/elements-added-after.js [strict mode] (previously Failed)
test/built-ins/Array/from/elements-added-after.js (previously Failed)
test/built-ins/Array/from/get-iter-method-err.js [strict mode] (previously Failed)
test/built-ins/Array/from/get-iter-method-err.js (previously Failed)
test/built-ins/Array/from/iter-map-fn-this-non-strict.js (previously Failed)
test/built-ins/Array/from/elements-updated-after.js [strict mode] (previously Failed)
test/built-ins/Array/from/elements-updated-after.js (previously Failed)
test/built-ins/Array/from/Array.from-name.js [strict mode] (previously Failed)
test/built-ins/Array/from/Array.from-name.js (previously Failed)
test/built-ins/Array/from/array-like-has-length-but-no-indexes-with-values.js [strict mode] (previously Failed)
test/built-ins/Array/from/array-like-has-length-but-no-indexes-with-values.js (previously Failed)
test/built-ins/Array/from/source-array-boundary.js [strict mode] (previously Failed)
test/built-ins/Array/from/source-array-boundary.js (previously Failed)
test/built-ins/Array/from/source-object-length.js [strict mode] (previously Failed)
test/built-ins/Array/from/source-object-length.js (previously Failed)
test/built-ins/Array/from/source-object-missing.js [strict mode] (previously Failed)
test/built-ins/Array/from/source-object-missing.js (previously Failed)
test/built-ins/Array/from/iter-get-iter-val-err.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-get-iter-val-err.js (previously Failed)
test/built-ins/Array/from/calling-from-valid-1-noStrict.js (previously Failed)
test/built-ins/Array/from/iter-map-fn-err.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-map-fn-err.js (previously Failed)
test/built-ins/Array/from/items-is-arraybuffer.js [strict mode] (previously Failed)
test/built-ins/Array/from/items-is-arraybuffer.js (previously Failed)
test/built-ins/Array/from/from-string.js [strict mode] (previously Failed)
test/built-ins/Array/from/from-string.js (previously Failed)
test/built-ins/Array/from/iter-cstm-ctor-err.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-cstm-ctor-err.js (previously Failed)
test/built-ins/Array/from/elements-deleted-after.js [strict mode] (previously Failed)
test/built-ins/Array/from/elements-deleted-after.js (previously Failed)
test/built-ins/Array/from/source-object-iterator-1.js [strict mode] (previously Failed)
test/built-ins/Array/from/source-object-iterator-1.js (previously Failed)
test/built-ins/Array/from/iter-map-fn-this-strict.js [strict mode] (previously Failed)
test/built-ins/Array/from/from-array.js [strict mode] (previously Failed)
test/built-ins/Array/from/from-array.js (previously Failed)
test/built-ins/Array/from/this-null.js [strict mode] (previously Failed)
test/built-ins/Array/from/this-null.js (previously Failed)
test/built-ins/Array/from/iter-set-elem-prop.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-set-elem-prop.js (previously Failed)
test/built-ins/Array/from/iter-map-fn-args.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-map-fn-args.js (previously Failed)
test/built-ins/Array/from/calling-from-valid-1-onlyStrict.js [strict mode] (previously Failed)
test/built-ins/Array/from/source-object-constructor.js [strict mode] (previously Failed)
test/built-ins/Array/from/source-object-constructor.js (previously Failed)
test/built-ins/Array/from/Array.from-descriptor.js [strict mode] (previously Failed)
test/built-ins/Array/from/Array.from-descriptor.js (previously Failed)
test/built-ins/Array/from/iter-map-fn-this-arg.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-map-fn-this-arg.js (previously Failed)
test/built-ins/Array/from/source-object-iterator-2.js [strict mode] (previously Failed)
test/built-ins/Array/from/source-object-iterator-2.js (previously Failed)
test/built-ins/Array/from/Array.from_arity.js [strict mode] (previously Failed)
test/built-ins/Array/from/Array.from_arity.js (previously Failed)
test/built-ins/Array/from/iter-get-iter-err.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-get-iter-err.js (previously Failed)
test/built-ins/Array/from/iter-map-fn-return.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-map-fn-return.js (previously Failed)
test/built-ins/Array/from/calling-from-valid-2.js [strict mode] (previously Failed)
test/built-ins/Array/from/calling-from-valid-2.js (previously Failed)
test/built-ins/Array/from/iter-adv-err.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-adv-err.js (previously Failed)
test/built-ins/Array/from/mapfn-throws-exception.js [strict mode] (previously Failed)
test/built-ins/Array/from/mapfn-throws-exception.js (previously Failed)
test/built-ins/Array/from/Array.from_forwards-length-for-array-likes.js [strict mode] (previously Failed)
test/built-ins/Array/from/Array.from_forwards-length-for-array-likes.js (previously Failed)
test/built-ins/Array/from/iter-cstm-ctor.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-cstm-ctor.js (previously Failed)
test/built-ins/Array/from/iter-set-elem-prop-err.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-set-elem-prop-err.js (previously Failed)
test/built-ins/Array/from/iter-set-elem-prop-non-writable.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-set-elem-prop-non-writable.js (previously Failed)
test/built-ins/Array/from/iter-set-length-err.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-set-length-err.js (previously Failed)
test/built-ins/Array/from/iter-set-length.js [strict mode] (previously Failed)
test/built-ins/Array/from/iter-set-length.js (previously Failed)
test/built-ins/Array/from/source-object-without.js [strict mode] (previously Failed)
test/built-ins/Array/from/source-object-without.js (previously Failed)
test/built-ins/Proxy/apply/trap-is-undefined-target-is-proxy.js [strict mode] (previously Failed)
test/built-ins/Proxy/apply/trap-is-undefined-target-is-proxy.js (previously Failed)
test/built-ins/TypedArray/prototype/sort/stability.js [strict mode] (previously Failed)
test/built-ins/TypedArray/prototype/sort/stability.js (previously Failed)
test/harness/deepEqual-mapset.js [strict mode] (previously Failed)
test/harness/deepEqual-mapset.js (previously Failed)

Broken tests (2):

test/annexB/built-ins/Array/from/iterator-method-emulates-undefined.js [strict mode] (previously Passed)
test/annexB/built-ins/Array/from/iterator-method-emulates-undefined.js (previously Passed)

boa_engine/src/builtins/array/mod.rs

Razican · 2022-03-10T18:18:30Z

@nrabulinski it would be nice to get this merged during the weekend, for the 0.14 release of Boa. Do you have time to check the comments by @jedel1043?

- Make `IfAbruptCloseIterator` a pub(crate) macro since it's used in more places in the spec.

HalidOdat

The comments have been resolved :)

jedel1043

No more nitpicks :)

Razican · 2022-03-12T19:33:01Z

bors r+

This Pull Request fixes/closes #1784. There're still a few tests failing, notably: - `iter-set-elem-prop-non-writable` - we don't have generator functions implemented - `calling-from-valid-1-noStrict`, `iter-map-fn-this-non-strict` - `thisArg` in non-strict mode, when undefined, should be inherited (that's what I'm guessing, I haven't confirmed this, but strict counterparts do pass with `thisArg` being `undefined`) - `source-array-boundary`, `elements-deleted-after` - ~~Not sure yet, still investigating, but they also include thisArg, so perhaps function calling has an underlying issue?~~ Failing because `this` on the top level evaluates to an empty object instead of containing everything from the top scope Co-authored-by: HalidOdat <halidodat@gmail.com>

bors · 2022-03-12T19:40:41Z

Pull request successfully merged into main.

Build succeeded:

This Pull Request fixes/closes #1784. There're still a few tests failing, notably: - `iter-set-elem-prop-non-writable` - we don't have generator functions implemented - `calling-from-valid-1-noStrict`, `iter-map-fn-this-non-strict` - `thisArg` in non-strict mode, when undefined, should be inherited (that's what I'm guessing, I haven't confirmed this, but strict counterparts do pass with `thisArg` being `undefined`) - `source-array-boundary`, `elements-deleted-after` - ~~Not sure yet, still investigating, but they also include thisArg, so perhaps function calling has an underlying issue?~~ Failing because `this` on the top level evaluates to an empty object instead of containing everything from the top scope Co-authored-by: HalidOdat <halidodat@gmail.com>

nrabulinski force-pushed the array_from branch from d64f00d to 7dbab31 Compare February 9, 2022 20:27

jedel1043 added builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request labels Feb 10, 2022

jedel1043 added this to the v0.14.0 milestone Feb 10, 2022

jedel1043 requested changes Feb 10, 2022

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

raskad requested changes Feb 11, 2022

View reviewed changes

boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved

raskad approved these changes Feb 11, 2022

View reviewed changes

Razican approved these changes Feb 12, 2022

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

HalidOdat force-pushed the array_from branch from 7dbab31 to 3535fc8 Compare March 7, 2022 17:36

HalidOdat requested a review from jedel1043 March 7, 2022 17:37

HalidOdat force-pushed the array_from branch from 3535fc8 to 2878e36 Compare March 7, 2022 17:41

jedel1043 requested changes Mar 7, 2022

View reviewed changes

boa_engine/src/builtins/array/mod.rs Outdated Show resolved Hide resolved

boa_engine/src/builtins/array/mod.rs Outdated Show resolved Hide resolved

jedel1043 changed the title ~~Implemented Array.from~~ Implement Array.from Mar 7, 2022

nrabulinski and others added 2 commits March 12, 2022 16:48

Implemented Array.from

a7672a6

Resolve review comments

d503142

- Make `IfAbruptCloseIterator` a pub(crate) macro since it's used in more places in the spec.

HalidOdat force-pushed the array_from branch from 2878e36 to d503142 Compare March 12, 2022 15:56

HalidOdat requested a review from jedel1043 March 12, 2022 15:57

HalidOdat approved these changes Mar 12, 2022

View reviewed changes

jedel1043 approved these changes Mar 12, 2022

View reviewed changes

bors bot changed the title ~~Implement Array.from~~ [Merged by Bors] - Implement Array.from Mar 12, 2022

bors bot closed this Mar 12, 2022

HalidOdat mentioned this pull request Mar 14, 2022

generated changelog for 0.14 #1882

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Implement Array.from #1831

[Merged by Bors] - Implement Array.from #1831

nrabulinski commented Feb 9, 2022 •

edited

Loading

codecov bot commented Feb 9, 2022 •

edited

Loading

jedel1043 left a comment

raskad left a comment

Razican left a comment

Razican commented Feb 23, 2022

HalidOdat commented Mar 7, 2022

Razican commented Mar 10, 2022

HalidOdat left a comment •

edited

Loading

jedel1043 left a comment

Razican commented Mar 12, 2022

bors bot commented Mar 12, 2022

[Merged by Bors] - Implement Array.from #1831

[Merged by Bors] - Implement Array.from #1831

Conversation

nrabulinski commented Feb 9, 2022 • edited Loading

codecov bot commented Feb 9, 2022 • edited Loading

Codecov Report

jedel1043 left a comment

Choose a reason for hiding this comment

raskad left a comment

Choose a reason for hiding this comment

Razican left a comment

Choose a reason for hiding this comment

Razican commented Feb 23, 2022

HalidOdat commented Mar 7, 2022

VM implementation

Razican commented Mar 10, 2022

HalidOdat left a comment • edited Loading

Choose a reason for hiding this comment

jedel1043 left a comment

Choose a reason for hiding this comment

Razican commented Mar 12, 2022

bors bot commented Mar 12, 2022

nrabulinski commented Feb 9, 2022 •

edited

Loading

codecov bot commented Feb 9, 2022 •

edited

Loading

HalidOdat left a comment •

edited

Loading