Repository navigation
fix(libc): compute strftime week numbers as the console's libc does - #11004
Conversation
%G, %g and %V no longer throw for a negative tm_yday, a tm_yday past the year or a tm_wday below -6. They follow the year and week rule of the SDK libc.prx, read from its disassembly and checked by running it, with its leap-year test and 32-bit arithmetic. %U and %W also wrap in 32 bits as that module does.
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the %G/%g/%V implementation and the cited console/SDK evidence. The new tests cover the reported negative and oversized fields, inconsistent year fields, and 32-bit boundary cases; the PR reports matching behavior against the SDK module over 675,076 inputs and six focused host tests. I independently verified the diff, conventions, and a local C++20 syntax check of Time.cpp. Hosted eligibility/conventions/report and non-product jobs pass; the PR does not claim a hosted full product build. Windows/macOS product tests and console measurements for untested cases remain outstanding as documented.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
A Windows run of this head: Windows 11 Home 10.0.26200, WinLibs GCC 15.2.0 r7 (MinGW-w64 UCRT, posix-seh), CMake 4.3.1, Ninja 1.13.2, Release.
Not run: Linux, macOS, the comparison against the title's AI-assisted: yes (Claude Code) |
|
First Windows run of head
AI-assisted: yes (Claude Code); results above were run on this machine. |
|
@verusferro Thanks for the careful work and reference-module evidence in this PR! I maintain #10895, which fixes partial output from Would you be happy with #10895 landing first, followed by rebasing #11004 onto the updated main and recording When rebasing, we would need to preserve the templated Thanks for coordinating! Please let me know if you would prefer a different order; the final merge decision remains with the maintainers. |
|
@StoneDC Agreed: #10895 first. I've set I tried that rebase locally, replaying
Both test sets merge without conflicts. On that combined tree, |
What
strftime's%G,%gand%Vthrew for a negativetm_yday, atm_ydaypast the year and atm_wdaybelow -6, because no rule fitted the console's results. They now follow the rule of the SDKlibc.prxthat PPSA06416 ships insce_module/(title SDK 7.00), read from its disassembly (REVERSE_ENGINEERING, source 2).strftime(Av3zjWi64Kw, 0x6d720) passes the C-locale time table to_Strftime(0x3ce10), whose conversion switch is at 0x3d2e0:%G0x3d41d,%V0x3d6bb,%g0x3d9a6,%U0x3d697,%W0x3d7fa.The rule, all in 32-bit arithmetic with C remainders:
1 January falls on
c = (tm_wday - tm_yday + 371) % 7and the next one onn = (c + 365 + leap) % 7.When
nis 2, 3 or 4 andtm_yday - leap >= 366 - n, the date is week 1 of the next year.Otherwise
week = (tm_yday - (tm_wday + 6) % 7 + 7) / 7, plus 1 whencis 2, 3 or 4. A negative week is week 1 of the next year. Week 0 belongs to the previous year:%Gprints that year, and%Vprints its last week computed from its length, or 00 when its last days are week 1.The leap test (0x3e830) is not Gregorian. A year divisible by 4 and not by 100 is leap, and of the years divisible by 100 only those with
tm_year / 400equal to 100 or -300:For every real date from year 1 to 9999 this still gives ISO 8601, so valid dates print as before. Fields that are each in range but disagree with each other can change through the leap test:
tm_year100,tm_wday1,tm_yday362 prints2001|01|01as the module does, where the FreeBSD loop gave2000|00|53.tm_year + 1and the other sums wrap at 32 bits as in the module; the code reproduces that throughuint32_twithout signed overflow.%Uand%Wkeep their formula,(tm_yday + 7 - (tm_wday + shift) % 7) / 7, but now also wrap at 32 bits as the module does, so a field nearINT_MAXmatches it. The debt entry now records where the rule comes from and what is not measured on a console.Tested
Ubuntu 24.04 (WSL2) x86-64, GCC 13.3, Release, full build with
BUILD_TESTING=ON:libc.prx, converted by the relinker and loaded in a host process withProcessParametersFixture, returns all 82 console results inguest_strftime(the zone conversions left out) and the console values the old debt entry listed:%V01, 00, 00 and 52 fortm_yday-25, -13, -12 and -1; week 01 of 2027; 59; 53; 04.strftimeand the module's print the same%G|%g|%V|%U|%W|%Y|%yfor all 675,076 inputs of three random runs: fields up toINT_MINandINT_MAX, everytm_wdayfrom -14 to 14 around year ends, and the years the leap test treats differently.guest_strftimegains the console's out-of-range cases, rows for each branch, the wrap atINT_MAXandINT_MIN, and two%U/%Wrows. The old code throws on the first ISO row and prints78|78fortm_ydayINT_MAX. A Gregorian leap test fails thetm_year100 row.ctest -R '^(guest_strftime|guest_time_conversion|guest_time_extras|guest_wide_formatting|patched_libraries|conventions_unknown_nids)$': 6/6 pass.tools/progress.py: libc 1344 → 1345 of 1355 (libc_strftimeimplemented).python3 tools/check_conventions.py --base origin/main: passed.python3 tools/pr_overlap.py: no conflicting lines or same exports. Of the pull requests it could not compare, feat(libc): add the std::__sce_v2 thread start, timed wait, clock and processor count calls #9447 changes_Xtime_get_ticksinTime.cppand fix(libc): write a 36-byte struct tm as the console does #10409 theGuestTmlayout; neither touches the week code. The rest share onlydocs/dev/debt/libc.md, in other entries.Windows: not run by me. This head passed in runs by @ycbuzi and @wachi35676; the second also compared every date from year 1 to 9999 against ISO 8601 with no mismatch.
Not tested: macOS, a console run of the cases the old debt entry did not list.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)