If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Implement %TypedArray%[@@species] and ArrayBuffer[@@species].

RESOLVED FIXED in Firefox 48

Status

()

Core
JavaScript Engine
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: arai, Assigned: arai)

Tracking

(Blocks: 1 bug)

Trunk
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 affected, firefox48 fixed)

Details

(URL)

Attachments

(12 attachments, 5 obsolete attachments)

5.44 KB, patch
lth
: review+
Details | Diff | Splinter Review
2.69 KB, patch
lth
: review+
Details | Diff | Splinter Review
2.46 KB, patch
lth
: review+
Details | Diff | Splinter Review
3.57 KB, patch
lth
: review+
Details | Diff | Splinter Review
2.70 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
9.47 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
6.39 KB, patch
Waldo
: review+
Details | Diff | Splinter Review
7.84 KB, patch
arai
: review+
Details | Diff | Splinter Review
9.27 KB, patch
arai
: review+
Details | Diff | Splinter Review
11.12 KB, patch
arai
: review+
Details | Diff | Splinter Review
6.62 KB, patch
lth
: review+
Details | Diff | Splinter Review
18.92 KB, patch
lth
: review+
Details | Diff | Splinter Review
(Assignee)

Description

2 years ago
derived from bug 1131043.

%TypedArray%[@@species] is used by CloneArrayBuffer, which is used in %TypedArray% constructor and %TypedArray%.prototype.set. we should add them at the same time, for feature test.

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-clonearraybuffer
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-%typedarray%-typedarray
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-%typedarray%.prototype.set-typedarray-offset

%TypedArray% constructor uses SpeciesConstructor also.

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-speciesconstructor

Following methods already call SpeciesConstructor, so what need there is enabling @@species in SpeciesConstructor.
  %TypedArray%.prototype.filter
  %TypedArray%.prototype.map
  %TypedArray%.prototype.slice
  %TypedArray%.prototype.subarray
(Assignee)

Comment 1

2 years ago
Oops, that was wrong. CloneArrayBuffer uses ArrayBuffer[@@species].
I guess we should implement ArrayBuffer[@@species] and SpeciesConstructor in ArrayBufer.prototype.slice here too.

http://people.mozilla.org/~jorendorff/es6-draft.html#sec-get-arraybuffer-@@species
http://people.mozilla.org/~jorendorff/es6-draft.html#sec-arraybuffer.prototype.slice
Summary: Implement %TypedArray%[@@species] and CloneArrayBuffer in %TypedArray% constructor and %TypedArray%.prototype.set. → Implement %TypedArray%[@@species] and ArrayBuffer[@@species].
(Assignee)

Comment 2

2 years ago
Correction again. %TypedArray%.prototype.set doesn't use @@species, because it passes %ArrayBuffer% to cloneConstructor argument of CloneArrayBuffer.

> 22.2.3.22.2 %TypedArray%.prototype.set(typedArray [, offset ] )
> 24. If SameValue(srcBuffer, targetBuffer) is true, then
>     a. Let srcBuffer be CloneArrayBuffer(targetBuffer, srcByteOffset, %ArrayBuffer%).

> 24.1.1.4 CloneArrayBuffer( srcBuffer, srcByteOffset [, cloneConstructor] )
> 2. If cloneConstructor is not present, then
>     a. Let cloneConstructor be SpeciesConstructor(srcBuffer, %ArrayBuffer%).

So, only %TypedArray% constructor and ArrayBuffer.prototype.slice use it.

Updated

2 years ago
See Also: → bug 1233040

Updated

2 years ago
Blocks: 1233040
See Also: bug 1233040
(Assignee)

Comment 3

2 years ago
Currently ArrayBuffer.prototype.slice is implemented as native function, and SpeciesConstructor is implemented as Self-hosted JS (as other TypedArray methods are also self-hosted).
To avoid having 2 copies of SpeciesConstructor, we might have to call self-hosted SpeciesConstructor from native ArrayBuffer.prototype.slice, or self-host ArrayBuffer.prototype.slice.
(Assignee)

Updated

2 years ago
Depends on: 1234038
(Assignee)

Comment 4

2 years ago
Created attachment 8708180 [details] [diff] [review]
Part 1: Handle when ArrayBuffer species constructor returns wrapped ArrayBuffer.

Prepared 9 patches.

[Part 1]

In current ArrayBuffer#splice, object returned by species constructor (actually the default constructor) is always from same compartment.  After we support @@species, it could be another global's ArrayBuffer, and we should not throw error for `!IsArrayBuffer(new_)`.
Added IsWrappedArrayBuffer intrinsic to check wrapped ArrayBuffer.

Then, in that case, we should perform IsDetachedBuffer and ArrayBufferByteLength in target compartment.  Added IsDetachedBufferThis and ArrayBufferByteLengthThis to perform IsDetachedBuffer and ArrayBufferByteLength on unwrapped ArrayBuffer.

Also, added "isWrapped" parameter to ArrayBufferCopyData to unwrap it.
Assignee: nobody → arai.unmht
Attachment #8708180 - Flags: review?(lhansen)
(Assignee)

Comment 5

2 years ago
Created attachment 8708181 [details] [diff] [review]
Part 2: Implement %TypedArray%[@@species] getter and ArrayBuffer[@@species] getter. r=evilpie

Part 2 and Part 3 are already r+ed in bug 1131043.  Just delayed to align with handling ArrayBuffer[@@species] in TypedArray ctor.
Attachment #8708181 - Flags: review+
(Assignee)

Comment 6

2 years ago
Created attachment 8708182 [details] [diff] [review]
Part 3: Add SpeciesConstructor tests for TypedArray.prototype.{filter,map,slice,subarray}. r=evilpie
Attachment #8708182 - Flags: review+
(Assignee)

Comment 7

2 years ago
Created attachment 8708183 [details] [diff] [review]
Part 4: Add SpeciesConstructor tests for ArrayBuffer.prototype.slice.
Attachment #8708183 - Flags: review?(lhansen)
(Assignee)

Comment 8

2 years ago
Created attachment 8708184 [details] [diff] [review]
Part 5: Add native SpeciesConstructor wrapper.

Now TypedArray ctors are native function, and SpeciesConstructor is already implemented in self-hosted JS, to avoid having duplicated impl, native SpeciesConstructor is just a wrapper to call self-hosted function.

we should be avoid calling this function on normal case (handled in Part 9).
Attachment #8708184 - Flags: review?(lhansen)
(Assignee)

Comment 9

2 years ago
Created attachment 8708185 [details] [diff] [review]
Part 6: Refactor TypedArrayObjectTemplate::fromArray.

To apply SpeciesConstructor change to wrapped TypedArray case, factored out typedArray case and object case from TypedArrayObjectTemplate::fromArray.

https://tc39.github.io/ecma262/#sec-typedarray-typedarray
https://tc39.github.io/ecma262/#sec-typedarray-object

object case is not spec-conformant, as it does not handle iterator.
it should be fixed in separated bug.

Some steps in typedArray case are skipped in this part, they're added in Part 7.
Attachment #8708185 - Flags: review?(lhansen)
(Assignee)

Comment 10

2 years ago
Created attachment 8708186 [details] [diff] [review]
Part 7: Call SpeciesConstructor in TypedArray ctors.

SpeciesConstructor is called in TypedArray ctor in 2 places, in CloneArrayBuffer and TypedArray ctor itself.

  https://tc39.github.io/ecma262/#sec-clonearraybuffer
  https://tc39.github.io/ecma262/#sec-typedarray-typedarray

Currently copy is done in TypedArrayMethods::setFromArrayLike, so added CloneArrayBufferNoCopy instead of CloneArrayBuffer, that does not copy data.
Attachment #8708186 - Flags: review?(lhansen)
(Assignee)

Comment 11

2 years ago
Created attachment 8708187 [details] [diff] [review]
Part 8: Add GetGetterPure.

GetGetterPure does almost same thing as GetPropertyPure, but returns JSFunction for getter property.
It's used by Part 9 to check if the getter value is original one.
Attachment #8708187 - Flags: review?(lhansen)
(Assignee)

Comment 12

2 years ago
Created attachment 8708188 [details] [diff] [review]
Part 9: Add IsArrayBufferSpecies and avoid calling SpeciesConstructor on normal case.

Added IsArrayBufferSpecies that checks if given object's @@species is current compartment's ArrayBuffer, without any visible operation,
and avoid calling SpeciesConstructor when it's true.
Attachment #8708188 - Flags: review?(lhansen)
Comment on attachment 8708180 [details] [diff] [review]
Part 1: Handle when ArrayBuffer species constructor returns wrapped ArrayBuffer.

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

I was just about to file a time-bomb bug for this, but I found this bug number when searching for a @@species bug to set a dependency on.  Might as well steal it, having looked into it that far.

The proliferation of compartment-handling APIs to type-test things (and access type-tested things) seems undesirable, but I don't have anything better.

::: js/src/builtin/TypedArray.js
@@ +1449,5 @@
> +    var actualLen;
> +    if (!isWrapped)
> +        actualLen = ArrayBufferByteLength(new_);
> +    else
> +        actualLen = callFunction(CallArrayBufferMethodIfWrapped, new_, "ArrayBufferByteLengthThis");

Rather than this, it might be nicer to do it similar to how TypedArray lengths are done, with intrinsic_PossiblyWrappedTypedArrayLength (and probably with inlining of it, which hasn't landed yet).  But this does work.  Up to you.
Attachment #8708180 - Flags: review?(lhansen) → review+
(Assignee)

Comment 14

2 years ago
Thank you for reviewing :)
Will post patches for adding PossiblyWrappedArrayBufferByteLength, and inlining PossiblyWrappedArrayBufferByteLength and ArrayBufferByteLength.

also, will rebase and update remaining parts.
(Assignee)

Comment 15

2 years ago
Looks like my WIP didn't handle SharedArrayBuffer correctly, and there should be some more space to refactor.
give me some time to fix them :)
(Assignee)

Comment 16

2 years ago
Created attachment 8716710 [details] [diff] [review]
Part 0.1: Add PossiblyWrappedArrayBufferByteLength self-hosting intrinsic.
Attachment #8716710 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 17

2 years ago
Created attachment 8716711 [details] [diff] [review]
Part 0.2: Inline PossiblyWrappedArrayBufferByteLength self-hosting intrinsic.

as a first step, implemented inlining with MLoadFixedSlot.
TypedArray's length has another optimized path that emits MConstant, but I don't yet understand how that works.
I'd like to leave it to another bug.
Attachment #8716711 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 18

2 years ago
Created attachment 8716712 [details] [diff] [review]
Part 0.3: Inline ArrayBufferByteLength self-hosting intrinsic.

Applied same optimization to ArrayBufferByteLength, that is called at ArrayBuffer#slice step 5, with non-wrapped buffer.
Attachment #8716712 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 19

2 years ago
Created attachment 8716713 [details] [diff] [review]
Part 1: Handle when ArrayBuffer species constructor returns wrapped ArrayBuffer. r=Waldo

Changed to use PossiblyWrappedTypedArrayLength
Attachment #8716713 - Flags: review+
(Assignee)

Comment 20

2 years ago
Created attachment 8716714 [details] [diff] [review]
Part 2: Implement %TypedArray%[@@species] getter and ArrayBuffer[@@species] getter. r=evilpie
Attachment #8708180 - Attachment is obsolete: true
Attachment #8708181 - Attachment is obsolete: true
Attachment #8716714 - Flags: review+
(Assignee)

Comment 21

2 years ago
Created attachment 8716715 [details] [diff] [review]
Part 3: Add SpeciesConstructor tests for TypedArray.prototype.{filter,map,slice,subarray}. r=evilpie
Attachment #8708182 - Attachment is obsolete: true
Attachment #8716715 - Flags: review+
(Assignee)

Comment 22

2 years ago
Created attachment 8716716 [details] [diff] [review]
Part 6: Refactor TypedArrayObjectTemplate::fromArray.

TypedArrayMethods::setFromArrayLike is called from TypedArrayObjectTemplate::fromTypedArray and TypedArrayObjectTemplate::fromObject. so we can just call TypedArrayMethods::setFromTypedArray and TypedArrayMethods::setFromNonTypedArray directly.
removed TypedArrayMethods::setFromArrayLike, and made both TypedArrayMethods::setFromTypedArray and TypedArrayMethods::setFromNonTypedArray public.
Attachment #8708185 - Attachment is obsolete: true
Attachment #8708185 - Flags: review?(lhansen)
Attachment #8716716 - Flags: review?(lhansen)
(Assignee)

Comment 23

2 years ago
Created attachment 8716717 [details] [diff] [review]
Part 7: Call SpeciesConstructor in TypedArray ctors.

To handle SharedArrayBuffer, added ArrayBufferObjectMaybeShared::isDetached method to call ArrayBufferObject::isDetached.
Also, changed AllocateArrayBuffer to receive elementLength instead of byteLength, as maybeCreateArrayBuffer also receives elementLength.
Attachment #8708186 - Attachment is obsolete: true
Attachment #8708186 - Flags: review?(lhansen)
Attachment #8716717 - Flags: review?(lhansen)
Comment on attachment 8708184 [details] [diff] [review]
Part 5: Add native SpeciesConstructor wrapper.

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

::: js/src/jsobj.cpp
@@ +3932,5 @@
> +bool
> +js::SpeciesConstructor(JSContext* cx, HandleObject obj, HandleValue defaultCtor, MutableHandleValue pctor)
> +{
> +    HandlePropertyName shName = cx->names().SpeciesConstructor;
> +    RootedAtom name(cx, cx->names().SpeciesConstructor);

Could just reference "shName" here?
Attachment #8708184 - Flags: review?(lhansen) → review+

Updated

2 years ago
Attachment #8708183 - Flags: review?(lhansen) → review+
Comment on attachment 8716716 [details] [diff] [review]
Part 6: Refactor TypedArrayObjectTemplate::fromArray.

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

Looks OK once the FIXMEs are taken care of.

::: js/src/vm/TypedArrayObject.cpp
@@ +763,1 @@
>      Rooted<ArrayBufferObject*> buffer(cx);

FIXME?  Requires a bug# at a minimum.

@@ +778,5 @@
> +
> +// FIXME: This is not compatible with TypedArrayFrom in the spec
> +// (ES 2016 draft Jan 21, 2016 22.2.4.4 and 22.2.2.1.1)
> +// We should handle iterator protocol.
> +template<typename T>

FIXME?  Requires a bug# at a minimum.
Attachment #8716716 - Flags: review?(lhansen) → review+
Comment on attachment 8708187 [details] [diff] [review]
Part 8: Add GetGetterPure.

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

::: js/src/jsobj.cpp
@@ +2377,5 @@
> +static inline bool
> +NativeGetGetterPureInline(NativeObject* pobj, Shape* shape, JSFunction** fp)
> +{
> +    if (shape->hasGetterObject()) {
> +        if (shape->getterObject()->is<JSFunction>()) {

Judging from the next patch you don't care if it's callable but not a JSFunction?
Attachment #8708187 - Flags: review?(lhansen) → review+

Updated

2 years ago
Attachment #8708188 - Flags: review?(lhansen) → review+
Comment on attachment 8716717 [details] [diff] [review]
Part 7: Call SpeciesConstructor in TypedArray ctors.

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

Seems OK to me.  I don't have any deep knowledge about wrapping matters so if there's something you're not sure about perhaps ask Waldo.

::: js/src/vm/ArrayBufferObject-inl.h
@@ +29,5 @@
>  
> +inline bool
> +ArrayBufferObjectMaybeShared::isDetached() const
> +{
> +    const ArrayBufferObjectMaybeShared* buf = this;

Why the alias?

::: js/src/vm/TypedArrayObject.cpp
@@ +736,5 @@
> +                                                 uint32_t elementLength,
> +                                                 MutableHandle<ArrayBufferObject*> buffer)
> +{
> +    // Step 1.
> +    // 2016 draft Jan 21, 2016 9.1.14 step 2.

Nit: "Steps 1-2", and remove the previous line, although it also says "Steps 1-6" below.  Probably clean up a little.

@@ +837,5 @@
>  {
>      // Allow nullptr newTarget for FriendAPI methods, which don't care about
>      // subclassing.
>      if (other->is<TypedArrayObject>())
> +        return fromTypedArray(cx, other, false, newTarget);

Nit: /*wrapped=*/false.

(The wasm people are using a neat idiom for this, first "typedef bool IsWrapped;" and then IsWrapped(false) and IsWrapped(true) as appropriate.)

@@ +840,5 @@
>      if (other->is<TypedArrayObject>())
> +        return fromTypedArray(cx, other, false, newTarget);
> +
> +    if (other->is<WrapperObject>() && UncheckedUnwrap(other)->is<TypedArrayObject>())
> +        return fromTypedArray(cx, other, true, newTarget);

Ditto.
Attachment #8716717 - Flags: review?(lhansen) → review+
(Assignee)

Comment 28

2 years ago
(In reply to Lars T Hansen [:lth] from comment #26)
> Comment on attachment 8708187 [details] [diff] [review]
> Part 8: Add GetGetterPure.
> 
> Review of attachment 8708187 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jsobj.cpp
> @@ +2377,5 @@
> > +static inline bool
> > +NativeGetGetterPureInline(NativeObject* pobj, Shape* shape, JSFunction** fp)
> > +{
> > +    if (shape->hasGetterObject()) {
> > +        if (shape->getterObject()->is<JSFunction>()) {
> 
> Judging from the next patch you don't care if it's callable but not a
> JSFunction?

yeah, for now, there is no need to handle non-function object.
Comment on attachment 8716710 [details] [diff] [review]
Part 0.1: Add PossiblyWrappedArrayBufferByteLength self-hosting intrinsic.

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

::: js/src/vm/SelfHosting.cpp
@@ +773,5 @@
> +        return false;
> +    }
> +
> +    MOZ_ASSERT(obj->is<ArrayBufferObject>());
> +    uint32_t length = obj->as<ArrayBufferObject>().byteLength();

as<> will do this assert, so I'd drop it even if the assertion message is worse.  But eh, this is fine too.
Attachment #8716710 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 8716711 [details] [diff] [review]
Part 0.2: Inline PossiblyWrappedArrayBufferByteLength self-hosting intrinsic.

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

::: js/src/jit-test/tests/ion/testPossiblyWrappedArrayBufferByteLength.js
@@ +9,5 @@
> +
> +  var f = function() {
> +    assertEq(PossiblyWrappedArrayBufferByteLength(abuf), arr.length * Int32Array.BYTES_PER_ELEMENT);
> +  };
> +  for (var i = 0; i < 40; i++) {

I'd prefer an inIon()-based loop myself, but you know more about the JITs than I do, I suspect.

@@ +24,5 @@
> +var abuf = tarr.buffer;
> +`);
> +
> +  var f = function() {
> +    assertEq(PossiblyWrappedArrayBufferByteLength(g.abuf), g.arr.length * g.Int32Array.BYTES_PER_ELEMENT);

Do you want to cache g.abuf/g.arr.length/g.Int32Array.BYTES_PER_ELEMENT so this loop doesn't have cross-compartment accesses in it?  Those seem dubious/worrisome to me as far as having the loop be JITted/inlined goes, and I'd not take the risk.

::: js/src/jit/IonBuilder.cpp
@@ +9455,5 @@
> +IonBuilder::addArrayBufferByteLength(MDefinition* obj)
> +{
> +    MLoadFixedSlot* ins = MLoadFixedSlot::New(alloc(), obj, ArrayBufferObject::BYTE_LENGTH_SLOT);
> +    current->add(ins);
> +    ins->setResultType(MIRType_Double);

This doesn't want to be MIRType_Int32?  I mean, I have no idea, and existing uses seem to suggest MLoadeFixedSlot can have a non-Value result type, but I'd have expected an instruction that specifically extracts an int32_t, myself.

::: js/src/jit/MCallOptimize.cpp
@@ +2194,5 @@
> +IonBuilder::inlinePossiblyWrappedArrayBufferByteLength(CallInfo& callInfo)
> +{
> +    MOZ_ASSERT(!callInfo.constructing());
> +    MOZ_ASSERT(callInfo.argc() == 1);
> +    if (callInfo.getArg(0)->type() != MIRType_Object)

Assign |callInfo.getArg(0)| to an |obj| local to avoid massive repetition?  And a blank line after the asserts.

::: js/src/vm/SelfHosting.cpp
@@ +1817,5 @@
>            intrinsic_IsInstanceOfBuiltin<SharedArrayBufferObject>,       1,0),
>  
>      JS_FN("ArrayBufferByteLength",   intrinsic_ArrayBufferByteLength,   1,0),
> +    JS_INLINABLE_FN("PossiblyWrappedArrayBufferByteLength", intrinsic_PossiblyWrappedArrayBufferByteLength, 1,0,
> +                    IntrinsicPossiblyWrappedArrayBufferByteLength),

Mm, so much repetition.
Attachment #8716711 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 8716712 [details] [diff] [review]
Part 0.3: Inline ArrayBufferByteLength self-hosting intrinsic.

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

::: js/src/jit-test/tests/ion/testArrayBufferByteLength.js
@@ +9,5 @@
> +
> +  var f = function() {
> +    assertEq(ArrayBufferByteLength(abuf), arr.length * Int32Array.BYTES_PER_ELEMENT);
> +  };
> +  for (var i = 0; i < 40; i++) {

Same inIon thing.

I slightly wonder if we'd ever want ArrayBuffer length accesses to be inlined as constants, like we do for typed arrays.  Probably should wait on a benchmark demonstrating its utility, as ArrayBuffers seem unlikely to be used raw that way much (let alone in hot code).  Also something to be very careful about, given neutering considerations and bug 991981-style concerns.

::: js/src/jit/MCallOptimize.cpp
@@ +2196,5 @@
> +IonBuilder::inlineArrayBufferByteLength(CallInfo& callInfo)
> +{
> +    MOZ_ASSERT(!callInfo.constructing());
> +    MOZ_ASSERT(callInfo.argc() == 1);
> +    if (callInfo.getArg(0)->type() != MIRType_Object)

Same local/blank line things as in the other patch.
Attachment #8716712 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 32

2 years ago
Thank you for reviewing :D

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #30)
> ::: js/src/jit/IonBuilder.cpp
> @@ +9455,5 @@
> > +IonBuilder::addArrayBufferByteLength(MDefinition* obj)
> > +{
> > +    MLoadFixedSlot* ins = MLoadFixedSlot::New(alloc(), obj, ArrayBufferObject::BYTE_LENGTH_SLOT);
> > +    current->add(ins);
> > +    ins->setResultType(MIRType_Double);
> 
> This doesn't want to be MIRType_Int32?  I mean, I have no idea, and existing
> uses seem to suggest MLoadeFixedSlot can have a non-Value result type, but
> I'd have expected an instruction that specifically extracts an int32_t,
> myself.

This is because the BYTE_LENGTH_SLOT contains DoubleValue.

https://dxr.mozilla.org/mozilla-central/rev/6ea654cad929c9bedd8a4161a182b6189fbeae6a/js/src/vm/ArrayBufferObject.cpp#588
> size_t
> ArrayBufferObject::byteLength() const
> {
>     return size_t(getSlot(BYTE_LENGTH_SLOT).toDouble());
> }
> 
> void
> ArrayBufferObject::setByteLength(size_t length)
> {
>     setSlot(BYTE_LENGTH_SLOT, DoubleValue(length));
> }

Maybe, we could change it to Int32Value there too?  so I can use MIRType_Int32 for MLoadeFixedSlot.
(Assignee)

Updated

2 years ago
Depends on: 1248930
(In reply to Tooru Fujisawa [:arai] from comment #32)
> This is because the BYTE_LENGTH_SLOT contains DoubleValue.
> 
> Maybe, we could change it to Int32Value there too?  so I can use
> MIRType_Int32 for MLoadeFixedSlot.

Huh, no kidding.  That seems like a fine change to make, in another patch/bug, for sure.  Wonder why we did it that way; maybe someone not realizing the buffer length is limited to the non-negative int32_t range?  *shrug*
(Assignee)

Comment 34

2 years ago
I'm going to land these patches with bug 887016 (RegExp[@@species]) and bug bug 1165052 (Array[@@species]) at once.

perhaps, we might have to fix the TypedArray ctor implementation to follow https://github.com/tc39/ecma262/issues/455 when it's fixed before other bugs.
Blocks: 1256376
(Assignee)

Comment 35

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/63d6812ce8ccc5968bb0a5359a5ac9052fc87e48
Bug 1165053 - Part 0.1: Add PossiblyWrappedArrayBufferByteLength self-hosting intrinsic. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/fffa0e5cc40628fcb7848dff1fe8c8c7605e9bc7
Bug 1165053 - Part 0.2: Inline PossiblyWrappedArrayBufferByteLength self-hosting intrinsic. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/f3195319f23d49c8f91b2edfe2b549a470d386db
Bug 1165053 - Part 0.3: Inline ArrayBufferByteLength self-hosting intrinsic. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/f66243f0197ec2f2c55d8e0481234a7768d17198
Bug 1165053 - Part 1: Handle when ArrayBuffer species constructor returns wrapped ArrayBuffer. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ec3a764a8a50ae594fc35f7f17aef550efbce34
Bug 1165053 - Part 2: Implement %TypedArray%[@@species] getter and ArrayBuffer[@@species] getter. r=evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/31ce940c509c21d24e9d59653ee5f4bad10f1cff
Bug 1165053 - Part 3: Add SpeciesConstructor tests for TypedArray.prototype.{filter,map,slice,subarray}. r=evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/fdef3048ed7784fd5d8ea97ebd92bc1946ce175c
Bug 1165053 - Part 4: Add SpeciesConstructor tests for ArrayBuffer.prototype.slice. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/71ef5f8009ec19ad4bfdbf009bfc0b11ffd788d2
Bug 1165053 - Part 5: Add native SpeciesConstructor wrapper. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/27189d8e678de79cb4b8e00ec6230cd82442e831
Bug 1165053 - Part 6: Refactor TypedArrayObjectTemplate::fromArray. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/51249df95c69025fc997ef74a38be9ee7a89dfcd
Bug 1165053 - Part 7: Call SpeciesConstructor in TypedArray ctors. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/77117d1570b1b653d0b6b497f95a5a6a1bc442f2
Bug 1165053 - Part 8: Add GetGetterPure. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/504f235c11cec4a47b0979c04ab97418b55bf217
Bug 1165053 - Part 9: Add IsArrayBufferSpecies and avoid calling SpeciesConstructor on normal case. r=lth
(Assignee)

Comment 36

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/90a83b5f5f0503a442236c4dddabc199efc5663c
Backed out changeset 504f235c11ce (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/b62e02bd33f9e6852fa629b8e8f3a1c6d9b3c05e
Backed out changeset 77117d1570b1 (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/8b4dc7fe55213d623cb903a9ae941b0c55b4ceca
Backed out changeset 51249df95c69 (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/e1bbed330deb558d6fa48736b80a3ab082d6f836
Backed out changeset 27189d8e678d (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/02e1bd9d6ee46566e0cd86d010047a58a8d9b7ef
Backed out changeset 71ef5f8009ec (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/6787d2ac55923e28b22beea92561d8fcbe782338
Backed out changeset fdef3048ed77 (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/2224fda64ca50e5a3d1852c4715e6d7858992747
Backed out changeset 31ce940c509c (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/6be7ee58f77a89127d2b7a810c9a118ae947fe8f
Backed out changeset 6ec3a764a8a5 (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1072c3d9684ffc793a6f81851ad0a6ea8691cb7
Backed out changeset f66243f0197e (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/2391f1cfd6060f28a62a011c18041250ec3d1c84
Backed out changeset f3195319f23d (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/5e115ad804f53ca5f7b69820ca3dfabb382aca01
Backed out changeset fffa0e5cc406 (bug 1165053)

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1c6cd86d9d42fd97bc835e3038eda6b01341197
Backed out changeset 63d6812ce8cc (bug 1165053) for jsreftest failure
(Assignee)

Comment 37

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/57748fc14171eb0075216a6961d1910db320778a
Bug 1165053 - Part 0.1: Add PossiblyWrappedArrayBufferByteLength self-hosting intrinsic. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/a4662da80dbd571d78ac5b80b904897ec11477b2
Bug 1165053 - Part 0.2: Inline PossiblyWrappedArrayBufferByteLength self-hosting intrinsic. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/12c5896fe0d221c27a3576059b7f010560b46053
Bug 1165053 - Part 0.3: Inline ArrayBufferByteLength self-hosting intrinsic. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/38d7887d6f237ea38887bb076d9cdad5d86a54c6
Bug 1165053 - Part 1: Handle when ArrayBuffer species constructor returns wrapped ArrayBuffer. r=jwalden

https://hg.mozilla.org/integration/mozilla-inbound/rev/5cdf65882fe2279e713b578ca319e0862072a491
Bug 1165053 - Part 2: Implement %TypedArray%[@@species] getter and ArrayBuffer[@@species] getter. r=evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/93b858a8ec08f0e6e14dd595d5010614a5ea480c
Bug 1165053 - Part 3: Add SpeciesConstructor tests for TypedArray.prototype.{filter,map,slice,subarray}. r=evilpie

https://hg.mozilla.org/integration/mozilla-inbound/rev/35e1fd91ca8900481a4dfc7be0110f619d3ca1e9
Bug 1165053 - Part 4: Add SpeciesConstructor tests for ArrayBuffer.prototype.slice. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/2fc8c19b6821e2916f8c0b29ecac2373747df57b
Bug 1165053 - Part 5: Add native SpeciesConstructor wrapper. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/9811622a5e7c878809e1155f3c088e13f9cb9128
Bug 1165053 - Part 6: Refactor TypedArrayObjectTemplate::fromArray. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a92f3cced1bd8b5e3279d8610036de53ab8a32b
Bug 1165053 - Part 7: Call SpeciesConstructor in TypedArray ctors. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/35bd15c16547dd563d97e0b1c7ca825ac93d07f2
Bug 1165053 - Part 8: Add GetGetterPure. r=lth

https://hg.mozilla.org/integration/mozilla-inbound/rev/1837056e2ce28c7a5149f60fb2392af9b72abce6
Bug 1165053 - Part 9: Add IsArrayBufferSpecies and avoid calling SpeciesConstructor on normal case. r=lth

Comment 38

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/57748fc14171
https://hg.mozilla.org/mozilla-central/rev/a4662da80dbd
https://hg.mozilla.org/mozilla-central/rev/12c5896fe0d2
https://hg.mozilla.org/mozilla-central/rev/38d7887d6f23
https://hg.mozilla.org/mozilla-central/rev/5cdf65882fe2
https://hg.mozilla.org/mozilla-central/rev/93b858a8ec08
https://hg.mozilla.org/mozilla-central/rev/35e1fd91ca89
https://hg.mozilla.org/mozilla-central/rev/2fc8c19b6821
https://hg.mozilla.org/mozilla-central/rev/9811622a5e7c
https://hg.mozilla.org/mozilla-central/rev/1a92f3cced1b
https://hg.mozilla.org/mozilla-central/rev/35bd15c16547
https://hg.mozilla.org/mozilla-central/rev/1837056e2ce2
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
(Assignee)

Comment 39

2 years ago
Updated following documents

https://developer.mozilla.org/en-US/Firefox/Releases/48
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/ArrayBuffer
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/ArrayBuffer/@@species
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/@@species
Thanks arai, very nice work!

One question: I updated https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/@@species to not use "TypedArray" but actual constructors like "Int8Array" etc. My understanding is that "TypedArray" is an abstract class that will throw an error when invoked and that makes it a bit less ideal for example code. Can you confirm my changes and clear the technical review on this page? Thank you!
Flags: needinfo?(arai.unmht)
(Assignee)

Comment 41

2 years ago
I agree that using actual type in the example is better.
Thank you for fixing the document :D
Flags: needinfo?(arai.unmht)
Depends on: 1263803
Depends on: 1268574
(Assignee)

Updated

a year ago
Duplicate of this bug: 1233040
You need to log in before you can comment on or make changes to this bug.