ES6: Adding support for size to Set and Map

ES6: Adding support for size to Set and Map

Section 15.14.5.10 and 15.16.5.7 in the October 26, 2012 ES6 draft,
http://wiki.ecmascript.org/doku.php?id=harmony:specification_drafts

This adds a getter for "size" to Set.prototype and Map.prototype which reflects
the number of elements in the Set and Map respectively.

BUG=v8:2395
Committed: https://code.google.com/p/v8/source/detail?r=12875

[arv (Not doing code reviews), 2012-11-05 21:17:54]

-Sven, +Michael

I intended mstarzinger to review this since he implemented Set and Map
initially.

[Michael Starzinger, 2012-11-06 09:32:40]

This is already looking good. Just a few comments.

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js
File src/collection.js (right):

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js#n...
src/collection.js:91: function SetSize() {
I think "SetGetSize" or "SetSizeGetter" would be a better naming convention for
getters.

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js#n...
src/collection.js:157: function MapSize() {
Likewise for "MapGetSize" or "MapSizeGetter".

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js#n...
src/collection.js:235: function DefineGetter(object, name, fun) {
Can we move this into v8natives.js near the implementation of InstallFunctions,
so that it can become the bottle-neck for all builtin getters that we install?

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
File test/mjsunit/harmony/collections.js (right):

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
test/mjsunit/harmony/collections.js:319: // Set and Map Set size
Comment seems to contain a typo.

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
test/mjsunit/harmony/collections.js:324:
assertFalse(setSizeDescriptor.enumerable);
I couldn't find the expected value of the attributes (i.e. enumerable and
configurable) in the draft spec. Is that still unspeced?

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
test/mjsunit/harmony/collections.js:343:
assertFalse(mapSizeDescriptor.enumerable);
Likewise.

[arv (Not doing code reviews), 2012-11-06 16:02:33]

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js
File src/collection.js (right):

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js#n...
src/collection.js:91: function SetSize() {
On 2012/11/06 09:32:40, Michael Starzinger wrote:
> I think "SetGetSize" or "SetSizeGetter" would be a better naming convention
for
> getters.

Done.

https://chromiumcodereview.appspot.com/11360089/diff/2001/src/collection.js#n...
src/collection.js:235: function DefineGetter(object, name, fun) {
On 2012/11/06 09:32:40, Michael Starzinger wrote:
> Can we move this into v8natives.js near the implementation of
InstallFunctions,
> so that it can become the bottle-neck for all builtin getters that we install?

Done.

I also used a lower level abstraction.

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
File test/mjsunit/harmony/collections.js (right):

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
test/mjsunit/harmony/collections.js:319: // Set and Map Set size
On 2012/11/06 09:32:40, Michael Starzinger wrote:
> Comment seems to contain a typo.

Done.

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
test/mjsunit/harmony/collections.js:324:
assertFalse(setSizeDescriptor.enumerable);
On 2012/11/06 09:32:40, Michael Starzinger wrote:
> I couldn't find the expected value of the attributes (i.e. enumerable and
> configurable) in the draft spec. Is that still unspeced?

Last sentence of last paragraph in the introduction to clause 15:

Every other property described in this clause has the attributes { [[Writable]]:
true, [[Enumerable]]: false, [[Configurable]]: true } unless otherwise
specified.

https://chromiumcodereview.appspot.com/11360089/diff/6/src/v8natives.js
File src/v8natives.js (right):

https://chromiumcodereview.appspot.com/11360089/diff/6/src/v8natives.js#newco...
src/v8natives.js:67: %FunctionRemovePrototype(getter);
I missed this one last time. The spec says that all spec functions unless,
otherwise stated, should NOT have a prototype property.

[Michael Starzinger, 2012-11-06 17:47:55]

LGTM. I'll land this for you.

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
File test/mjsunit/harmony/collections.js (right):

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
test/mjsunit/harmony/collections.js:324:
assertFalse(setSizeDescriptor.enumerable);
On 2012/11/06 16:02:33, arv wrote:
> On 2012/11/06 09:32:40, Michael Starzinger wrote:
> > I couldn't find the expected value of the attributes (i.e. enumerable and
> > configurable) in the draft spec. Is that still unspeced?
> 
> Last sentence of last paragraph in the introduction to clause 15:
> 
> Every other property described in this clause has the attributes {
[[Writable]]:
> true, [[Enumerable]]: false, [[Configurable]]: true } unless otherwise
> specified.

Ah, yes, I didn't see this sentence. To be pedantic (as Andreas pointed out),
this sentence is talking about data properties (as it contains the writable
attribute) and it's unclear whether is also applies to accessor properties. But
I am fine to land this CL as it is even without a clarification on this part.

[arv (Not doing code reviews), 2012-11-06 18:49:43]

Thanks.

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
File test/mjsunit/harmony/collections.js (right):

https://chromiumcodereview.appspot.com/11360089/diff/2001/test/mjsunit/harmon...
test/mjsunit/harmony/collections.js:324:
assertFalse(setSizeDescriptor.enumerable);
On 2012/11/06 17:47:55, Michael Starzinger wrote:
> On 2012/11/06 16:02:33, arv wrote:
> > On 2012/11/06 09:32:40, Michael Starzinger wrote:
> > > I couldn't find the expected value of the attributes (i.e. enumerable and
> > > configurable) in the draft spec. Is that still unspeced?
> > 
> > Last sentence of last paragraph in the introduction to clause 15:
> > 
> > Every other property described in this clause has the attributes {
> [[Writable]]:
> > true, [[Enumerable]]: false, [[Configurable]]: true } unless otherwise
> > specified.
> 
> Ah, yes, I didn't see this sentence. To be pedantic (as Andreas pointed out),
> this sentence is talking about data properties (as it contains the writable
> attribute) and it's unclear whether is also applies to accessor properties.
But
> I am fine to land this CL as it is even without a clarification on this part.

I had a hard time finding this too. I even filed an ES6 bug before Allen pointed
out where to find this.

https://bugs.ecmascript.org/show_bug.cgi?id=931