Skip to content

Fix Bugs in NSCalendar Date Calculations #44

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

Closed
wants to merge 0 commits into from
Closed

Fix Bugs in NSCalendar Date Calculations #44

wants to merge 0 commits into from

Conversation

brownleej
Copy link

I ran into a couple of crashers when writing tests for NSCalendar date calculations, and this PR is designed to address them.

The first crasher was in initializing an NSCalendar instance. It was attempting to compare the calendar identifiers to the kCF*Calendar constants, but that was getting routed through NSObject#isEqual, which prevented it from matching up the calendar identifiers. I was able to address this by adding an implementation of isEqual for NSString, but I don't know if this implementation is sufficient or correct, so any feedback on that would be welcome.

The second crasher was in _CFCalendarDecomposeAbsoluteTimeV, getting the value for the "leap month" component. The mapping for that component had not been defined yet.

@@ -422,6 +422,7 @@ static UCalendarDateFields __CFCalendarGetICUFieldCodeFromChar(char ch) {
case 'G': return UCAL_ERA;
case 'y': return UCAL_YEAR;
case 'M': return UCAL_MONTH;
case 'l': return UCAL_IS_LEAP_MONTH;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is something we are going to have to verify since it will upstream into CoreFoundation on darwin.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks correct to me.

@parkera
Copy link
Contributor

parkera commented Dec 6, 2015

@brownleej This looks good, once the couple of issues @phausler brought up are addressed (splitting out NSString diffs and adding test file to build.py).

@phausler
Copy link
Contributor

phausler commented Dec 7, 2015

I ran this through the tests by manually fixing up the calendar issue and it seems we have a second issue here that causes a crash in the tests the _identifier in the calendar is null which is attempted to be released in __CFCalendarDeallocate which crashes. It might be good to address that in this as well. (not certain if that is a setup or a implementation flaw currently without further debugging)

@brownleej brownleej closed this Dec 7, 2015
atrick pushed a commit to atrick/swift-corelibs-foundation that referenced this pull request Jan 12, 2021
[swiftpm] Add more tests and lightly refactor the swiftpm build system
kateinoigakukun pushed a commit to kateinoigakukun/swift-corelibs-foundation that referenced this pull request Apr 29, 2021
…6142f411f6831e9e0932

Add a test that makes sure a static Foundation is included in toolcha…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants