GetMethod should not box the receiver argument

RESOLVED FIXED in Firefox 48

Status

()

Core
JavaScript: Standard Library
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: anba, Assigned: evilpie)

Tracking

(Blocks: 1 bug)

Trunk
mozilla48
Points:
---

Firefox Tracking Flags

(firefox43 affected, firefox48 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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"`
(Assignee)

Updated

2 years ago
Blocks: 694100

Comment 1

2 years ago
I guess this is due to Bug 603201.
(Reporter)

Comment 2

2 years ago
(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
(Assignee)

Updated

2 years ago
Assignee: nobody → evilpies
(Assignee)

Comment 3

2 years ago
Created attachment 8738948 [details] [diff] [review]
v1 Update GetMethod
Attachment #8738948 - Flags: review?(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.
Attachment #8738948 - Flags: review?(till) → review+
(Assignee)

Comment 5

2 years ago
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/
(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

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/77d7242cc337
https://hg.mozilla.org/mozilla-central/rev/77d7242cc337
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.