Skip to content
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

[SR-2846] Calendar date(byAdding:to:wrappingComponents:) returns nil when it shouldn't #4323

Closed
swift-ci opened this issue Oct 4, 2016 · 4 comments

Comments

@swift-ci
Copy link
Contributor

swift-ci commented Oct 4, 2016

Previous ID SR-2846
Radar None
Original Reporter UberJason (JIRA User)
Type Bug
Status Closed
Resolution Done
Environment

This code functions on OS X 10.11 but not in an Ubuntu 14.04 Docker image.

Here's the swift -version output on OS X 10.11:

Apple Swift version 3.0 (swiftlang-800.0.46.2 clang-800.0.38)
Target: x86_64-apple-macosx10.9

And the swift -version output on the Ubuntu 14.04 Docker image:

Swift version 3.0 (swift-3.0-RELEASE)
Target: x86_64-unknown-linux-gnu
Additional Detail from JIRA
Votes 0
Component/s Foundation
Labels Bug, Linux
Assignee None
Priority Medium

md5: a9665d5c4946832d28b144569d47aae4

Issue Description:

The below snippet of code works on OS X 10.11 El Capitan but not on an Ubuntu 14.04 Docker image with Swift 3.0 RELEASE installed or the October 2 snapshot. Tested in REPL as well as, originally, in a Vapor web application, where I was doing some Date manipulations.

import Foundation

let today = Date()
let diffComponents = DateComponents(day: -1)
let newDate = Calendar.current.date(byAdding: diffComponents, to: today)        //returns nil

A few notes:

  • today is a valid Date object

  • Calendar.current is a valid Calendar object

  • I also tried manually checking that the date(from: DateComponents) function works, and it does:

     todayComponents = DateComponents(year: 2016, month: 10, day: 4) 
    Calendar.current.date(from: todayComponents)        // returns valid Date on both platforms
@alblue
Copy link
Contributor

alblue commented Oct 5, 2016

I think the bug is here:

https://github.com/apple/swift-corelibs-foundation/blob/master/Foundation/NSCalendar.swift#L464

_convert(comps.isLeapMonth, type: "L", vector: &vector, compDesc: &compDesc)

The leap month is defined with a capital letter here, but a lower-case letter when it's decoded:

case 'l': return UCAL_IS_LEAP_MONTH;

case 'l': return UCAL_IS_LEAP_MONTH;

The lower-case l should be the correct version so this probably needs to be changed here. In addition, there are optional-of-bool issues here; the 'comps.isLeapMonth' is a boolean value, which means it attempts to roll forward the leap month (which doesn't make sense). I think the error was introduced here, although it didn't help that the capitalisation was also wrong at the time:

d3300b7

@swift-ci
Copy link
Contributor Author

swift-ci commented Oct 5, 2016

Comment by Jason Ji (JIRA)

Thanks Alex - I saw your response on the mailing list too, but for posterity's sake I'll re-post my reply here as well:

I came to the same conclusion, with one more minor bug (bringing the total to three): the definition of the date(byAdding:to:options: ) function itself was attempting to do math on CFAbsoluteTime 0.0, rather than the passed-in date's own timeIntervalSinceReferenceDate. I submitted a pull request to fix these issues last night here: #668

Your email address suggests you're with Apple; do you have the ability to review that PR and merge it in if it meets your specs? Our team kind of desperately needs this bug fix, as we can't do date math on our Swift-based server right now. Thanks!!

@swift-ci
Copy link
Contributor Author

swift-ci commented Oct 5, 2016

Comment by Jason Ji (JIRA)

I submitted PR #668 on GitHub to fix this issue: #668

@swift-ci
Copy link
Contributor Author

swift-ci commented Oct 5, 2016

Comment by Jason Ji (JIRA)

Resolved in PR #668: #668

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants