1204562 - GetMethod should not box the receiver argument

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, defect)

Description

[André Bargull [:anba]]

Test case:
---
Object.defineProperty(Number.prototype, Symbol.iterator, {
  get() {
    "use strict";
    assertEq(typeof this, "number");
    return () => [this][Symbol.iterator]();
  }, configurable: true
});

Array.from(1);
---

Expected: assertEq test passes
Actual: assertEq throws `Error: Assertion failed: got "object", expected "number"`

Comment 1

[Claude Pache]

I guess this is due to Bug 603201.

Comment 2

[André Bargull [:anba]]

(In reply to Claude Pache from comment #1)
> I guess this is due to Bug 603201.

No, GetMethod performs an explicit ToObject [1] which needs to be removed. 

[1] https://dxr.mozilla.org/mozilla-central/rev/031db40e2b558c7e4dd0b4c565db4a992c1627c8/js/src/builtin/Utilities.js?offset=0#127

Comment 3

[Tom S]

Attachment: v1 Update GetMethod

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 8738948 [details] [diff] [review]
v1 Update GetMethod

Review of attachment 8738948 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/Utilities.js
@@ +110,5 @@
>  function SameValueZero(x, y) {
>      return x === y || (x !== x && y !== y);
>  }
>  
> +// ES8 rev 643a7ff106e3a1fd879e88b53675047502343f51

Even though this is more precise, I'd much prefer having a date in here. I.e. the date shown in the currently published draft. Without permanent links, the rev is admittedly a bit easier to navigate to, but only in the almost entirely unusable source view, anyway.

Also, "ES8" isn't right on two counts: it makes less and less sense, I'm afraid because the official naming scheme has changed, and if anything it should be "ES7". Please change it to "ES2016".

::: js/src/tests/ecma_6/Array/from_primitive.js
@@ +1,2 @@
> +/* Any copyright is dedicated to the Public Domain.
> + * http://creativecommons.org/licenses/publicdomain/ */

Not that it hurts or anything, but a CC0 header isn't required, as it's the default license for tests.

Comment 5

[Tom S]

Yeah we should probably stop using the abbreviations. However we started using the rev ids recently, I still hope we will get a way to view specific revisions as HTML. ES2016 is wrong, we are on ES2017 already see https://tc39.github.io/ecma262/

Comment 6

[Till Schneidereit [:till]]

(In reply to Tom Schuster [:evilpie] from comment #5)
> Yeah we should probably stop using the abbreviations. However we started
> using the rev ids recently

We still do have a mix of both styles, and I very much prefer the date :)

> I still hope we will get a way to view specific
> revisions as HTML. ES2016 is wrong, we are on ES2017 already see
> https://tc39.github.io/ecma262/

Right, I conflated two things: the GetMethod change was introduced earlier (in ES2016), but the current draft is of course ES2017. Since we don't have a way to get a permanent link to either, I agree that we should use the latest one, i.e. 2017.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/77d7242cc337

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/77d7242cc337