Fixes for editorial issues #2708
Merged
Conversation
This file contains 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
In BalancePossiblyInfiniteTimeDurationRelative, we call AddDaysToZonedDateTime with the overflow parameter unconditionally set to 'constrain'. In this case, we can just omit it, since that's also the default value. Thanks to Anba for spotting this. Closes: #2693
I thought we were always calling this with largestUnit "day" if the largestUnit was smaller than days, but that's not the case. Just remove this assertion because it doesn't make any sense, and the spec is correct without it. Closes: #2695
This avoids calling NanosecondsToDays with the precalculated datetime parameter present but undefined, which is disallowed according to that operation's type assertions. Credits to Anba for catching this. Closes: #2694
Thanks again to Anba for spotting that this _instant_ is used lower down even if it wasn't created. Closes: #2702
…spec Change the reference code so that these operations work exactly like in the spec text, instead of the looping implementation that the code previously had.
…oDays This approach was suggested by Anba. All of these steps are unobservable in any case, so they can be edited and rearranged as necessary. Closes: #2703
As suggested by Anba, the precalculation will be done at the start of DifferenceZonedDateTime if it is not done prior, so we can unconditionally do it prior. Closes: #2704
AdjustRoundedDurationDays can be called with _precalculatedPlainDateTime_ undefined, but it may only be undefined if we take the fast path. Express this better with an assertion. Thanks to Anba for pointing this out. Closes: #2705
In Duration.p.round(), the smallestUnit condition for precalculating the PlainDateTime is already subsumed by the roundingGranularityIsNoop condition, so also checking smallestUnit is redundant. Thanks to Anba for pointing this out. Closes: #2698
As pointed out by Anba, in this call to RoundDuration the years...days parameters are not used within RoundDuration because unit is less than days. They are returned unchanged and discarded, so we may as well not pass them in. This change isn't observable by userland code. Closes: #2617
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2708 +/- ##
==========================================
+ Coverage 96.29% 96.33% +0.03%
==========================================
Files 20 20
Lines 12201 12211 +10
Branches 2286 2274 -12
==========================================
+ Hits 11749 11763 +14
- Misses 389 390 +1
+ Partials 63 58 -5
☔ View full report in Codecov by Sentry. |
Ms2ger
approved these changes
Oct 12, 2023
Comment on lines
+5913
to
+5915
| if (s1 !== s2) return ComparisonResult(s1 - s2); | ||
| if (ms1 !== ms2) return ComparisonResult(ms1 - ms2); | ||
| if (µs1 !== µs2) return ComparisonResult(µs1 - µs2); |
There was a problem hiding this comment.
codecov claims the second and microsecond branches are not covered, could you have a look?
There was a problem hiding this comment.
I added another commit on to tc39/test262#3944: tc39/test262@315b4df
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Dec 15, 2023
Implement the changes from: tc39/proposal-temporal#2708 Differential Revision: https://phabricator.services.mozilla.com/D195881
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified-and-comments-removed
that referenced
this pull request
Dec 16, 2023
Implement the changes from: tc39/proposal-temporal#2708 Differential Revision: https://phabricator.services.mozilla.com/D195881 UltraBlame original commit: 09aff6fb6f431199b708c9b326de74081373a598
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-wordified
that referenced
this pull request
Dec 16, 2023
Implement the changes from: tc39/proposal-temporal#2708 Differential Revision: https://phabricator.services.mozilla.com/D195881 UltraBlame original commit: 09aff6fb6f431199b708c9b326de74081373a598
gecko-dev-updater
pushed a commit
to marco-c/gecko-dev-comments-removed
that referenced
this pull request
Dec 16, 2023
Implement the changes from: tc39/proposal-temporal#2708 Differential Revision: https://phabricator.services.mozilla.com/D195881 UltraBlame original commit: 09aff6fb6f431199b708c9b326de74081373a598
aosmond
pushed a commit
to aosmond/gecko
that referenced
this pull request
Dec 16, 2023
Implement the changes from: tc39/proposal-temporal#2708 Differential Revision: https://phabricator.services.mozilla.com/D195881
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Most of these are minor issues with #2671. Best reviewed commit by commit, each commit message contains an explanation and a link to the issue tracker.
Cc @anba who reported all of these issues.