-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
SystemTime conversions invite programmer error #52522
Copy link
Copy link
Open
Labels
A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsArea: Documentation for any part of the project, including the compiler, standard library, and toolsC-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.Relevant to the library API team, which will review and decide on the PR/issue.
Metadata
Metadata
Assignees
Labels
A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsArea: Documentation for any part of the project, including the compiler, standard library, and toolsC-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.Relevant to the library API team, which will review and decide on the PR/issue.
Type
Fields
Give feedbackNo fields configured for issues without a type.
Yesterday, I found and reported a panic bug in
serde_jsonwhich traces its origins to the mechanism for extracting usable information fromSystemTime.Specifically, using the best method they believed to be available, it would panic when I tried to serialize metadata for files with negative
mtimevalues. (I have one file with an mtime of-1and another with an mtime so far in the future that it overflows into a large negative number if the Rust code is compiled for thei686-unknown-linux-musltarget.The current design of the API encourages users to assume that sources of
SystemTimevalues use unsigned values internally (ie. that modifications to the system clock are the only source ofErrwith valid input), and, further more, in the bug I reported forserde_json, it led @lambda to the incorrect conclusion that there is currently no way to losslessly serialize and deserializeSystemTimevalues.If nothing else, the documentation should be improved in the following ways:
The documentation for both
UNIX_EPOCHdefinitions should explicitly mention that the underlying OS may return validSystemTimevalues prior to the epoch and code should be prepared for such an eventuality. (Perhaps with the concrete example ofext3filesystems allowing negativemtimevalues to be stored)The documentation for
SystemTime::duration_sinceshould use "may returnSystemTimeError" rather than "may fail" to avoid the connotation that anErrresult is erroneous in any sense other than being an "until" rather than a "since".That is, "failure" connotes a response that should prompt either retry or error-propagation, while it is not necessary to re-run the operation with the arguments reversed in order to extract the
Durationand, depending on the input, nothing may be wrong at all. (ie. theResultdiscriminant is semantically equivalent to a sign bit in this case.)"and the error contains how far from self the time is" is too easy to overlook, given how important it is for robust software to handle this eventuality. It should explicitly mention
SystemTimeError::durationto make it more eye-catching.Perhaps "Returns an Err if
earlieris later thanself. ADurationrepresenting how farselfis beforeearliercan be retrieved via theduration()function on the resultingSystemTimeError.There are probably other ways to better tune the writing to guard against human error, but I don't want to be here all night trying to perfect this when you might think of the same improvements immediately, simply by virtue of having a different perspective.