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

Reorder endNs/endInstant computation in NanosecondsToDays #2529

Closed
anba opened this issue Mar 24, 2023 · 2 comments · Fixed by #2657
Closed

Reorder endNs/endInstant computation in NanosecondsToDays #2529

anba opened this issue Mar 24, 2023 · 2 comments · Fixed by #2657
Labels
normative Would be a normative change to the proposal spec-text Specification text involved
Milestone
Stage "3.5"

Comments

@anba
Copy link
Contributor

anba commented Mar 24, 2023

When nanoseconds is too large, endNs won't be a valid epoch nanoseconds value and a RangeError is thrown.

Implementations currently can't handle this case early, because the GetPlainDateTimeFor call can have user-visible side-effects.

Reordering the steps as follows allows implementations to handle the error case earlier.

  1. Let startNs be ℝ(relativeTo.[[Nanoseconds]]).
  2. Let startInstant be ! CreateTemporalInstant(ℤ(startNs)).
  3. Let endNs be startNs + nanoseconds.
  4. If ! IsValidEpochNanoseconds(ℤ(endNs)) is false, throw a RangeError exception.
  5. Let endInstant be ! CreateTemporalInstant(ℤ(endNs)).
  6. Let startDateTime be ? GetPlainDateTimeFor(relativeTo.[[TimeZone]], startInstant, relativeTo.[[Calendar]]).
  7. Let endDateTime be ? GetPlainDateTimeFor(relativeTo.[[TimeZone]], endInstant, relativeTo.[[Calendar]]).
@gibson042 gibson042 added spec-text Specification text involved normative Would be a normative change to the proposal labels Mar 30, 2023
@gibson042
Copy link
Collaborator

This change makes sense and should be revisited after #2519.

@gibson042 gibson042 added this to the Stage "3.5" milestone Mar 30, 2023
ptomato added a commit that referenced this issue Apr 26, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
ba5a3d2 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
@ptomato
Copy link
Collaborator

ptomato commented Apr 26, 2023

This is now handled by #2519 — there was already a change in that PR which changed the order of operations around that invocation, so I think we can incorporate this as a review comment on that PR.

@ptomato ptomato mentioned this issue Apr 27, 2023
Closed
ptomato added a commit that referenced this issue Apr 27, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
b4ffe4c 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue May 3, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
41cc54c 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue May 11, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
c5ac8ec 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue May 13, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
b458389 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue May 13, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
c4256df 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue May 19, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
8fb2583 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue May 31, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
b651937 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Jun 12, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
47c12f6 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Jun 20, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
db6bbf7 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Aug 18, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
a6c68d0 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Aug 18, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
1e1e313 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
@ptomato ptomato mentioned this issue Aug 18, 2023
Merged
ptomato added a commit that referenced this issue Aug 22, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
4b7e274 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Aug 26, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
8aebe23 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Sep 11, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
65954ad 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Sep 12, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
046482e 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Sep 12, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
c60c39d 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Sep 13, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
d9f043a 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
ptomato added a commit that referenced this issue Sep 13, 2023
@ptomato
Normative: Separate zoned and plain operations in RoundDuration
742248e 
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
normative Would be a normative change to the proposal spec-text Specification text involved
Projects
None yet
Milestone
Stage "3.5"
Development

Successfully merging a pull request may close this issue.

Audit of user code calls, part 2 tc39/proposal-temporal
3 participants
@ptomato @gibson042 @anba