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.

Sign up for GitHub

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

Jump to bottom

Normative: ToInteger normalizes -0 to +0 #1827

Merged
merged 1 commit into from
Feb 13, 2020
Merged

Normative: ToInteger normalizes -0 to +0 #1827

merged 1 commit into from
Feb 13, 2020

Conversation

ljharb
Copy link
Member

@ljharb ljharb commented Jan 3, 2020

Fixes #1637.

This only has an observable impact on Atomics.store.

@ljharb ljharb added normative change Affects behavior required to correctly evaluate some ECMAScript source text needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Jan 3, 2020
ljharb added a commit to ljharb/ecma262 that referenced this pull request Jan 3, 2020
@ljharb
Normative: ToInteger normalizes -0 to +0 (tc39#1827)
7b67249 
Fixes tc39#1637.

This only has an observable impact on `Atomics.store`.
ljharb added a commit to ljharb/ecma262 that referenced this pull request Jan 6, 2020
@ljharb
Normative: ToInteger normalizes -0 to +0 (tc39#1827)
b4b2cfe 
Fixes tc39#1637.

This only has an observable impact on `Atomics.store`.
syg
syg reviewed Jan 8, 2020
View reviewed changes
spec.html Outdated Show resolved Hide resolved
ljharb added a commit to ljharb/ecma262 that referenced this pull request Jan 8, 2020
@ljharb
Normative: ToInteger normalizes -0 to +0 (tc39#1827)
b65c65b 
Fixes tc39#1637.

This only has an observable impact on `Atomics.store`.
@anba
Copy link
Contributor

anba commented Feb 1, 2020

  • ToIndex should probably be changed to use SameValue instead of SameValueZero, because with the proposed changed integerIndex is never -0 and therefore we don't need the special-handling to allow integerIndex=-0 and index=+0. (ToLength(-0) returns +0, which made it necessary to use SameValueZero in ToIndex.)
  • Array.prototype.indexOf, step 7.a no longer needs the special-handling for -0.
  • Same for Array.prototype.lastIndexOf

anba
anba suggested changes Feb 1, 2020
View reviewed changes
Copy link
Contributor

@anba anba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment

ljharb added a commit to ljharb/ecma262 that referenced this pull request Feb 1, 2020
@ljharb
Normative: ToInteger normalizes -0 to +0 (tc39#1827)
ca92d09 
Fixes tc39#1637.

This only has an observable impact on `Atomics.store`.
@ljharb ljharb requested a review from anba February 1, 2020 17:39
@ljharb ljharb requested a review from syg February 4, 2020 18:32
@ljharb ljharb self-assigned this Feb 4, 2020
@ljharb ljharb added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. labels Feb 5, 2020
ljharb added a commit to ljharb/test262 that referenced this pull request Feb 9, 2020
@ljharb
Atomics.store now normalizes -0 to +0
8ec20ad 
Per tc39/ecma262#1827
@ljharb ljharb mentioned this pull request Feb 9, 2020
Merged
@ljharb
Copy link
Member Author

ljharb commented Feb 9, 2020

Tests: tc39/test262#2496

@ljharb
Normative: ToInteger normalizes -0 to +0 (tc39#1827)
ddac91d 
Fixes tc39#1637.

This only has an observable impact on `Atomics.store`.
rwaldron pushed a commit to tc39/test262 that referenced this pull request Feb 13, 2020
@ljharb @rwaldron
Atomics.store now normalizes -0 to +0
a1fbd31 
Per tc39/ecma262#1827
@ljharb ljharb added has test262 tests and removed needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Feb 13, 2020
@ljharb ljharb merged commit ddac91d into tc39:master Feb 13, 2020
@ljharb ljharb deleted the to-integer-neg-zero branch February 13, 2020 17:44
@devsnek
Copy link
Member

devsnek commented Feb 15, 2020 •
edited
Loading

This can still return -0 for inputs in the exclusive range -1 to 0.

@ljharb
Copy link
Member Author

ljharb commented Feb 15, 2020

Good call; I’ll put up a bug fix.

ljharb added a commit to ljharb/ecma262 that referenced this pull request Feb 15, 2020
@ljharb
Normative: ToInteger: fix spec bug from tc39#1827 that allows (-1,0…
d4f4652 
…) to produce `-0`
@ljharb ljharb mentioned this pull request Feb 15, 2020
Merged
ljharb added a commit to ljharb/ecma262 that referenced this pull request Feb 15, 2020
@ljharb
Normative: ToInteger: fix spec bug from tc39#1827 that allows (-1,0…
b221da8 
…) to produce `-0` (tc39#1871)
ljharb added a commit to ljharb/ecma262 that referenced this pull request Feb 15, 2020
@ljharb
Normative: ToInteger: fix spec bug from tc39#1827 that allows (-1,0…
b09fb0e 
…) to produce `-0` (tc39#1871)
ljharb added a commit to ljharb/ecma262 that referenced this pull request Feb 15, 2020
@ljharb
Normative: ToInteger: fix spec bug from tc39#1827 that allows (-1,0…
8e5ae7b 
…) to produce `-0` (tc39#1871)
sthagen added a commit to sthagen/tc39-ecma262 that referenced this pull request Feb 15, 2020
@sthagen
Merge pull request #23 from tc39/master
508cc38 
Normative: `ToInteger`: fix spec bug from tc39#1827 that allows (-1,0) to…
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Apr 23, 2020
@jmdyck
Editorial: Delete <emu-note> in TimeClip clause
170a850 
PR tc39#1827 (among other things) removed step 4 from the algorithm for TimeClip,
obsoleting the accompanying emu-note that describes "the point of step 4".

Conceivably, the note could be reworded to describe the effect of
'ToInteger' on step 3, but I don't think it'd be worth the bother.
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Apr 24, 2020
@jmdyck
Editorial: Delete <emu-note> in TimeClip clause
9d046c3 
PR tc39#1827 (among other things) removed step 4 from the algorithm for TimeClip,
obsoleting the accompanying emu-note that describes "the point of step 4".

Conceivably, the note could be reworded to describe the effect of
'ToInteger' on step 3, but I don't think it'd be worth the bother.
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Apr 30, 2020
@jmdyck
Editorial: Delete <emu-note> in TimeClip clause
b330f62 
PR tc39#1827 (among other things) removed step 4 from the algorithm for TimeClip,
obsoleting the accompanying emu-note that describes "the point of step 4".

Conceivably, the note could be reworded to describe the effect of
'ToInteger' on step 3, but I don't think it'd be worth the bother.
ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request May 6, 2020
@jmdyck @ljharb
Editorial: quick fixes (tc39#1965)
c112315 
Editorial: Reinstate an SDO rule

PR tc39#1933 deleted SDO rules that are handled by the chain production rule,
but it also deleted this one which isn't.
(It has "TV" on the left and "TRV" on the right.)

Editorial: Move an <emu-note> element

PR tc39#1490 (among other things) moved the evaluation semantics for ArrowFunction
from the Evaluation SDO to the NamedEvaluation SDO.
The accompanying <emu-note> should have moved at the same time
(in particular because of the reference to "step 3").

Editorial: Delete <emu-note> in TimeClip clause

PR tc39#1827 (among other things) removed step 4 from the algorithm for TimeClip,
obsoleting the accompanying emu-note that describes "the point of step 4".

Conceivably, the note could be reworded to describe the effect of
'ToInteger' on step 3, but I don't think it'd be worth the bother.

Editorial: Change "Step 2.a" to "Step 2.b" in RepeatMatcher note

PR tc39#1889 (among other things) inserted a step before the former 2.a,
but didn't update the note that referenced it.

Editorial: Change step 7 to step 6 in SortCompare note

Commit 9c1e076 (2015-10-26) introduced the '?' abbreviation for ReturnIfAbrupt.
This caused the ToString call on step 7 to move to step 6,
but the note that referred to it wasn't updated.

Editorial: Fix typo: "Descritor" -> "Descriptor"

Editorial: Fix typo: "GeneratorObject" -> "generator object"

(There's no such thing as a GeneratorObject.)

Editorial: Delete "as a parameter" after "is present"

(It's the only place in the spec where we use that phrasing.)

Editorial: Change "which" to "that"

... in "{String,Array,Map,Set} methods which return such iterators"

Editorial: Insert a comma in SetDefaultGlobalBindings()

Formerly, it read like "containing" modified "the property",
when it actually modified "the property descriptor".

Editorial: Change "lexical environment" to "Environment Record"

... in FunctionDeclarationInstantiation,
to balance the NOTE in the other arm of the if-else,
and also for consistency with the NOTE at 27.a.

(I should have done this in PR tc39#1697.)

Editorial: Change "step 3" to "step 4" in Note

... that accompanies the NamedEvaluation semantics for
    ArrowFunction : ArrowParameters `=&gt;` ConciseBody

PR tc39#1870 (among other things) inserted a step before the former step 3,
but didn't update the note that referenced it.
@anba anba mentioned this pull request Jul 14, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ExE-Boss ExE-Boss ExE-Boss approved these changes

@bakkot bakkot bakkot approved these changes

@michaelficarra michaelficarra michaelficarra approved these changes

@syg syg syg approved these changes

@anba anba Awaiting requested review from anba

Assignees

@ljharb ljharb

Labels
has consensus This has committee consensus. has test262 tests normative change Affects behavior required to correctly evaluate some ECMAScript source text
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Change ToInteger(-0) to return +0?
7 participants
@ljharb @anba @devsnek @syg @michaelficarra @bakkot @ExE-Boss