-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Constify SystemTime
methods
#144519
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
base: master
Are you sure you want to change the base?
Constify SystemTime
methods
#144519
Conversation
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
(This does not, in fact, include changes to clippy. That's from the other PR and can be safely ignored.) |
The job Click to see the possible cause of the failure (guessed by this bot)
|
I'm mildly surprised that this is even possible at compile time, but I guess it makes sense given the internals. My concern is that this would (if/when stabilized) provide a guarantee that all future targets having But if the team is fine with this, I'm not going to get in the way. The final commit LGTM once CI passes and it's unblocked. |
My main thought process is that we've effectively guaranteed that all targets have the ability to represent this due to the presence of We even have a direct example of this for Windows, documented here: the epoch for Windows' I also just find it very doubtful that any target would have a concept of "system time" where the units are not defined by the spec. The only theoretical target we could support now which would not fall under this system is one where:
Imagine such a target where, for example, the "current time" system call had configurable units: depending on the system configuration, such a system call would return either milliseconds or seconds since the epoch. That just sounds like an absolute hell to deal with when it comes to figuring out relative times it makes a lot more sense for there to just be different system calls for different units. Plus, what happens if the configuration changes while the system is running? Like, again, it's not strictly impossible to happen, but it feels like the inability to support this kind of functionality is just the target's problem, not ours. To me, it's equivalent to a target just not having any clock functionality, which is much more likely than this, despite that also being unlikely. (Unrelated to that, I will also definitely fix the compilation errors before this is unblocked. I definitely missed a couple feature flags & const attributes.) |
This is separated out from #143949 due to the fact that it touches nontrivial system code. While the same arithmetic methods on
Instant
could be made const, since that type explicitly cannot be constructed at const-time, we don't bother. However, due to the fact thatSystemTime::UNIX_EPOCH
exists, it can be useful to create other anchors at other points in time, and thus these methods should be possible to use in const context.Reviewer note: please only pay attention to the last commit of this PR, until its blocker is merged.
Since this depends on the previous PR:
@rustbot blocked