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

Normative: Use floor instead of truncate in epoch time getters #2424

Merged
merged 1 commit into from
Nov 29, 2022

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Oct 20, 2022

In Temporal.{Instant,ZonedDateTime}.epoch{S,Millis,Micros}econds, the rounding should use floor instead of truncate semantics, in order to always truncate towards the beginning of time, as .toString() and .round() do.

Closes: #2423

In Temporal.{Instant,ZonedDateTime}.epoch{S,Millis,Micros}econds, the
rounding should use floor instead of truncate semantics, in order to
always truncate towards the beginning of time, as .toString() and .round()
do.

Closes: #2423
@ptomato ptomato added spec-text Specification text involved normative Would be a normative change to the proposal labels Oct 20, 2022
@ptomato ptomato marked this pull request as draft October 20, 2022 23:05
@ptomato
Copy link
Collaborator Author

ptomato commented Oct 20, 2022

Draft until presented at TC39.

The reference code already worked as described in this change, and the behaviour is already covered by test262.

@codecov
Copy link

codecov bot commented Oct 20, 2022

Codecov Report

Merging #2424 (df39373) into main (0af69d3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2424   +/-   ##
=======================================
  Coverage   94.66%   94.66%           
=======================================
  Files          20       20           
  Lines       11074    11074           
  Branches     1970     1970           
=======================================
  Hits        10483    10483           
  Misses        544      544           
  Partials       47       47           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ptomato ptomato marked this pull request as ready for review November 29, 2022 19:15
@ptomato
Copy link
Collaborator Author

ptomato commented Nov 29, 2022

This change achieved consensus at the TC39 plenary meeting today.

@ptomato ptomato merged commit c0209c2 into main Nov 29, 2022
@ptomato ptomato deleted the 2423-fix-truncation-in-epoch-properties branch November 29, 2022 19:17
anba added a commit to anba/test262 that referenced this pull request Mar 24, 2023
ptomato added a commit that referenced this pull request Apr 8, 2023
See #2423 for the issue and #2424 for the spec text change, which was
already adopted some time ago. I thought the reference code already worked
like this, but it does not.

Anba pointed this out in tc39/test262#3802 and
provided correct test262 tests for the feature.

Note that the big-integer library doesn't have a floor division function.
We roll our own. If the divisor or dividend is negative (but not both) and
there is a remainder, then we subtract 1 from the result (round towards
negative infinity instead of 0).
Ms2ger pushed a commit to anba/test262 that referenced this pull request Apr 10, 2023
Ms2ger pushed a commit to tc39/test262 that referenced this pull request Apr 10, 2023
ptomato added a commit that referenced this pull request Apr 10, 2023
See #2423 for the issue and #2424 for the spec text change, which was
already adopted some time ago. I thought the reference code already worked
like this, but it does not.

Anba pointed this out in tc39/test262#3802 and
provided correct test262 tests for the feature.

Note that the big-integer library doesn't have a floor division function.
We roll our own. If the divisor or dividend is negative (but not both) and
there is a remainder, then we subtract 1 from the result (round towards
negative infinity instead of 0).
ptomato added a commit that referenced this pull request Apr 10, 2023
See #2423 for the issue and #2424 for the spec text change, which was
already adopted some time ago. I thought the reference code already worked
like this, but it does not.

Anba pointed this out in tc39/test262#3802 and
provided correct test262 tests for the feature.

Note that the big-integer library doesn't have a floor division function.
We roll our own. If the divisor or dividend is negative (but not both) and
there is a remainder, then we subtract 1 from the result (round towards
negative infinity instead of 0).
justingrant pushed a commit that referenced this pull request Apr 20, 2023
See #2423 for the issue and #2424 for the spec text change, which was
already adopted some time ago. I thought the reference code already worked
like this, but it does not.

Anba pointed this out in tc39/test262#3802 and
provided correct test262 tests for the feature.

Note that the big-integer library doesn't have a floor division function.
We roll our own. If the divisor or dividend is negative (but not both) and
there is a remainder, then we subtract 1 from the result (round towards
negative infinity instead of 0).

UPSTREAM_COMMIT=6e0ad2030170540084a4a400005cc2bc8c791bdb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
normative Would be a normative change to the proposal spec-text Specification text involved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent truncation for pre-epoch exact times in epochSeconds, epochMilliseconds, and epochMicroseconds
2 participants