Repository navigation
fix(json2): keep numeric conversion independent of host locale - #7215
simonbronk wants to merge 2 commits into
Conversation
|
Full CI on 8706b82 (run 39052899521): every test passes on both lanes, and The Linux runner has no decimal-comma locale, so the test exits 77 with "No decimal-comma numeric locale is installed". On Linux, the guard only accepts skips that report a device limitation. Could you make the test cover Linux without an installed locale? For example, build the numeric facet in the test, or set |
|
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.
|
|
I checked the Linux skip path in |
jotitadev
left a comment
There was a problem hiding this comment.
Linux run of guest_json_numeric_locale with a decimal-comma locale, since the Linux CI job skips it (no such locale installed) and the description has no Linux run.
Machine: Linux x86_64, i7-8700, Ubuntu 26.04, GCC 15.2, Release, BUILD_TESTING=ON, full build.
Head 8706b8258 merged onto main 5de7a46e0 (clean merge).
The host has only C, C.utf8 and en_US.utf8. I generated de_DE.UTF-8 into a user directory without root (localedef -i de_DE -f UTF-8 <dir>/de_DE.UTF-8) and pointed glibc at it with LOCPATH=<dir>.
- With the PR and
LOCPATHset:guest_jsonandguest_json_numeric_localepass, 5 of 5 runs. The test reportsJSON numeric locale: de_DE.UTF-8. - With the PR and no
LOCPATH:guest_json_numeric_localeexits 77 and CTest shows it as skipped, as in CI. - Test file and its registration only, on
main, withLOCPATHset:guest_json_numeric_localefails with the bug described above, e.g.Decimal-point serialization failed: 1,5,Decimal-point parsing failed: 1.5andNested numeric serialization failed: {"value":1,"list":[2,-0]}. So the test covers the change on Linux too. - Non-GPU suite (
ctest -E "^agc|gpu|vulkan|video_out", noLOCPATH): 698/698, the 697 tests ofmainplusguest_json_numeric_localecounted as passed with its skip.
On the CI skip: on glibc a test-local locale works without installing one system-wide, as long as localedef and the locale sources (/usr/share/i18n/locales) are present. I did not check whether the GitHub runner image has them.
Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.
|
Thanks for running this with a generated |
|
Windows runtime check. Windows 11 Pro 10.0.26200, Ryzen 7 7800X3D, WinLibs GCC 15.2.0 r7 (UCRT), Release. This PR's head
AI-assisted: yes (Claude Code). |
|
Thanks for the Linux locale run and negative control. Generating |
Prevent decimal-comma LC_NUMERIC from emitting invalid JSON and silently truncating decimal-point input. Keep 17-digit output, signed underflow and overflow rejection without changing the process locale. AI-assisted: yes (OpenAI Codex). Windows WinLibs GCC15.2 Release: original shared Json2 fails the final locale regression; fixed guest_json and guest_json_numeric_locale pass (2/2, no skips). Final conventions pass. Only relevant runtime/test targets built; full build/full suite and game/console/Linux/macOS behavior unverified. No performance or coverage increase claimed.
8706b82 to
2b991e8
Compare
# Conflicts: # core/libs/tests/GuestJson.cpp
|
Full CI on our merge-main head 0b51b5b (run 39383582595): every test passes, but the Linux job still fails at The check is deliberate. A skip there means Linux CI never runs the case, and we won't add a "no locale" category to the guard. The Linux runner has no decimal-comma locale and we can't install one on it, so the test has to bring its own. This works without root and without any system locale sources. Ship two small data files next to the test:
Then build the locale in the build tree as a CTest fixture (Linux only), point the test at it with if(NOT WIN32)
set(numeric_locale_dir ${CMAKE_CURRENT_BINARY_DIR}/numeric-locale)
add_test(NAME guest_json_numeric_locale_setup COMMAND sh -c
"rm -rf '${numeric_locale_dir}' && mkdir -p '${numeric_locale_dir}' && I18NPATH='${numeric_locale_dir}' localedef -c -f '<data>/comma.charmap' -i '<data>/comma.locale' '${numeric_locale_dir}/xx_COMMA'; test -f '${numeric_locale_dir}/xx_COMMA/LC_NUMERIC'")
set_tests_properties(guest_json_numeric_locale_setup PROPERTIES FIXTURES_SETUP json_numeric_locale)
set_tests_properties(guest_json_numeric_locale PROPERTIES FIXTURES_REQUIRED json_numeric_locale
ENVIRONMENT "LOCPATH=${numeric_locale_dir}")
endif()
Checked on glibc with every system i18n path hidden: Please also keep |
What
With a decimal-comma host
LC_NUMERIC, Json2 emits invalid JSON (1.5becomes1,5) and silently reads JSON1.5as1. In the regression,{"value":1.5,"list":[2.5,-0.125]}becomes{"value":1,"list":[2,-0]}after parsing and serializing.Use
to_charswith the existing 17 significant digits for output andfrom_charsfor input. On an out-of-range input only, use a stream explicitly imbued with the classic locale to retain the prior underflow-to-signed-zero behavior and overflow exception. Do not change the process locale. The numeric grammar still rejects a decimal comma.RFC 8259 section 6 requires
.as the decimal point. The C++ numeric conversion specification specifies C-locale formatting. Console rounding and formatting remain unmeasured and are recorded in Json2 debt. No title unblock, percentage increase or performance gain is claimed.Tested
Windows, WinLibs MinGW-w64 GCC 15.2.0r7 UCRT, Release, actual shared
libSceJson2.prxand its CMake test executable; base66702f1cffd3646f2d674ed093fde0a85a70761f:guest_jsonpasses, newguest_json_numeric_localefails. The output records invalid decimal commas, truncated input values, and the corrupted nested document above.aa92ad8072ff2efc82851c64fc6c95c6af31bded: both CTests pass, no skips (2/2, 0.43 s). The comma locale selected isde_DE.UTF-8.check_conventions.pyagainst the pinned base andgit diff --checkpass for the final committed change.pr_overlap.pycannot execute becauseghis unavailable; live connector searches found no open Json2 numeric-locale fix. feat(libSceJson2): implement Value::setTypeMismatchHandler and store the element access failure and special float handlers #6126 adds callbacks (including special-float handling), fix(json2): release partial copies on allocation failure #6593 handles allocation failures, and fix: complete compatibility gaps #6447 is a broader compatibility batch; those address different behavior but may require textual reconciliation.AI assistance: OpenAI Codex. Evidence is attached below.
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)json-numeric-locale-evidence-20261017.zip