1131043 - Implement Symbol.species

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Tom S [:evilpie]]

This is quite easy to implement, and would be useful for a correct implementation of SpeciesConstructor.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Part 1: Add Symbol.species.

Just implemented Symbol.species and @@species accessors for built-ins (except Promise).
@@species is used in following places, I think it would be better to fix them in separated bug.
It might be better to delay each @@species accessors until those are fixed, to make the feature testable.

  RegExp
    RegExp.prototype[@@split]       (bug 887016: in my queue)
  TypedArray%
    %TypedArray% ctor
    %TypedArray%.prototype.set
    %TypedArray%.prototype.filter   (done)
    %TypedArray%.prototype.map      (done)
    %TypedArray%.prototype.slice    (done)
    %TypedArray%.prototype.subarray (done)
  ArrayBuffer
    ArrayBuffer.prototype.slice
  Array (ArraySpeciesCreate)
    Array.prototype.concat
    Array.prototype.filter
    Array.prototype.map
    Array.prototype.slice
    Array.prototype.splice
  Promise
    get Promise[@@species]
    Promise.all
    Promise.race
    Promise.reject
    Promise.resolve
    Promise.prototype.then

Almost green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b61aa6afbdf

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Part 2: Implement @@species getter for builtin types.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Part 3: Add SpeciesConstructor tests for TypedArray.prototype.{filter,map,slice,subarray}.

Comment 4

[Tom S [:evilpie]]

Comment on attachment 8604317 [details] [diff] [review]
Part 2: Implement @@species getter for builtin types.

Review of attachment 8604317 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/tests/ecma_6/Symbol/species.js
@@ +8,5 @@
> +               Int16Array, Uint16Array, Int32Array, Uint32Array,
> +               Float32Array, Float64Array,
> +               Map, Set,
> +               ArrayBuffer]) {
> +  assertEq(C[Symbol.species], C);

You should assert something like:

var desc = Object.getOwnPropertyDescriptor(C, Symbol.species);
assertDeepEq(desc, {get: C, set: undefined, enumerable: false, configurable: true}) // might be slightly different
assertEq(desc.get.apply(null), null);
assertEq(desc.get.apply(undefined), undefined);
assertEq(desc.get.apply(42), 42);

@@ +11,5 @@
> +               ArrayBuffer]) {
> +  assertEq(C[Symbol.species], C);
> +}
> +
> +var SpeciesGetter = Object.getOwnPropertyDescriptor(RegExp, Symbol.species).get;

Don't need this.

::: js/src/vm/GlobalObject.cpp
@@ +392,5 @@
>      }
>  
> +    RootedValue symbol_species(cx);
> +    symbol_species.setSymbol(cx->wellKnownSymbols().get(JS::SymbolCode::species));
> +    if (!JS_DefineProperty(cx, global, "symbol_species", symbol_species,

Let's use std_species for consistency with std_iterator.

::: js/src/vm/TypedArrayObject.cpp
@@ +823,5 @@
>      JS_FS_END
>  };
>  
> +/* static */ const JSPropertySpec
> +TypedArrayObject::staticAccessors[] = {

I think staticProperties would be more common.

Comment 5

[Tom S [:evilpie]]

Comment on attachment 8604318 [details] [diff] [review]
Part 3: Add SpeciesConstructor tests for TypedArray.prototype.{filter,map,slice,subarray}.

Review of attachment 8604318 [details] [diff] [review]:
-----------------------------------------------------------------

I am not sure we need so many permutations, but you already did this anyway.

Comment 6

[Tom S [:evilpie]]

Oh, and would you mind filing bugs for the missing changes that you described in comment 1?

Comment 7

[Tooru Fujisawa [:arai]]

Thank you for reviewing!

How do you think about the feature test? So, something like following:
  if (Symbol.species in Array) {
    // ArraySpeciesCreate should be supported in Array.prototype methods. let's use it.
    ...
  } else {
    // not supported. let's polyfill.
    ...
  }
If this kind of code will be common, I'm going to land only Part 1 for now, and add @@species accessor to each constructor in each bug (might be able to add RegExp[@@species] here, since RegExp.prototype[@@split] can be tested by itself anyway). If not, I'll land all of them here at once.
(actually I'm not sure if people want to polyfill it...)

I'll file bugs after deciding above.

Comment 8

[Tom S [:evilpie]]

Sounds perfect. We should not enable @@species for one constructor, unless all the methods can support it.

Comment 9

[Tooru Fujisawa [:arai]]

Okay, so now I think we can land Map[@@species] and Set[@@species] here. since there's no direct dependencies in the spec. that will make other bugs simpler (since we no more need to worry about which bug lands the changes in jsapi.h and tests/ecma_6/Symbol/species.js)

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/10e3e580af23
https://hg.mozilla.org/integration/mozilla-inbound/rev/139f99a75ab8

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3538f1133510

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/10e3e580af23
https://hg.mozilla.org/mozilla-central/rev/139f99a75ab8
https://hg.mozilla.org/mozilla-central/rev/3538f1133510

Comment 13

[Tooru Fujisawa [:arai]]

Updated following documents, but still needs some contents and examples:
  https://developer.mozilla.org/en-US/Firefox/Releases/41
  https://developer.mozilla.org/en-US/docs/Web/JavaScript/New_in_JavaScript/ECMAScript_6_support_in_Mozilla
  https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map
  https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/@@species
  https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set
  https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/@@species
  https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol
  https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/species

Comment 14

[Florian Scholz (Open Web Docs)]

Thanks a lot, arai! Added some bits of content. More once we have Array sub-classing, etc.