Normative: Use array indices instead of integer indices in OrdinaryOwnPropertyKeys

[mathiasbynens]

All major JavaScript engines agree on the following behavior:

$ eshost -e 'Reflect.ownKeys({ a: 1, [Number.MAX_SAFE_INTEGER]: 1, 42: 1, [2**32-1]: 1, [2**32-2]: 1 })'

#### Chakra
42,4294967294,a,9007199254740991,4294967295

#### V8 --harmony
42,4294967294,a,9007199254740991,4294967295

#### V8
42,4294967294,a,9007199254740991,4294967295

#### JavaScriptCore
42,4294967294,a,9007199254740991,4294967295

#### SpiderMonkey
42,4294967294,a,9007199254740991,4294967295

That is, the order is:

• array indices, in ascending numeric index order
• strings that are not array indices, in ascending chronological creation order
• symbols, in ascending chronological creation order

This patch makes the spec for OrdinaryOwnPropertyKeys match Web reality.

[IgnoredAmbience]

test262 was recently changed to match spec behaviour in tc39/test262#1580 the corresponding test will need to be changed to match the above.

[ChALkeR]

> Reflect.ownKeys({ a: 1, [Number.MAX_SAFE_INTEGER]: 1, '9z':3, 42: 1, [2**32-1]: 1, [2**32-2]: 1 })
[ '42', '4294967294', 'a', '9007199254740991', '9z', '4294967295' ]

2. strings that are not integer indices, in ascending chronological creation order

s/integer/array/?

[ChALkeR]

Count Qt in, as they are going to implement it as the browsers have it now (and not as written in spec), ref.
This is also where I first noticed the inconsistency and why I brought it up in #tc39 IRC.

[ljharb]

Is this also true for Object.keys/values/entries, and Object.getOwnPropertyNames? Does it impact for..in at all (i wouldn’t assume it does)? What about Object.assign enumeration order, and object spread/rest?

[leobalter]

I wonder if @allenwb knows some context for this, which is spec'ed this way since ES2015.

I'm not sure if we need to call it Web Reality if the change was intentional, just not yet updated in the major implementations.

This is all curiosity, I'm fine with any results from this PR.

[mathiasbynens]

@ChALkeR Good catch! I fixed that mistake in the commit description.

@ljharb Yeah, I suspect most uses of “integer index” vs. “array index” are similarly affected. As promised in https://bugs.chromium.org/p/v8/issues/detail?id=7874#c9, I’ll look into that tomorrow and submit individual PRs as appropriate.

[ChALkeR]

For example of other uses: what about #sec-string-exotic-objects-ownpropertykeys, which is very close to #sec-ordinaryownpropertykeys?

[allenwb]

I wonder if @allenwb knows some context for this, which is spec'ed this way since ES2015.

Yes, because this is about Ordinary Objects, not Array Exotic Objects. For Ordinary Objects is nothing special about integer property keys below or above 2³²-1. So, there is no reason to treat property keys 2³²-1 differently when enumerating them as property keys. Why would a user expect that behavior? They might be using an Ordinary Object to define an "array-like" object specifically to get around the 2³² limit.

It sounds to me like implementations are trying to use the same the same logic to optimize both Ordinary Objects and Arrays but that wasn't the intent of the ES6 spec.

[mathiasbynens]

I encourage everyone to be pragmatic here.

• The current spec cannot be implemented in existing engines without major changes, and implementing it would incur a significant performance cost.
• All engines agree on their current behavior.

Let’s just fix the spec?

We can then propose changing the behavior of existing engines if there is a strong motivation for it. What would we gain with this change, and does it pay for itself?

---

In cases like this, IMHO the burden should not lie with Test262 authors and JS engine implementers. Instead, we should just fix the spec to match reality.

In general, if all engines violate the spec in exactly the same way, the pragmatic reaction is “let’s fix the spec to match reality” rather than “let’s expect all engines to update their implementations to match the spec”. Doing it the other way around causes unnecessary churn for everyone involved. This same problem came up a couple of times in other issues/proposals recently.

[ljharb]

@mathiasbynens although I agree that in this case we should change the spec; I don't think the long-term view is served by a default of "let's just do the pragmatic thing" - we should make the spec, and implementations, do whatever is the best thing ideologically and what is web-compatible. If there were a strong reason to prefer changing all of the engines, and if it were web compatible to do so, then that's what we should do - engine churn is a much smaller problem to solve than "the language is forever sub-par". (again, I'm speaking primarily about the other issues/proposals you referred to; I don't see a strong argument here to prefer the spec over web reality)

[mathiasbynens]

@ljharb I’m saying that being pragmatic should be preferred over theoretical purity by default. When there’s strong motivation and clear gains to be had from making a web-compatible change, then sure, we can talk about that. It sounds like you and I can agree on that?

[domenic]

In particular I'd suggest a philosophy of always aligning the spec with reality as the first step. It's important to not have your specification gain a reputation as containing fiction that does not match the real world; this leads to the unfortunate scenario we had a few years ago with the official ECMAScript spec, which then had to get patched by the "Web ECMAScript" spec that implementers and web developers had to consult to find out what was actually implemented and interoperable. If your spec gains a reputation as disconnected from reality and full of "the editor preferred it this way" decisions, the damage can be serious.

Then, from a solid foundation, you can proceed to take proposals for changing all implementations through the usual proposal or normative-PR process. Don't skip that process, by saying "in the past we snuck this in without implementer buy-in, so now we're keeping it!"

[zenparsing]

I'm very uncomfortable with the direction that this discussion is taking. Philosophical positions that devalue the specification can easily lead to situations where a single engine that conforms to the spec gets thrown under the bus (as I believe is happening to Chakra in another PR).

[allenwb]

@domenic

In particular I'd suggest a philosophy of always aligning the spec with reality as the first step.

And how would that have worked, when these parts of ES6 were being specified. This was really all about specifying property enumeration order and at that time, different implementations in reality exposed differing orders. So, what should TC39 have done? Continued to leave it unspecified until "browser game theory" forced all browsers to align with whatever the most popular browser currently did. But they had already had 15 years and it hadn't happened. Why should we have expected anything to change without TC39 leading the way?

It's important to not have your specification gain a reputation as containing fiction that does not match the real world; this leads to the unfortunate scenario we had a few years ago with the official ECMAScript spec, which then had to get patched by the "Web ECMAScript" spec

In whose reality did this occur? Browsers (even IE) actually did a quite good (not perfect, but good) job of implementing what was actually specified in ES1-3. But those specifications intentionally, at the behest of browser venders, left some things un or under specific in order to accommodate existing implementation variation. But that is not what the majority of "Web ECMAScript" as first published was about. Most of what is in that document is features that simply were not included in ES5 either because they were specific to the web (ECMA-262 was a host independent spec, not a web spec) or because they did not meet the "reality" requirement of reflecting what (3 out of 4 major implementations did (and not that Chrome was not there when many of those decisions were made). So, I don't know what you are thinking about when you suggest that the ES5 spec. contained a lot of fiction.

Above all of this is probably a difference in opinion of the role of language implementors and language designers. Language designer need to think about feature interactions, the user's conceptual models of the language, and how the language is likely to evolve over time. Implementors need to be engineering pragmatists whose job is to figure out how to most economically and effectively implement a language feature. Language designers and implementors tend to make different design trade-off because they have different goals.

Most language implementors think they could be language designers and many of them could. And language implementation experience is important background for language designers. But language design and implementation are different roles and even when a person is involved in both they need to carefully think about which had they are wearing at the instance.

I find implications that the ES6 designers were somehow naive WRT to the realities of language implementations mildly offensive. We had plenty of experienced language implementors involved with ES6 and personally I have literally decades of experience as an implementor of both static and dynamic language products.

The enumeration order of properties was well thought out and fully discuss during the development of ES6. There was consensus that for all objects integer properties would enumerate first and in order. This is because that order made sense and is what users would expect once they say such properties enumerating first for common arrays. Nobody representing an implementation said, fine, but we want to be able to do something different our internal optimizations make it inconvenient to always produce that order.

What this patch is saying, is that some implementors either didn't read the spec or decide to ignore it and because of this we should now value the implementor convince of not having to fix their past mistake over providing to users a clean conceptual model of property enumeration order.

[ljharb]

to the earlier replies, i prefer to think about these things in this order:

1. what philosophically/ideologically would we prefer the spec/language to do, absent any differing implementations?
2. what do browsers actually do?
3. If they differ, is it web compatible and implementable to have the browsers change to match the first answer?
   1. If so, even if all the browsers agree, the spec should match (stay, or change to) the first answer while the change is attempted.
   2. If not, or if the attempt fails, the spec should change to match web reality.

[allenwb]

@mathiasbynens

I encourage everyone to be pragmatic here.
. The current spec cannot be implemented in existing engines without major changes, and implementing it would incur a significant performance cost.

Supporting evidence for this assertion??

IME this is the classic push-back from implementers who just don't want to be bothered to make what they consider to be an unnecessary fix or change. Heck, I've even used it my self. And I'm, pretty sure it is something I heard when I told the old IE team that for web compat they needed to enumerate properties in insertion order.

Concretely , consider this console output:
Object.keys({length: 100, 5: 0, 0:0})
(3) ["0", "5", "length"]
Object.keys({length: 100, 5000000: 0, 0:0})
(3) ["0", "5000000", "length"]
Object.keys({length: 100, 5000000000: 0, 0:0})
(3) ["0", "length", "5000000000"]

What reasonable conceptual model do you expect a user to develop to explain these results?

Implementors, If you can't get the 3rd expression to conform to the specification you aren't trying hard enough. If you can get it to conform then you are most of the way there to making it work with various optimized object representation that optimize the storage of some (but perhaps not all) integer-indexed properties.

[mathiasbynens]

@allenwb

Concretely, consider this console output:

Object.keys({ length: 100, 5: 0, 0: 0 });
// ['0', '5', 'length']
Object.keys({ length: 100, 5000000: 0, 0: 0 });
// ['0', '5000000', 'length']
Object.keys({ length: 100, 5000000000: 0, 0: 0 });
// ['0', 'length', '5000000000']

What reasonable conceptual model do you expect a user to develop to explain these results?

Using integer indices instead of array indices doesn’t solve that problem; it just moves the point at which user confusion occurs from one number to another. With the current spec text, the results would be just as surprising to users:

Object.keys({ length: 42, 9007199254740991: 1, 9007199254740992: 2 });
// ['9007199254740991', 'length', '9007199254740992']

Anyhow, can we please stop talking about the justification for having the current spec text? That is a separate discussion altogether, which — as you pointed out — has already been had, in committee, before ES2015 was finalized.

The purpose of this discussion thread is to figure out what to do about this piece of spec text that, years after it was added, has no implementations.

Do we keep the spec, knowing it doesn’t reflect what implementations actually do?
Or do we fix the spec, aligning it with reality, and then move the orthogonal discussion on what to do next to a separate thread?

[leobalter]

Do we keep the spec, knowing it doesn’t reflect what implementations actually do?
Or do we fix the spec, aligning it with reality, and then move the orthogonal discussion on what to do next to a separate thread?

I support the options presented as strategies but I have a different point of view I could rephrase using different words for the second option

• "Do we change the spec, aligning it with the current implementations?"

and then move the orthogonal discussion on what to do next to a separate thread?

If the idea is to rediscuss if we want this for the specs, I think it's just too much work to remove it to land it again to the specs later. If we remove it, we should express we're not intending to discuss this again.

My preference is to keep the spec for now, we didn't have tests before and now we have them to support any implementation. If web reality is anything not yet implemented, that's not a problem or not something new since ES2015 was released.

My own concept of web reality is the case where implementations tell TC39 something is not possible to implement, when a spec might break the web. I don't think this change would harm anything and it feels like it's possible to at least try implementing. I wonder if other implementations would try to follow the current specs considering we finally covered them with the tests in Test262. We can keep track of it.

@mathiasbynens you are the voice for implementing this on V8. I'll trust you if you tell the V8 team tried but found valid points to not implement it.

The only other case I'd support removing this from the specs, would be a expressed voice of all 4 major implementations saying they will not even try to implement. I'd also be upset, I think it's worth a try.

Again, if this is removed from the specs, I hope it's for not bringing this back again as a proposal or a PR for a long while.

[littledan]

It's great to see this effort to align implementations and the specification. Thanks for your work here. In deciding whether to change the specification or the implementations, I think it's useful to look at the details of the particular case.

@mathiasbynens

The current spec cannot be implemented in existing engines without major changes, and implementing it would incur a significant performance cost.

@leobalter

My preference is to keep the spec for now, we didn't have tests before and now we have them to support any implementation.

Sometimes, many engines disagree with the specification simply because they didn't know better, since the wording was subtle and/or there weren't any tests. This may be the sort of case @leobalter is getting at, and I agree that tests and implementation fixes can sometimes be enough to work through the situation.

Sometimes, the specification is a bit impractical or unreasonable from some perspective or other; if it's true that implementing this change would require major changes or have significant performance cost, as @mathiasbynens says, then that's a strong reason to make a change.

@mathiasbynens Could you elaborate on the implementation concerns here? That could help piece this out on concrete technical grounds, and pull us down from this higher-level discussion of what a specification is.

[mathiasbynens]

JavaScript engines use separate backing stores for array elements (i.e. array-indexed properties with default property attributes) and other properties. The property 4294967294 (i.e. the largest possible array index) is stored in the Elements backing store, and e.g. 4294967295 (i.e. the smallest integer index that is not an array index) is stored in the Properties backing store. (The latter is a simplification, as engines actually use either Shapes or Dictionaries depending on the situation, but that’s not relevant here.) Entries in the Elements backing store are ordered by numerical index. Entries in the Properties backing store are ordered by insertion time.

When enumerating properties, engines do the following (in pseudo-notation):

1. loop through the Elements backing store (cfr. “array indices, in ascending numeric index order”), and store the keys in A
2. loop through the Properties backing store,
   1. …store non-symbol keys (cfr. “strings that are not array indices, in ascending chronological creation order”) in B
   2. …store symbol keys (cfr. “symbols, in ascending chronological creation order”) in C
3. return [...A, ...B, ...C]

To match the current spec, engines would have to:

1. loop through the Elements backing store (cfr. part 1 of “integer indices, in ascending numeric index order”), and store the keys in A
2. loop through the Properties backing store,
   1. …store any properties that are integer indices (cfr. part 2 of “integer indices, in ascending numeric index order”) in B
   2. …store any non-symbols separately (cfr. “strings that are not integer indices, in ascending chronological creation order”) in C
   3. …store any symbols separately (cfr. “symbols, in ascending chronological creation order”) in D
3. sort B by numeric index order, ascending
4. return [...A, ...B, ...C, ...D]

2.i. is the most expensive part, since engines need to parse all the keys (which are strings) to check if they happen to be numeric, and if so, check if they’re within the integer index range.

As long as “array index” means something else than “integer index”, this is not a trivial change to make.

Increasing the max array length to align “array index” with “integer index” would be a more consistent choice, but that would come with different implementation challenges.

[littledan]

Thanks for these detailed explanations, @mathiasbynens and @jakobkummerow . Based on this information, the PR sounds like a good idea to me.

[bterlson]

FWIW I agree we need to document web reality first in most cases. Larger deviations, or deviation that impact only a subset of browsers, are potentially different matters. But things like this should just be fixed I think.

Since there's much discussion, let's make this a needs-consensus PR and we can discuss in a couple weeks?

[ajklein]

Looks like this was expected to be discussed at the July meeting, but wasn't? What can we do to make sure that the September meeting covers any needs-consensus PRs (including this one)?

[ljharb]

@ajklein adding it to the "Needs Consensus PRs" section on the september agenda should accomplish that (it wasn't added to the july agenda)

[ajklein]

@ljharb @bterlson @bmeck Who is expected to do that, in general? The PR owner? Previously it seemed like Brian would be responsible for tracking and going through any needs-consensus PRs. Ideally that's a process that I would like the editor group to continue. Brian, please correct me if my memory is wrong.

[bterlson]

@ajklein it's a bit of both to be honest. I (or the editor group) can (and does) push needs-consensus PRs but sometimes we don't have all the context and for such cases it's much better if the people with the context present their case. We'll do a better job of communicating this in the future.

[ajklein]

@bterlson Thanks for the context; and apologies for the off-topic discussion on this thread (which I do hope we get a chance to decide in September).

[bakkot]

EDIT: sorry, instead of reading this comment, you should see #1243 and #1244.

---

I think the corresponding change should also be made to the overload for [[OwnPropertyKeys]] for String exotic objects.

This program:

let obj = Object('asdf');

obj.a = 1;
obj[Number.MAX_SAFE_INTEGER] = 1;
obj[42] = 1;
obj[2**32-1] = 1;
obj[2**32-2] = 1;

console.log(Reflect.ownKeys(obj).map(x => x.toString()).join(','));

outputs
0,1,2,3,42,4294967294,length,a,9007199254740991,4294967295 in every browser (except that ChakraCore has a bug where it puts 'length' at the end.)

It's less clear whether the same should be applied to the overload for Integer Indexed exotic objects. I think it's only observable there if you actually allocate a typed array with > 2**32 - 2 entries, which probably isn't going to work out too well.

Proxies of course require no changes, and there are no other overloads. (Arguments exotic objects defer to the ordinary definition for this method.)

[ljharb]

@mathiasbynens @bakkot it’d still be ideal if all of the potential user-observable changes as a result of this PR could be summarized in a single, exhaustive place. Any chance someone has the time for that? :-)

[mathiasbynens]

@ljharb Searching the spec for [[OwnPropertyKeys]] and then following the trail of breadcrumbs results in the following list of potentially-impacted APIs:

• Reflect.ownKeys
• Object.assign
• Object.getOwnPropertyDescriptors
• Object.entries
• Object.keys
• Object.values
• Object.create
• Object.freeze / Object.isFrozen
• Object.seal / Object.isSealed
• for (foo in bar) ...
• JSON.parse (through InternalizeJSONProperty which can call EnumerableOwnPropertyNames)
• JSON.stringify (through SerializeJSONProperty which calls SerializeJSONObject/SerializeJSONArray which then calls EnumerableOwnPropertyNames)
• ...did I miss any?

This is an academic exercise, though. It's not really an "observable change" if no one implemented the old behavior in the first place.