Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: evilpie, Assigned: arai)

Tracking

({dev-doc-complete})

Trunk
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

(Whiteboard: [DocArea=JS], )

Attachments

(3 attachments)

Reporter

Description

4 years ago
This is quite easy to implement, and would be useful for a correct implementation of SpeciesConstructor.
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
Assignee

Comment 1

4 years ago
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
Assignee: nobody → arai.unmht
Attachment #8604316 - Flags: review?(evilpies)
Reporter

Updated

4 years ago
Attachment #8604316 - Flags: review?(evilpies) → review+
Reporter

Comment 4

4 years ago
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.
Attachment #8604317 - Flags: review?(evilpies) → review+
Reporter

Comment 5

4 years ago
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.
Attachment #8604318 - Flags: review?(evilpies) → review+
Reporter

Comment 6

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

Comment 7

4 years ago
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.
Flags: needinfo?(evilpies)
Reporter

Comment 8

4 years ago
Sounds perfect. We should not enable @@species for one constructor, unless all the methods can support it.
Flags: needinfo?(evilpies)
Assignee

Comment 9

4 years ago
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)
Assignee

Updated

4 years ago
Blocks: 1165052
Assignee

Updated

4 years ago
Blocks: 1165053
Assignee

Updated

4 years ago
Blocks: 1165075
Thanks a lot, arai! Added some bits of content. More once we have Array sub-classing, etc.
You need to log in before you can comment on or make changes to this bug.