Bug 144188 - Set#forEach does not pass "key" or "set" arguments to callback.
Summary: Set#forEach does not pass "key" or "set" arguments to callback.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jordan Harband
URL: https://people.mozilla.org/~jorendorf...
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-24 23:15 PDT by Jordan Harband
Modified: 2015-04-26 15:55 PDT (History)
6 users (show)

See Also:


Attachments
Patch (4.40 KB, patch)
2015-04-25 00:10 PDT, Jordan Harband
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (561.89 KB, application/zip)
2015-04-25 00:46 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews101 for mac-mavericks (529.25 KB, application/zip)
2015-04-25 00:58 PDT, Build Bot
no flags Details
Patch (4.38 KB, patch)
2015-04-26 14:20 PDT, Jordan Harband
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jordan Harband 2015-04-24 23:15:30 PDT
Per https://people.mozilla.org/~jorendorff/es6-draft.html#sec-set.prototype.foreach, the Set#forEach callback should pass 3 arguments to the callback.

Currently, it passes only 1.
Comment 1 Jordan Harband 2015-04-25 00:10:08 PDT
Created attachment 251617 [details]
Patch
Comment 2 Build Bot 2015-04-25 00:46:27 PDT
Comment on attachment 251617 [details]
Patch

Attachment 251617 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4715717987401728

New failing tests:
js/set-foreach-calls-back-with-right-args.html
Comment 3 Build Bot 2015-04-25 00:46:30 PDT
Created attachment 251620 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 4 Build Bot 2015-04-25 00:58:44 PDT
Comment on attachment 251617 [details]
Patch

Attachment 251617 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6619609880854528

New failing tests:
js/set-foreach-calls-back-with-right-args.html
Comment 5 Build Bot 2015-04-25 00:58:47 PDT
Created attachment 251623 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 6 Darin Adler 2015-04-26 12:05:33 PDT
Comment on attachment 251617 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=251617&action=review

> LayoutTests/js/script-tests/set-foreach-calls-back-with-right-args.js:11
> +    print(value === 'value' ? 'PASS value is "value"' : 'FAIL value is ' + value);
> +    print(key === 'value' ? 'PASS key is "value"' : 'FAIL key is ' + key);
> +    print(set === s ? 'PASS set arg is set' : 'FAIL set arg is ' + set);
> +    print(this === receiver ? 'PASS "this" is "receiver"' : 'FAIL "this" is ' + this);

Not sure where you’re expecting the "print" function to come from. I don’t  think there is such a function. Instead the code should be more like this:

    shouldBe("value", "'value'");
    shouldBe("key", "'value'");
    shouldBe("set", "s");
    shouldBe("this", "receiver");
Comment 7 Jordan Harband 2015-04-26 12:07:18 PDT
Thanks for the review! I found "print" in another test, and I was unable to get `shouldBe` to pass properly. `print`, however, passed just fine locally. It appears to be like `debug`.

I'll give it another try with `shouldBe`.

(In reply to comment #6)
> Comment on attachment 251617 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251617&action=review
> 
> > LayoutTests/js/script-tests/set-foreach-calls-back-with-right-args.js:11
> > +    print(value === 'value' ? 'PASS value is "value"' : 'FAIL value is ' + value);
> > +    print(key === 'value' ? 'PASS key is "value"' : 'FAIL key is ' + key);
> > +    print(set === s ? 'PASS set arg is set' : 'FAIL set arg is ' + set);
> > +    print(this === receiver ? 'PASS "this" is "receiver"' : 'FAIL "this" is ' + this);
> 
> Not sure where you’re expecting the "print" function to come from. I don’t 
> think there is such a function. Instead the code should be more like this:
> 
>     shouldBe("value", "'value'");
>     shouldBe("key", "'value'");
>     shouldBe("set", "s");
>     shouldBe("this", "receiver");
Comment 8 Darin Adler 2015-04-26 12:23:10 PDT
(In reply to comment #7)
> I found "print" in another test

Which test?
Comment 9 Jordan Harband 2015-04-26 14:20:26 PDT
Created attachment 251706 [details]
Patch
Comment 10 Jordan Harband 2015-04-26 14:22:35 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > I found "print" in another test
> 
> Which test?

map-iterate-first-entry-is-a-hole.js. It looks like since the callback is never actually called, that the line might not be testing things correctly, and should instead throw an exception or something.
Comment 11 WebKit Commit Bot 2015-04-26 15:55:34 PDT
Comment on attachment 251706 [details]
Patch

Clearing flags on attachment: 251706

Committed r183371: <http://trac.webkit.org/changeset/183371>
Comment 12 WebKit Commit Bot 2015-04-26 15:55:39 PDT
All reviewed patches have been landed.  Closing bug.