New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypedArray constructors don't call the species constructor for ArrayBuffers they create. #455

Open
kmiller68 opened this Issue Mar 4, 2016 · 12 comments

Comments

Projects
None yet
5 participants
@kmiller68

kmiller68 commented Mar 4, 2016

In the TypedArray constructors if a user passes a TypedArray, v, as an argument into the constructor we create a new backing ArrayBuffer for the new view. However, in the current spec, we do not call the species constructor, s, for v's backing ArrayBuffer. Instead, we create a new ArrayBuffer with the prototype set to s's prototype. This is inconsistent with the way species constructors work elsewhere in the spec. Is this intentional or a bug?

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Mar 4, 2016

Member

It looks like a bug to me. I think that when I spec'ed this I totally overloaded the possibility that an ArrayBuffer subclass could have a non-empty constructor that needs to run when it is instantiated.

The call to AllocateArrayBuffer probably should be replaced to a call to something similarly to ArraySpeciesCreate (except for the a Array specific stuff)

Hopefully it isn't to late to fix this, as I suspect that subclassing of ArrayBuffer isn't happening yet on the web.

Member

allenwb commented Mar 4, 2016

It looks like a bug to me. I think that when I spec'ed this I totally overloaded the possibility that an ArrayBuffer subclass could have a non-empty constructor that needs to run when it is instantiated.

The call to AllocateArrayBuffer probably should be replaced to a call to something similarly to ArraySpeciesCreate (except for the a Array specific stuff)

Hopefully it isn't to late to fix this, as I suspect that subclassing of ArrayBuffer isn't happening yet on the web.

@kmiller68

This comment has been minimized.

Show comment
Hide comment
@kmiller68

kmiller68 Mar 4, 2016

I agree with your proposed change but, obviously, we would need to check that the thing returned by the species constructor is actually an ArrayBuffer. I'm fairly confident that this change can still happen, however, since species constructors aren't really pollyfillable (at least not in the TypedArray constructor case).

kmiller68 commented Mar 4, 2016

I agree with your proposed change but, obviously, we would need to check that the thing returned by the species constructor is actually an ArrayBuffer. I'm fairly confident that this change can still happen, however, since species constructors aren't really pollyfillable (at least not in the TypedArray constructor case).

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Mar 4, 2016

Member

I agree with your proposed change but, obviously, we would need to check that the thing returned by the species constructor is actually an ArrayBuffer.

The hypothetical ArrayBufferSpeciesCreate abstract operation should check that the new object has a [[ArrayBufferData]] internal slot

Member

allenwb commented Mar 4, 2016

I agree with your proposed change but, obviously, we would need to check that the thing returned by the species constructor is actually an ArrayBuffer.

The hypothetical ArrayBufferSpeciesCreate abstract operation should check that the new object has a [[ArrayBufferData]] internal slot

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Mar 4, 2016

Member

It's very important that the [[ArrayBufferData]] internal slot be checked for, otherwise this change would have a high implementation burden. Otherwise this kind of change sounds doable to me.

@allenwb Are you saying you want to do something like ArraySpeciesCreate does with respect for realms? Or with respect to only doing it if the input is an ArrayBuffer? I'm not sure what need we have for either, as opposed to just using SpeciesConstructor.

Another possibility is to not actually do much for ArrayBuffer subclassing in TypedArrays. We could just construct an %ArrayBuffer% to back the constructed TypedArray. What was the use case for ArrayBuffer subclassing? It doesn't seem like TypedArrays would call out to any ArrayBuffer methods (besides, in the future, the constructor) in the TypedArray methods or property reads.

Member

littledan commented Mar 4, 2016

It's very important that the [[ArrayBufferData]] internal slot be checked for, otherwise this change would have a high implementation burden. Otherwise this kind of change sounds doable to me.

@allenwb Are you saying you want to do something like ArraySpeciesCreate does with respect for realms? Or with respect to only doing it if the input is an ArrayBuffer? I'm not sure what need we have for either, as opposed to just using SpeciesConstructor.

Another possibility is to not actually do much for ArrayBuffer subclassing in TypedArrays. We could just construct an %ArrayBuffer% to back the constructed TypedArray. What was the use case for ArrayBuffer subclassing? It doesn't seem like TypedArrays would call out to any ArrayBuffer methods (besides, in the future, the constructor) in the TypedArray methods or property reads.

@kmiller68

This comment has been minimized.

Show comment
Hide comment
@kmiller68

kmiller68 Mar 4, 2016

Checking for the [[ArrayBufferData]] slot is totally reasonable. I'm not sure how much the use case subclassing ArrayBuffer is important; I think it makes more sense for the language to be consistent. That said, I think that species constructors are already inconsistently used by builtin functions. So I'm largely indifferent about either @littledan's or @allenwb's solutions but I dislike the status quo.

kmiller68 commented Mar 4, 2016

Checking for the [[ArrayBufferData]] slot is totally reasonable. I'm not sure how much the use case subclassing ArrayBuffer is important; I think it makes more sense for the language to be consistent. That said, I think that species constructors are already inconsistently used by builtin functions. So I'm largely indifferent about either @littledan's or @allenwb's solutions but I dislike the status quo.

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Mar 4, 2016

Member

Are you saying you want to do something like ArraySpeciesCreate does with respect for realms?
Nope, that realm stuff only exists to match legacy Array constructor behavior

just using SpeciesConstructor.
need actually call the constructor and check that the result has a [[ArrayBufferData]] internal slot. It could be done inline but there at (at least) two places (ClineArrayBuffer) is the other were this sort of species construction is done.

What was the use case for ArrayBuffer subclassing?
Prior to ES6 there were a noticeable number of complaints about the fact that the built-in classes were not subclassable. So for ES6, we made of point to designing for subclassing. Subclassing is an open-ended extension mechanism. The use case, is that the JS dev wants to create a subclass. We can't anticipate why. Maybe it's just to add some methods without modify the built-in constructor's prototype. Overall, we don't need a specific use case to justify making sure that the built-in are reasonably subclassable. Rather it is the design policy we adopted.

In the case of ArrayBuffer, it seems quite plausible that SharedArrayBuffer (or something similar) would be a subclass of ArrayBuffer and that it's @@species method would ArrayBuffer rather than this.

Member

allenwb commented Mar 4, 2016

Are you saying you want to do something like ArraySpeciesCreate does with respect for realms?
Nope, that realm stuff only exists to match legacy Array constructor behavior

just using SpeciesConstructor.
need actually call the constructor and check that the result has a [[ArrayBufferData]] internal slot. It could be done inline but there at (at least) two places (ClineArrayBuffer) is the other were this sort of species construction is done.

What was the use case for ArrayBuffer subclassing?
Prior to ES6 there were a noticeable number of complaints about the fact that the built-in classes were not subclassable. So for ES6, we made of point to designing for subclassing. Subclassing is an open-ended extension mechanism. The use case, is that the JS dev wants to create a subclass. We can't anticipate why. Maybe it's just to add some methods without modify the built-in constructor's prototype. Overall, we don't need a specific use case to justify making sure that the built-in are reasonably subclassable. Rather it is the design policy we adopted.

In the case of ArrayBuffer, it seems quite plausible that SharedArrayBuffer (or something similar) would be a subclass of ArrayBuffer and that it's @@species method would ArrayBuffer rather than this.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Mar 4, 2016

Member

Tangent, but in the current SharedArrayBuffer draft spec, this subclass relationship doesn't exist http://tc39.github.io/ecmascript_sharedmem/shmem.html#StructuredData.SharedArrayBuffer.prototype . Should this be changed? @allenwb @lars-t-hansen

@kmiller68 Agreed that inconsistencies are worth fixing, especially after this style of semantics was removed from TypedArrays.

Member

littledan commented Mar 4, 2016

Tangent, but in the current SharedArrayBuffer draft spec, this subclass relationship doesn't exist http://tc39.github.io/ecmascript_sharedmem/shmem.html#StructuredData.SharedArrayBuffer.prototype . Should this be changed? @allenwb @lars-t-hansen

@kmiller68 Agreed that inconsistencies are worth fixing, especially after this style of semantics was removed from TypedArrays.

@lars-t-hansen

This comment has been minimized.

Show comment
Hide comment
@lars-t-hansen

lars-t-hansen Mar 22, 2016

Contributor

@littledan, SharedArrayBuffer should IMO not be a subtype of ArrayBuffer. (Sorry I did not see this sooner.) SharedArrayBuffers cannot be detached, for one thing, and have different structured clone semantics (a SAB must always be transfered, or an exception is thrown) so a SAB does not substitute for an AB.

I believe @allenwb has brought up this once before but I'm not aware of any reason why it's a good idea to create that subtype relationship. If anyone wants to pursue this, please file a bug at the tc39/ecmascript_sharedmem repo.

Contributor

lars-t-hansen commented Mar 22, 2016

@littledan, SharedArrayBuffer should IMO not be a subtype of ArrayBuffer. (Sorry I did not see this sooner.) SharedArrayBuffers cannot be detached, for one thing, and have different structured clone semantics (a SAB must always be transfered, or an exception is thrown) so a SAB does not substitute for an AB.

I believe @allenwb has brought up this once before but I'm not aware of any reason why it's a good idea to create that subtype relationship. If anyone wants to pursue this, please file a bug at the tc39/ecmascript_sharedmem repo.

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Mar 21, 2018

Member

@kmiller68 could you summarize the next steps here? Thanks!

Member

ljharb commented Mar 21, 2018

@kmiller68 could you summarize the next steps here? Thanks!

@allenwb

This comment has been minimized.

Show comment
Hide comment
@allenwb

allenwb Mar 21, 2018

Member

SharedArrayBuffer should IMO not be a subtype of ArrayBuffer. ... a SAB does not substitute for an AB.

To quote a fairly famous paper, in a dynamic languages like JS, "subclassing is not subtyping".

I don't really have a current position on whether SAB should or shouldn't be a subclass of AB. I believe we did discuss it in an issue when you were originally working on the spec. and I probably took a position then. It would be good to try to find that discussion.

Member

allenwb commented Mar 21, 2018

SharedArrayBuffer should IMO not be a subtype of ArrayBuffer. ... a SAB does not substitute for an AB.

To quote a fairly famous paper, in a dynamic languages like JS, "subclassing is not subtyping".

I don't really have a current position on whether SAB should or shouldn't be a subclass of AB. I believe we did discuss it in an issue when you were originally working on the spec. and I probably took a position then. It would be good to try to find that discussion.

@lars-t-hansen

This comment has been minimized.

Show comment
Hide comment
@lars-t-hansen

lars-t-hansen Mar 21, 2018

Contributor

I sure don't have a position on AB->SAB subclassing at the moment either. I observe that the current spec says that SAB has the [[ArrayBufferData]] slot with a Shared Data Block value (so it smells like an ArrayBuffer and can be used with TypedArray) but that the prototype hierarchy leads directly up to the Object prototype, not via ArrayBuffer (so it is not an ArrayBuffer).

Contributor

lars-t-hansen commented Mar 21, 2018

I sure don't have a position on AB->SAB subclassing at the moment either. I observe that the current spec says that SAB has the [[ArrayBufferData]] slot with a Shared Data Block value (so it smells like an ArrayBuffer and can be used with TypedArray) but that the prototype hierarchy leads directly up to the Object prototype, not via ArrayBuffer (so it is not an ArrayBuffer).

@ljharb

This comment has been minimized.

Show comment
Hide comment
@ljharb

ljharb Mar 21, 2018

Member

I'd say the internal slot - the brand - tells you what it is, whereas the prototype tells you what it's an instance of.

Member

ljharb commented Mar 21, 2018

I'd say the internal slot - the brand - tells you what it is, whereas the prototype tells you what it's an instance of.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment