Repository navigation
feat(libc): format time_put<char>::do_put as the console's libc does - #11044
1 commit merged into
Conversation
Progress report (0119103 → 3338ffe)➖ System libraries: 49.65% (+0.0%, +1 implemented, -1 removed functions) ✅ 1 implemented
🗑️ 1 removed
Changed libraries (implemented exports)
|
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head 186fbf3. The guest streambuf offsets and vtable slot are statically asserted, the formatter follows the observed leading-sentinel and overflow behavior, and failed iterators still receive every output character. The focused test covers formatting, overflow rejection, null put-next, and null iterator buffer. The author also reports 8,176 comparisons against the title SDK implementation with only the documented timezone difference. Hosted conventions, intake, eligibility, relinker, sanitizer, and register-context checks pass; product jobs are skipped by path policy. I reviewed the implementation and test but did not run the full local build here.
|
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) |
MarcZX10
left a comment
There was a problem hiding this comment.
I manually checked the exact heads of #9456 (159a6f44078e56a677b354da7e0d88688583a7f7) and #9468 (ef2f603255e817198618bd38a1fddd803b115ce5) against this PR (186fbf3b8f92f8c649bd869180af12887b4a8922). git merge-tree reports a content conflict only in core/libs/tests/GuestLocale.cpp; all three branches close CheckFacetGetcat and add a separate check at that insertion point. The implementation additions appear independent, and a combined result should retain one closing brace plus all three checks. Per CONTRIBUTING.md, please coordinate merge order with the other authors and declare the agreed dependency on the checklist (Depends on: #N) before this PR is considered ready. This is a merge-order issue, not a finding that the time_put implementation overlaps semantically with time_get or num_get.
The time_put facet's do_put formats % with the modifier and conversion through strftime and writes the result through the output iterator's stream buffer, following the SDK libc.prx: it stores into the put area while room remains, calls overflow otherwise, and marks the iterator failed when overflow returns eof or the buffer is null.
186fbf3 to
3338ffe
Compare
|
Thanks. Instead of a merge order, I moved this PR's test off the lines #9456 and #9468 change: The code is unchanged: the commit only moves 45 test lines, and it is now rebased onto |
|
Windows run, which the description lists as not tested. WinLibs GCC 15.2.0, Release, Windows 11 10.0.26300. I tested this branch as the parent of #11054, with #11054's head
AI-assisted: yes (Claude Code); results above were run on this machine. |
|
A Windows rerun of the new head
AI-assisted: yes (Claude Code) |
d6ac53b
What
time_put<char>::do_putthrew (TimePutMethodUnsupported), so a title that formats astd::tmthrough a stream (std::put_time,time_put::put) stopped there. It now does what the SDKlibc.prxthat PPSA06416 ships insce_module/(title SDK 7.00) does, read from its disassembly (REVERSE_ENGINEERING, source 2). The module exports it as+i81FtUCarA(0x9aa00):!%followed by the modifier and the conversion, or the conversion alone when the modifier is 0, and runs_Strftimeon it, the same functionstrftimecalls. It starts with a 16-byte buffer and doubles it until the result fits; the leading!tells an empty result from one that did not fit, and is dropped.sputcdoes. When the stream buffer's put pointer (*(buffer + 0x40)) is set and(int)(*(buffer + 0x60) - next) > 0, it stores the character and advances the pointer. Otherwise it callsoverflow, vtable slot 4, and marks the iterator failed when that returnseof. A null buffer marks the iterator failed. A failed iterator still offers the rest of the characters, and the stream and fill arguments are not read.The facet gets a vtable with its single
do_putslot instead of eight throwing slots. The formatting goes through this library'sstrftime, so its gaps carry over.GuestLocale.hppdescribes the three guest types this touches: the stream buffer's put pointers, itsoverflowslot and the 16-byte iterator, which arrives in two registers and is returned in RAX:RDX.The debt entry for these facets now says
time_putformats throughstrftime. To keep it under 1000 characters, thestd::clog,std::wclogand facet id sizes that shared it move to an entry of their own, unchanged.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, compared against this library's facet with the same fake stream buffer. That covers every conversion byte 0-255 with no modifier and withE,O,#,x,Y, 0x80 and%, each with room in the buffer, 2 bytes of room withoverflowrefusing every character or every other one, a null put pointer and a null buffer: 8,176 calls. Stored bytes, the order ofoverflowcalls, the failed flag and the returned iterator match in all of them except the 12%Zcalls. There the module prints the empty zone name of a process withoutTZand this library prints the host's, as the existingtznamedebt entry records. A format that ends afterEorOis left out: the console crashes there andstrftimethrows.guest_localegainsCheckTimePut: a conversion that fits, one that runs intooverflowpart way (and needs a 32-byte buffer),EandO, a 0 conversion, an unknown conversion,overflowrefusing (all 8 characters are still offered), a null put pointer and a null buffer. Emitting the!as well fails it.ctest -R '^(guest_locale|guest_strftime|patched_libraries|conventions_unknown_nids)$': 4/4 pass.tools/progress.py: libc +1 implemented (TimePutDoPut),TimePutMethodUnsupportedremoved.python3 tools/check_conventions.py --base origin/main: passed.python3 tools/pr_overlap.py: no conflicting lines or same exports; none of the 39 pull requests it could not compare touchestime_put, the stream buffer types or this debt entry.Windows: not run by me.
guest_localepassed in @ycbuzi's run of186fbf3b8fand in @wachi35676's run of this head3338ffefff, as the parent of #11054.Not tested: macOS, a title that calls
put_time.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)