Skip to content
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

Editorial: `FlattenIntoArray`: add assertions #1620

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@ljharb
Copy link
Member

commented Jul 10, 2019

Add some assertions to clarify the intended usage of this abstract operation.

I wanted to assert that target is an Array exotic object, but I think because of ArraySpeciesCreate I can't.

@ljharb ljharb requested review from michaelficarra, jmdyck, zenparsing and tc39/ecma262-editors Jul 10, 2019

@jmdyck
Copy link
Collaborator

left a comment

I wanted to assert that _target_ is an Array exotic object, but I think because of ArraySpeciesCreate I can't.

I think you're right. That is, the result of ArraySpeciesCreate (ignoring abrupts) isn't guaranteed to be an Array (despite what its preamble says), because a subclass of Array could override its @@species property to return a non-Array constructor?

Show resolved Hide resolved spec.html
1. Assert: Type(_source_) is Object.
1. Assert: _sourceLen_ is an integer Number ≥ 0.
1. Assert: _start_ is an integer Number ≥ 0.
1. Assert: _depth_ is an integer Number ≥ 0.

This comment has been minimized.

Copy link
@jmdyck

jmdyck Jul 10, 2019

Collaborator

When A.p.flat invokes this operation, the value passed to _depth_ can be any normal return from ToInteger, which includes negative integers and also some odd values like +/- zero and +/- infinity. The infinities fail the "integer" part of this assertion, and the negative integers fail the ≥ 0 part. Maybe change to:

Assert: _depth_ is an integer Number or *+∞* or *-∞*.

Alternatively, A.p.flat could at least map all negatives to (positive) zero. Then you could say:

Assert: _depth_ is either *+∞* or an integer Number ≥ 0.

(Tangent: I was going to complain about _depth_ - 1 when _depth_ is +infinity, but I guess it became well-defined with the merge of PR #1135 ("an operation with no subscript is interpreted to be a Number operation"). However, it might be worth a Note.)

This comment has been minimized.

Copy link
@michaelficarra

michaelficarra Jul 12, 2019

Member

Alternatively, A.p.flat could at least map all negatives to (positive) zero.

I like this suggestion a lot.

@ljharb ljharb force-pushed the ljharb:flattenintoarray_assertions branch 2 times, most recently from eb35757 to e893b3b Jul 10, 2019

ljharb added a commit to ljharb/ecma262 that referenced this pull request Jul 10, 2019

@ljharb ljharb requested a review from jmdyck Jul 10, 2019

Show resolved Hide resolved spec.html

@ljharb ljharb force-pushed the ljharb:flattenintoarray_assertions branch from e893b3b to 6136ace Jul 11, 2019

@ljharb ljharb requested a review from jmdyck Jul 11, 2019

@jmdyck
Copy link
Collaborator

left a comment

Looks okay to me.

Show resolved Hide resolved spec.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.