-
Notifications
You must be signed in to change notification settings - Fork 153
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
AddDuration: Unnecessary call to GetPlainDateTimeFor #2696
Comments
Thanks for spotting, as usual! I'll add a fix for this one on to part 4 (#2519). |
ptomato
added a commit
that referenced
this issue
Oct 11, 2023
This is an addendum to part 3 (#2671) that removes a ZonedDateTime to PlainDateTime conversion in one place where a fast path doesn't need the PlainDateTime. This is observable in the case where you add or subtract two Temporal.Durations that don't have any units higher than hours. Credit to Anba for spotting this. Closes: #2696
ptomato
added a commit
that referenced
this issue
Oct 18, 2023
This is an addendum to part 3 (#2671) that removes a ZonedDateTime to PlainDateTime conversion in one place where a fast path doesn't need the PlainDateTime. This is observable in the case where you add or subtract two Temporal.Durations that don't have any units higher than hours. Credit to Anba for spotting this. Closes: #2696
ptomato
added a commit
that referenced
this issue
Oct 26, 2023
This is an addendum to part 3 (#2671) that removes a ZonedDateTime to PlainDateTime conversion in one place where a fast path doesn't need the PlainDateTime. This is observable in the case where you add or subtract two Temporal.Durations that don't have any units higher than hours. Credit to Anba for spotting this. Closes: #2696
ptomato
added a commit
that referenced
this issue
Nov 6, 2023
This is an addendum to part 3 (#2671) that removes a ZonedDateTime to PlainDateTime conversion in one place where a fast path doesn't need the PlainDateTime. This is observable in the case where you add or subtract two Temporal.Durations that don't have any units higher than hours. Credit to Anba for spotting this. Closes: #2696
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
AddDuration only needs to call GetPlainDateTimeFor when calendar units are present. Current steps 10-11 can be rewritten as follows:
When
largestUnit
is smaller than"day"
, all calendar units ({y,mon,w,d}{1,2}
) are zero, so the two AddZonedDateTime calls will take the AddInstant fast path and noTemporal.PlainDateTime
object is needed.The text was updated successfully, but these errors were encountered: