[libc] rewrite mktime_internal to use `time_t` internally instead of `int64_t`

[nickdesaulniers]

See also: #126947. The current implementation works for targets where sizeof(time_t) == sizeof(int64_t), but we shouldn't make that assumption. Instead of using int64_t perhaps we can use time_t.

[llvmbot]

@llvm/issue-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

See also: #126947. The current implementation works for targets where `sizeof(time_t) == sizeof(int64_t)`, but we shouldn't make that assumption. Instead of using `int64_t` perhaps we can use `time_t`.

[ayokunle321]

@nickdesaulniers Hey! Just starting with libc and wanted to get my hands dirty. I just looked though the mtkime.cpp file and it already returns a time_t so I'm wondering if you mean to change all the internal expressions and arguments that are of the int64_t type to the time_t type i.e. int64_t month = tm_out->tm_mon; to time_t month = tm_out->tm_mon;.

[michaelrj-google]

Hi @ayokunle321,

If you're just cleaning up this issue then I'd say just change the return type for mktime_internal to be time_t, but leave the internals alone.

If you're interested in a deeper rewrite of mktime then I'd be happy to explain what would be necessary for that. mktime was written before our current coding standards and so it has some behaviors we now don't allow, specifically setting errno in internal functions (e.g. out_of_range() and invalid_value()).

Either way we'd be happy to review a PR, and if you're interested I'll assign this issue to you.

[ayokunle321]

@michaelrj-google Thanks for the reply! So I believe it already returns a time_t so that's fine

I'm quite interested in the rewrite though and would like an explanation of what to do exactly.

[michaelrj-google]

The public entrypoint does return a time_t, but the function mktime_internal defined in time_utils.cpp returns an int64_t. That's also the function that needs to get rewritten.
For the moment here's what I'd like to see in a PR:

1. change the return type of mktime_internal to time_t, and remove any now-unnecessary static_cast<time_t>s.
2. update mktime_internal to not set errno, instead the error state should be detected in mktime and errno should be set there.

[wsehjk]

Hi, can you assign me to work on this?

[michaelrj-google]

Sure, I've assigned you.