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

Replace Foundation.Date with TimePoint #1171

Merged
merged 5 commits into from
Sep 8, 2022
Merged

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Sep 7, 2022

A TimePoint functions like a Duration, but for products that cannot yet use the niceties in Swift 5.7 like the Swift driver. This type carries a 64-bit second value and a 32-bit nanosecond value. Like Duration, it does not represent any one OS clock value, but is rather just a "point in time".

Using this type, we can simplify and clean up quite a few places that were trafficking in Foundation.Date and prepare for a future world where we have Duration too. The driver uses time point values to compare file modification times to each other - hence why we can get away with ignoring the epoch. The one oddity is the build record, which serializes time point values. But build records, like the rest of the incremental state, are not actually meant to leave a given user's machine.

@CodaFi CodaFi force-pushed the time-lords branch 2 times, most recently from abd78f0 to 0e7f367 Compare September 7, 2022 02:54
@CodaFi
Copy link
Contributor Author

CodaFi commented Sep 7, 2022

@swift-ci test

@CodaFi
Copy link
Contributor Author

CodaFi commented Sep 7, 2022

@swift-ci test Windows

Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

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

Fantastic, thank you!

@CodaFi
Copy link
Contributor Author

CodaFi commented Sep 7, 2022

@swift-ci test Windows

A TimePoint functions like a Duration, but for products that cannot yet use the niceties in Swift 5.7 like the Swift driver. This type carries a 64-bit second value and a 32-bit nanosecond value. Like Duration, it does not represent any one OS clock value, but is rather just a "point in time".

Using this type, we can simplify and clean up quite a few places that were trafficking in Foundation.Date and prepare for a future world where we have Duration too.

These simplifications come in the ensuing patches.
Now that the relevant file system utilities have been migrated, there are no more dependencies on Foundation.Date left in the library.
@CodaFi
Copy link
Contributor Author

CodaFi commented Sep 7, 2022

@swift-ci test

@compnerd
Copy link
Member

compnerd commented Sep 7, 2022

Eek; this seems to regress the test suite pretty badly:

Executed 1 test, with 34 failures (0 unexpected) in 129.017 (129.017) seconds

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Seems to regress WIndows

@CodaFi
Copy link
Contributor Author

CodaFi commented Sep 7, 2022

My bad, I need to correct for the Windows epoch difference in the file modification time function.

@CodaFi
Copy link
Contributor Author

CodaFi commented Sep 7, 2022

@swift-ci test

@CodaFi
Copy link
Contributor Author

CodaFi commented Sep 7, 2022

@swift-ci test Windows

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

Thanks, this seems to resolve the Windows issues.

@CodaFi
Copy link
Contributor Author

CodaFi commented Sep 8, 2022

CodaFi added a commit to CodaFi/swift-package-manager that referenced this pull request Sep 8, 2022
In response to swiftlang/swift-driver#1171, adopt TimePoint over Foundation.Date
in the interface of DriverExecutor.
@CodaFi CodaFi merged commit f754205 into swiftlang:main Sep 8, 2022
@CodaFi CodaFi deleted the time-lords branch September 8, 2022 22:06
CodaFi added a commit to swiftlang/swift-package-manager that referenced this pull request Sep 8, 2022
In response to swiftlang/swift-driver#1171, adopt TimePoint over Foundation.Date
in the interface of DriverExecutor.
// Other candidates include std::chrono's std::time_point::max - which is
// about 9223372036854775807 - enough to comfortably bound the age of the
// universe.
return .seconds(64_092_211_200)
Copy link
Contributor

Choose a reason for hiding this comment

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

@CodaFi, it appears this line broke building this repo for armv7, because seconds(value:) takes an Int above:

swift-driver/Sources/SwiftDriver/Utilities/DateAdditions.swift:126:21: error: integer literal '64092211200' overflows when stored into 'Int'
    return .seconds(64_092_211_200)

Maybe it should take an explicit Int64 instead?

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.

5 participants