Editorial: Add missing checks before calling ApplyUnsignedRoundingMode#2884
Merged
Editorial: Add missing checks before calling ApplyUnsignedRoundingMode#2884
Conversation
Contributor
Author
|
Instead of checking |
Contributor
Author
|
Hmm, it looks like 9be3190 needs more checks to guarantee that |
ApplyUnsignedRoundingMode requires `r1 ≤ t < r2`, see step 2. Add missing checks to ensure this requirement is fulfilled when called from NudgeToCalendarUnit. Drive-by change: Update RoundNumberToIncrement to call GetUnsignedRoundingMode with correct spec type. Also fixes tc39#2893.
ptomato
approved these changes
Jun 28, 2024
Collaborator
ptomato
left a comment
ptomato
left a comment
There was a problem hiding this comment.
Makes sense, thanks.
I think without custom calendars, the RangeErrors may be changed to assertions, but I'll merge this first and update that if necessary when merging the PR removing custom calendars.
Implements the extra checks and assertions from the previous commit, as well as the use of abs() to simplify the didExpandCalendarUnit check and to satisfy the requirements of ApplyUnsignedRoundingMode.
Collaborator
|
I've made the corresponding changes in the reference code as well, to exercise them on the test262 suite. |
ptomato
added a commit
to fullcalendar/proposal-temporal
that referenced
this pull request
Aug 21, 2024
This reverts a line from tc39#2884 that changed an exception to an assertion. The assertion can actually be hit, so change it back.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes:
absfor all arguments passed toApplyUnsignedRoundingModeto ensure ther1 ≤ x < r2requirement inApplyUnsignedRoundingModeis fulfilled.startEpochNsandendEpochNsare now required to be correctly ordered. This ensures thatprogressis a proper fraction. (If it's not a proper fraction, thenabs(total)can be larger thanabs(r2), which is invalid forApplyUnsignedRoundingMode.)roundedUnitagainstabs(r2).