1923146 - [Outreachy] Add placeholder implementation for Iterator.Zip

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Dan Minor [:dminor]]

This bug is about adding a placeholder implementation for the Iterator.Zip method of the joint iteration proposal. The steps to do this are:

1. Make sure that you are able to build SpiderMonkey and run the existing tests. There are instructions here. You want to run the jstests, using this command ./mach jstests.
2. In Iterator.js add a placeholder method for IteratorZip. We use NIGHTLY_BUILD so this will only be built for Nightly, and will not be available in release. Returning false means this method will fail all the time for now.

#ifdef NIGHTLY_BUILD
/**
 * Iterator.zip ( iterables [, options] )
 *
 * https://tc39.es/proposal-joint-iteration/#sec-iterator.zip
 */
function IteratorZip(predicate) {
  return false;
}
#endif

3. Add the new function as a static method on Iterators. Please use #ifndef NIGHTLY_BUILD / #endif here as well.
4. You can check that your new method is available in the shell, e.g. print(Iterator.zip) from the JavaScript shell.
5. The final step is to remove it from the Iterator object when the pref for joint iteration is not enabled. There's an example of how to do this in this patch, you'll want to do a check on JSProto_Iterator. Note that you'll have to add zip to CommonPropertyNames.h.
6. Make sure you can still build and run the tests.
7. Once the tests all pass for you, you can commit your changes. Your commit message should look like: Bug X - Add placeholder for Iterator.zip; r=dminor!, where X is the number for this bug.
8. Once you have your changes committed, request review. There are instructions on using phabricator and on getting reviews.

Comment 1

[Syed Bariman Jan]

:dminor, I am working on it.

Comment 2

[Dan Minor [:dminor]]

Let's save this one for a new contributor, I know that you are capable of doing this one already :)

Comment 3

[Serah Nderi]

I'd like to work on this issue.

Comment 4

[tawanda]

(In reply to Serah Nderi from comment #3)

I'd like to work on this issue.

Hi Serah, already working on this issue

Comment 5

[tawanda]

Attachment: Bug 1923146 - Add placeholder for Iterator.zip; r=dminor!

Comment 6

[Dan Minor [:dminor]]

Hi Tawanda, your changes are good, but need to be rebased on the latest mozilla-central because of merge conflicts (https://lando.services.mozilla.com/D224921/) before I can land this.

Comment 7

[tawanda]

Thanks, rebasing

Comment 8

[tawanda]

(In reply to Dan Minor [:dminor] from comment #6)

Hi Tawanda, your changes are good, but need to be rebased on the latest mozilla-central because of merge conflicts (https://lando.services.mozilla.com/D224921/) before I can land this.

I have pushed another revision/diff , is that sufficient ?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a247701650bc
Add placeholder for Iterator.zip; r=dminor

Comment 10

[Syed Bariman Jan]

:dminor, This is removing the math_sumprecise conditional in the JSObject.cpp file.

Comment 11

[Dan Minor [:dminor]]

Oops, I missed that, and it's already landed. I'll open a new bug.

Comment 12

[Serah Nderi]

Attachment: Bug 1923146 - Re-add sumPrecise check. r=syedbarimanjan

Comment 13

[Serah Nderi]

Attachment: Bug 1923146 - Re-add sumPrecise check. r=syedbarimanjan

Comment 14

[Serah Nderi]

Attachment: Bug 1923146 - Re-add sumPrecise check. r=syedbarimanjan

Comment 15

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/a247701650bc

Comment 16

[Phabricator Automation]

Comment on attachment 9430009 [details]
Bug 1923146 - Re-add sumPrecise check. r=syedbarimanjan

Revision D225134 was moved to bug 1923710. Setting attachment 9430009 [details] to obsolete.