Reflect.ownKeys: order with large integer "index" keys

[rwaldron]

No description provided.

[ChALkeR]

Is there any engine that implements it correctly per spec that way?
Perhaps s/integer index/array index/ in the spec for OwnPropertyKeys (and similar, if needed) would be easier?

[rwaldron]

Even if I spec change were made, we still need to have tests for the current specification.

[leobalter]

This should also be covered at least by Object.getOwnPropertyNames and Object.keys.

[mathiasbynens]

This test doesn't match reality:

• https://bugs.chromium.org/p/v8/issues/detail?id=7874
• Normative: Use array indices instead of integer indices in OrdinaryOwnPropertyKeys ecma262#1242

Relatedly, I somewhat disagree with this statement:

Even if I spec change were made, we still need to have tests for the current specification.

Having tests is great! However, if it turns out all major engines fail a new test in exactly the same way, then updating the spec to match reality is preferred over merging the test, IMHO.

[rwaldron]

@mathiasbynens there is presently a gap in coverage of the spec, which I'm filling. If there had been a test that caught this behavior, written in 2014-2015, it's likely that all implementations would've implemented it correctly (correctly could mean "as it is presently specified" or "according to some newly revised specification").

If there is sufficient support for changing the spec, then it would have to go through the "needs consensus" process. If that happens, we'll immediately update this test to reflect the expected behavior. In the meantime, runtimes are welcome to fix the bug or add this test to their respective "skip lists".

[mathiasbynens]

@rwaldron I very much appreciate your work on increasing test coverage — in fact, it’s because of your efforts that this discrepancy was discovered in the first place!

If there had been a test that caught this behavior, written in 2014-2015, it's likely that all implementations would've implemented it correctly

I can tell you that at least V8 would not have implemented the currently spec’d behavior. JavaScript engines commonly store array elements (i.e. values mapped to array indices) separately from other properties (such as integer indices outside of the array index range). As such, using array indices in some parts of the spec but then suddenly using the much wider integer index range elsewhere would greatly complicate the implementation. The current spec cannot be implemented in existing engines without major changes, and it would incur a significant performance cost.

In general, if all engines violate the spec in exactly the same way, I’d like the assumption to be “let’s fix the spec to match reality” rather than “let’s merge this test, expect all engines to investigate it individually, add it to their skip lists, and then hopefully update their implementations to match the spec”. Doing it the other way around causes unnecessary churn for Test262 contributors such as yourself as well as for JS engine developers.