New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2015
Cc:
Components:
HW: ----
NextAction: ----
OS: ----
Priority: 2
Type: Bug



Sign in to add a comment
link

Issue 4118: Object.getOwnPropertyNames returns names in wrong order

Reported by arv@chromium.org, May 15 2015 Project Member

Issue description

Comment 2 by arv@chromium.org, May 15 2015

To be clear:

  var str = new String("abc");
  str[5] = "de";

  var expected = ["0", "1", "2", "5", "length"];
  var actual = Object.getOwnPropertyNames(str);

our actual is

  ["5", "0", "1", "2", "length"];

I assume this is because "5" becomes a non indexed property?

Comment 3 by adamk@chromium.org, Jul 14 2015

Cc: -arv@chromium.org rossberg@chromium.org
Owner: adamk@chromium.org
Status: Started

Comment 4 by verwa...@chromium.org, Jul 15 2015

5 is an indexed property, but the mapped characters are only virtually present. I guess we mix those in at the wrong time. I presume the same is true for arguments objects with added or reconfigured elements, given that also there there are 2 elements backing stores; and those are even more complex to merge. In both cases there's always a single correct holder though, so we don't need to check for duplicates. (I fixed some bad cornercases wrt to string wrappers obtaining duplicates recently).

Comment 5 by bugdroid1@chromium.org, Jul 15 2015

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/1e146c07088d4e0f5c25177be6b2245b562cb929

commit 1e146c07088d4e0f5c25177be6b2245b562cb929
Author: adamk <adamk@chromium.org>
Date: Wed Jul 15 07:31:26 2015

[es6] JSObject::GetOwnElementKeys should collect String wrapper keys first

This makes Object.getOwnPropertyNames() return the integer keys in the
proper order, following the spec:

http://www.ecma-international.org/ecma-262/6.0/#sec-ordinary-object-internal-methods-and-internal-slots-ownpropertykeys

BUG= v8:4118 
LOG=n

Review URL: https://codereview.chromium.org/1228803006

Cr-Commit-Position: refs/heads/master@{#29667}

[modify] http://crrev.com/1e146c07088d4e0f5c25177be6b2245b562cb929/src/objects.cc
[modify] http://crrev.com/1e146c07088d4e0f5c25177be6b2245b562cb929/test/test262-es6/test262-es6.status

Comment 6 by adamk@chromium.org, Jul 15 2015

Cc: verwa...@chromium.org
Status: Fixed
I don't see any issue with arguments:

d8> var f = function() { arguments[5] = 4; return Object.getOwnPropertyNames(arguments) }
undefined
d8> f(1, 2)
["0", "1", "5", "length", "callee"]
d8> f = function() { Object.defineProperty(arguments, 75, {enumerable: false, value: 42}); return Object.getOwnPropertyNames(arguments) }
function () { Object.defineProperty(arguments, 75, {enumerable: false, value: 42}); return Object.getOwnPropertyNames(arguments) }
d8> f(1, 2, 3)
["0", "1", "2", "75", "length", "callee"]

Closing. Toon, feel free to reopen if you have a test case that doesn't do the right thing WRT arguments.

Comment 7 by verwa...@chromium.org, Jul 15 2015

No that was written from my phone before looking at the code. It doesn't seem pretty, but seems to work afaict. Thanks for fixing.

Comment 8 by mridg...@yahoo-inc.com, Aug 18 2015

I am seeing strange ordering for getOwnPropertyNames as well:

var t = {}; 'abcdefghijklmnopqrs'.split('').forEach(function (letter) { t[letter] = letter; }); Object.getOwnPropertyNames(t);
> ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s"]

var t = {}; 'abcdefghijklmnopqrst'.split('').forEach(function (letter) { t[letter] = letter; }); Object.getOwnPropertyNames(t);
> ["g", "b", "h", "a", "t", "c", "f", "r", "k", "e", "p", "l", "q", "n", "s", "m", "o", "j", "i", "d"]

There seems to be a specific object size where the properties get shifted around.

Comment 9 by adamk@chromium.org, Aug 18 2015

Status: Assigned
Thanks for the report; this is due to normalization from fast properties to dictionary mode.

Apparently we don't do the proper magic in getOwnPropertyNames to order the keys by insertion order. Should be relatively easy to fix, since we do the right thing for for..in.

Comment 11 by adamk@chromium.org, Aug 18 2015

Status: Fixed
Closing this in preference to  issue 3056  (as this issue was originally about indexed properties, which are fixed).

Comment 12 by hablich@chromium.org, Mar 23 2017

Labels: Priority-2

Sign in to add a comment