307791 - Implement ES5's Object.keys(O)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Erik Arvidsson]

Along the lines of improving JS towards a version 1.6 I think it might be useful
to be able get the keys and values as arrays (iterators would have been better).

Basic JS implementation (with the ugly side effects of modifying Object.prototype)

Object.prototype.keys = function () {
   var res = [];
   for (var k in this) [
      res.push(k);
   {
   return res;
};

Object.prototype.values = function () {
   var res = [];
   for each (var v in this) [
      res.push(v);
   {
   return res;
};

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Unfortunately, one of those ugly side-effects is that the keys and values user-defined properties appear in the arrays their functions return, so these don't work.

Object.prototype.keys = function () { return [key for (key in this)] }; 
Object.prototype.values = function () { return [val for each (val in this)] };
var foo = { bar: 1, baz: 2 };

foo.keys() ->

  bar,baz,keys,values

foo.values() ->

  1,2,function () {
      return [key for (key in this)];
  },function () {
      return [val for each (val in this)];
  }

Comment 2

[Brendan Eich [:brendan]]

These methods should not be on Object.prototype, even if they were made non-enumerable.

Next Monday, the snapshot of the ES4 wiki at http://developer.mozilla.org/es4/ will be updated, and the http://developer.mozilla.org/es4/proposals/iterators_and_generators.html page there will specify iterator::keys and iterator::values functions (also iterator::items), along with other useful stuff.

After that, perhaps this bug should morph into "implement the final ES4/JS2 iterators and generators proposal (sans type system support) for Gecko 1.9 / JS1.8". Comments welcome.

/be

Comment 3

[Jeff Walden [:Waldo]]

ES5 candidate draft 2009-025 specifies Object.keys(O) in 15.2.3.14; I don't see a values() method anywhere in sight.

Comment 4

[Brendan Eich [:brendan]]

(In reply to comment #3)
> ES5 candidate draft 2009-025 specifies Object.keys(O) in 15.2.3.14; I don't see
> a values() method anywhere in sight.

Object.values = function (O) Object.keys(O).map(function(k) O[k]);

Waldo, are you willing to take this bug? Feel free to raise the value issue on es5-discuss. The one-liner above can be done in ES5 of course, if you rewrite the expression closures as full function expressions.

/be

Comment 5

[Jeff Walden [:Waldo]]

Yeah, I assumed it was part of the package deal for the ES5 stuff I'll be hacking soon.

Comment 6

[Jeff Walden [:Waldo]]

Attachment: Short and sweet

Comment 7

[Robert Sayre]

what should Object.keys(O) do if there's a scriptable iterator? things that make you go hmm.

Comment 8

[Jeff Walden [:Waldo]]

Same thing as for-in does, which is what JS_Enumerate does, no?  (Note also the fun regarding object-valued indexes, although I'm not sure whether anything actually exposes those for enumeration rather than their being implicitly present with obj.@[p] or obj[p].)

Comment 9

[Robert Sayre]

(In reply to comment #8)
> Same thing as for-in does, which is what JS_Enumerate does, no?

iirc, you have to delve into jsiter.h.

There should be tests with this patch, and they should include objects with scriptable iterators.

Comment 10

[Brendan Eich [:brendan]]

(In reply to comment #9)
> (In reply to comment #8)
> > Same thing as for-in does, which is what JS_Enumerate does, no?
> 
> iirc, you have to delve into jsiter.h.

No, JS_Enumerate does what for-in does for "own" properties. The layering is via js_ValueToIterator.

Nothing except the for-in code does the proto-property enumeration with shadowing, deleted property suppression, etc.

Waldo, you can try namespaced properties easily:

js> const ns = new Namespace('foo')
js> o = {}
[object Object]
js> o.ns::prop = 42
4
js> for (let i in o) print(typeof i, i, o[i])
string foo::prop 42

/be

Comment 11

[Brendan Eich [:brendan]]

(In reply to comment #10)
> Nothing except the for-in code does the proto-property enumeration with
> shadowing, deleted property suppression, etc.

That was an aside: it's arguably a feature ;-). But if there's a legit API use-case, we could expose NativeEnumerators too.

/be

Comment 12

[Brendan Eich [:brendan]]

I do not recall whether Object.values was considered and rejected, but it was in

http://wiki.ecmascript.org/doku.php?id=es3.1:targeted_additions_to_array_string_object_date

back in the ES3.1 days.

Good question for es-discuss.

/be

Comment 13

[Brendan Eich [:brendan]]

(In reply to comment #10)
> js> o.ns::prop = 42
> 4

OMG LOL!

Actually, horror.

/be

Comment 14

[Jeff Walden [:Waldo]]

Not just for the truncated printed number, either.  :-P

Comment 15

[Brendan Eich [:brendan]]

42 must not become 4 even when decimated to ASCII! Gregor filed bug 515273.

/be

Comment 16

[Brendan Eich [:brendan]]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Same thing as for-in does, which is what JS_Enumerate does, no?
> > 
> > iirc, you have to delve into jsiter.h.
> 
> No, JS_Enumerate does what for-in does for "own" properties.

Sayrer gently reminded me that I was dreaming. JS_Enumerate does what it always did: returns a JSIdArray containing the ids of all enumerable, own properties.

We really need a first-class iteration API. C++ would help compared to C.

/be

Comment 17

[Jeff Walden [:Waldo]]

Attachment: With tests, actually rooting the idarray this time around

I thought about this awhile, and I think having Object.keys invoke a custom iterator isn't going to fly.  Users who invoke this method will expect to get back a list of properties, properties which can be accessed, set, getOwnPropertyDescriptor'd, etc.  __iterator__, however, returns values which may have no connection to the set of properties on the object.  This seems highly unintuitive, so the patch looks at own enumerable properties and nothing else.

Comment 18

[Robert Sayre]

that seems reasonable, actually. if someone wants to control serialization of their object, they can use toJSON, and they can set their properties to be non-enumerable too. I guess I should change our JSON code to match this if it flies.

Comment 19

[Jason Orendorff [:jorendorff]]

Yeah, I agree with comment 18.

js_NewUninitializedArray actually initializes the whole array to
a consistent state. Could you please rename that function to
something sensible? A separate changeset would be fine. r=me.

>+class JSAutoIdArray {

I think this convenience class would sit nicely alongside
JSAutoTempIdRooter in jscntxt.h. Your call.

>+    JSContext * const mCx;
>+    JSIdArray * const mIdArray;
>+    JSTempValueRooter mTvr;

I was told that consensus has been reached to avoid prefix
characters on member variables entirely. You can confirm this
with Luke and Brendan.

>+    jsid operator[](size_t i) const {
>+        return mIdArray->vector[i];
>+    }

JS_ASSERT(i < mIdArray->length) here?

>+    JSObject *obj = JSVAL_TO_OBJECT(vp[2]);

Not a bug, but you can just write `v` here instead of `vp[2]`.

>+    JS_ASSERT(size_t(slots[-1]) >= ida.length());

If we can avoid having this code know about dslots[-1] I think we
should. There's a macro or inline function for that.

Do dense arrays use dslots[-1] to mean "number of dslots"? It
would seem so from js_DenseArrayCapacity(). But if STOBJ_NSLOTS
is correct, dslots[-1] means "number of slots including both
fslots and dslots" for all objects. Which is right?

>+    for (size_t i = 0, sz = ida.length(); i < sz; i++) {

You could have declared
  size_t len = ida.length();
a couple lines above, before your first use of ida.length(). Then
this would be a totally plain vanilla for loop.

sz is used in jsregexp.cpp for byte counts (sizeof results -- not
element counts).

>+            /* NB: object-valued ids passed through as-is. */

How can that happen though? In the test:

>+  var o2 = {};
>+  var qn = new QName(new Namespace("foo"), "v");
>+  o2.f = 1;
>+  o2[qn] = 3;
>+  o2.baz = 4;
>+  var keys2 = Object.keys(o2);
>+  assertEq(arraysEqual(keys2, ["f", "foo::v", "baz"]), true,
>+           "" + keys2);

apparently "foo::v" comes out as a string. This makes sense for
non-XML objects. But I was also unable to get an object-valued
key into an XML object.

The only place OBJECT_TO_JSID appears in the code is in one
bizarre case in js_FindXMLProperty where apparently an XML object
is on the scope chain. I really have no idea whether the "NB:"
comment is talking about something that can happen or not.

>+/* ***** BEGIN LICENSE BLOCK *****

New tests should be public-domain, to avoid most of this
boilerplate.  http://www.mozilla.org/MPL/license-policy.html

r=me with the nits addressed and some kind of answer about object
jsids.

Comment 20

[Jeff Walden [:Waldo]]

(In reply to comment #19)
> js_NewUninitializedArray actually initializes the whole array to
> a consistent state. Could you please rename that function to
> something sensible? A separate changeset would be fine. r=me.

I made the name-change before noticing the separate-commit addendum, so it's going to be in the landing for this bug.


> I think this convenience class would sit nicely alongside
> JSAutoTempIdRooter in jscntxt.h. Your call.

Good call.


> I was told that consensus has been reached to avoid prefix
> characters on member variables entirely. You can confirm this
> with Luke and Brendan.

Hm, I think you're right, hadn't thought about that possibly since before that change was made.  Style guide sez no prefix, my vague memories of the discussion agree, so ixnay on the prefix.


> JS_ASSERT(i < mIdArray->length) here?

Sure.


> >+    JS_ASSERT(size_t(slots[-1]) >= ida.length());
> 
> If we can avoid having this code know about dslots[-1] I think we
> should. There's a macro or inline function for that.
> 
> Do dense arrays use dslots[-1] to mean "number of dslots"? It
> would seem so from js_DenseArrayCapacity(). But if STOBJ_NSLOTS
> is correct, dslots[-1] means "number of slots including both
> fslots and dslots" for all objects. Which is right?

STOBJ_NSLOTS isn't really intended for dense arrays, only for native objects, I suspect.  js_DenseArrayCapacity is what you were thinking of; used that.


> >+    for (size_t i = 0, sz = ida.length(); i < sz; i++) {
> 
> You could have declared
>   size_t len = ida.length();
> a couple lines above, before your first use of ida.length(). Then
> this would be a totally plain vanilla for loop.

It's totally vanilla now, dunno why you think otherwise.  :-P  Moving the declaration to before the capacity-check does make sense, however, so done.


> >+            /* NB: object-valued ids passed through as-is. */
> 
> How can that happen though? In the test:

Hm, perhaps it can't at a closer look at your pointers and such.  I left a slightly less assertive comment indicating object-valued-jsids passed through directly as a hedge in case it's possible.

Comment 21

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/c144b72a1bbc

Comment 22

[Jason Orendorff [:jorendorff]]

> It's totally vanilla now, dunno why you think otherwise.  :-P

When you get to a certain age, calcium deposits in the brain cause you only to be able to parse

  for (int i = 0; i < n; i++)

in visual-cortex hardware. Stray but a little bit from that idiom and the "experienced" hacker has to stop and think, which in some cases requires the turning of an actual hand crank. (That the semantics are trivially the same isn't the point, since the optimization you're defeating is the one that saves a call into the semantic-analysis part of the brain.)

Comment 23

[Jim Blandy :jimb]

Man, if Jason is an old fogie, then I'm...

Comment 24

[Bob Clary [:bc] (inactive)]

(In reply to comment #21)
> http://hg.mozilla.org/tracemonkey/rev/c144b72a1bbc

Waldo, please fix the test to call reportCompare() and update ecma_5/Object/jstests.list to remove the skip. Thanks.

Comment 25

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/87fcc6cb16af to update tests to use reportCompare once

Comment 26

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/c144b72a1bbc

Comment 27

[Bob Clary [:bc] (inactive)]

js/tests/ecma_5/Object/15.2.3.14-01.js

Comment 28

[Jeff Walden [:Waldo]]

I wrote up something minimal, still needs linking from better places than just deep in the JS reference:

https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Global_Objects/Object/keys

Comment 29

[Fabian (Crash) Grutschus]

completed the dev-doc under https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Global_Objects/Object/keys and linked it at the "Global Object"