Skip to content

Fix loop in diff_iso_date #31

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

Merged
merged 3 commits into from
Mar 4, 2024
Merged

Fix loop in diff_iso_date #31

merged 3 commits into from
Mar 4, 2024

Conversation

nekevss
Copy link
Member

@nekevss nekevss commented Mar 3, 2024

Closes #29

Fixes the issue that was being causing diff_iso_date to loop and notes it for the future.

Adds the test case that was causing the initial issue.

@nekevss nekevss added the C-bug Something isn't working label Mar 3, 2024
@nekevss nekevss added this to the 0.1 Blocking milestone Mar 3, 2024
@nekevss
Copy link
Member Author

nekevss commented Mar 4, 2024

There's an issue with calculating negative durations. I'm working on updating the above.

The issue was actually with not using div_euclid

Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

let y = year + (month - 1) / 12;
// 1. Assert: year and month are integers.
// 2. Set year to year + floor((month - 1) / 12).
let y = year + (month - 1).div_euclid(12);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, that's a really tricky bug.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took longer than I'd like to admit to notice it too 😭

@jedel1043 jedel1043 merged commit d1319da into main Mar 4, 2024
@jedel1043 jedel1043 deleted the iso-diff-fix branch March 4, 2024 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: infinite loop in CalendarDateUntil calculation
2 participants