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

RFC: introduce notations for Duration and Timestamp and remove dangerous OfNat instances #6793

Open
TwoFX opened this issue Jan 27, 2025 · 2 comments
Labels
P-medium We may work on this issue if we find the time RFC accepted RFC is waiting for a corresponding PR (external or internal) RFC Request for comments

Comments

@TwoFX
Copy link
Member

TwoFX commented Jan 27, 2025

Proposal

We currently provide OfNat Duration and OfNat Timestamp instances which turn natural numbers into seconds and seconds since Unix epoch, respectively. Keeping with the design of "make incorrect conversions impossible and make dubious conversions explicit", we should change this as follows:

  • Introduce notation for Duration types (this was considered during initial implementation, but was postponed to reduce the size of the PR).
  • Add Timestamp.ofDurationSinceUnixEpoch (this is just the constructor).
  • Remove the OfNat Duration and OfNat Timestamp instances.
  • Possibly introduce notation for Timestamp that allows entering a timestamp with time zone in ISO format without having to go through ZonedDateTime.

Impact

Add 👍 to issues you consider important. If others benefit from the changes in this proposal being added, please ask them to add 👍 to it.

@TwoFX TwoFX added the RFC Request for comments label Jan 27, 2025
@Kha Kha added the RFC accepted RFC is waiting for a corresponding PR (external or internal) label Jan 28, 2025
@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Jan 28, 2025
@lambda-fairy
Copy link

lambda-fairy commented Jan 30, 2025

I wonder if it's worth making the constructor of Timestamp harder to access as well. (With protected?)

Semantically, Timestamp is a physical point in time. The fact that it's an offset from Jan 1, 1970 is an implementation detail and should be hidden.

For reference, Rust enforces that semantic boundary strictly, such that getting the Unix epoch offset requires subtraction: timestamp.duration_since(UNIX_EPOCH). (I'm not suggesting we need to go that far – just offering a data point.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium We may work on this issue if we find the time RFC accepted RFC is waiting for a corresponding PR (external or internal) RFC Request for comments
Projects
None yet
Development

No branches or pull requests

4 participants