Repository navigation
feat(libSceIcu): implement property and case exports from ICU tables - #13578
Open
KernelPanicNT wants to merge 5 commits into
Open
KernelPanicNT wants to merge 5 commits into
KernelPanicNT wants to merge 5 commits into
Conversation
Add ICU 78.1 (release-78.1, 049e0d6a4206) as the 3rdparty/icu submodule and build the icu4c common sources these exports reach as a static library with the compiled-in property, case, bidi, property-name and NFC tables, stub data and file I/O disabled. Implement 49 libSceIcu exports on top of it: character categories and classifiers, numeric, age, combining class, bidi and mirroring queries, property and value names, binary and enumerated property values, simple case folding, string case mapping and case-insensitive comparison. Properties whose ICU data lives in data files (emoji, NFKC, NFKC_Casefold, Indic categories, vertical orientation) and string case mapping in the default locale throw NotImplemented_nid_no_patch.
12 tasks done
Progress report (0263899 → 5d01281)📈 System libraries: 26.81% (+0.12%, +44 implemented functions) ✅ 44 implemented
Changed libraries (implemented exports)
|
|
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.
|
Contributor
|
Tested on Windows, since the description lists MinGW as untested. Head
Not tested: macOS, or with a title. 🤖 Addressed by Claude Code |
12 tasks done
12 tasks done
12 tasks done
This was referenced Oct 31, 2026
12 tasks done
12 tasks done
12 tasks done
12 tasks done
12 tasks done
This was referenced Nov 1, 2026
12 tasks done
CnRJay
reviewed
Nov 1, 2026
Contributor
There was a problem hiding this comment.
Correction (2026-11-01): the title-ID/log demand below is not a CONTRIBUTING requirement; Unblocks: none is allowed. The technical points below are unaffected.
The source pinning to ICU 78.1 and the Unicode 17.0 consistency with #13457 is good. Two requests.
-
There is no title ID and no log line showing a title reaching any of the property/case exports on main ("Unblocks: none"). 14 files and +843 lines of ICU table-driven behaviour need a consumer named; which title calls these, and with which arguments?
-
Table-derived exports are only as good as their coverage of the full code point range. Does the test sweep every code point
0..0x10FFFFagainst an independent oracle (ICU itself), including surrogates and out-of-range inputs, or only spot values? A boundary-only test would not catch a table offset error in the middle of the range. Please state what the oracle is.
12 tasks done
12 tasks done
added 3 commits
November 4, 2026 14:06
Main's Iterator.hpp declares the guest-ABI UCharIterator and UCharIteratorOrigin in the global namespace, and the umbrella libSceIcu.hpp pulls it in. CaseMapping.cpp also includes ICU's ustring.h, which defines ICU's own UCharIterator, so the merged tree failed to compile. Properties.cpp and CaseMapping.cpp now include only their own export headers.
guest_icu_unicode_data reads UnicodeData.txt from the pinned ICU submodule at test time (path passed by CMake) and fails if it is missing or malformed. For every code point 0..10FFFF it compares u_charType, u_getCombiningClass, u_charDigitValue and u_charDirection with the file, expanding First/Last ranges and treating unlisted code points as Cn, class 0 and no digit. u_charDirection is skipped for unlisted code points because the submodule has no DerivedBidiClass.txt. u_enumCharTypes ranges are checked against the same table, and INT32_MIN, -1, 0x110000 and INT32_MAX against ICU's trie error values.
Contributor
Author
|
Thanks a lot @CnRJay, the point about coverage is a good one: the old tests only checked spot values, so a table offset in the middle of the range would have slipped through. And thanks for the correction on point 1. What changed:
@CodingBreadd, thank you for the MinGW run. About ADVAPI32: the registry calls are in |
Contributor
✅ AMD GPU test: passedPR head
Unstable on main (4, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (13, not counted against this PR)Validation IDs already present on main (8 IDs in 809 tests)Skipped (62)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Source of the behaviour
ICU 78.1, tag
release-78.1, commit049e0d6a4206. The library already follows this release (see the version and escape entries inlibSceIcu.md); its Unicode version is 17.0 (uchar.h#L64), the value #13457 returns fromu_getUnicodeVersion.uchar.h,ustring.h,ucasemap_imp.h#L57(u_caseInsensitivePrefixMatch).uchar.cpp(u_charTypeL50,u_enumCharTypesL77,u_getNumericValueL356,u_charAgeL524) overuchar_props_data.h; case dataucase.cpp#L1625-L1656overucase_props_data.h; bidiubidi_props.cpp#L237-L255overubidi_props_data.h; combining classnormalizer2.cpp#L551over the built-in NFC data (normalizer2impl.h#L40,norm2_nfc_data.h); namespropname.cpp#L290-L322overpropname_data.h.uprops.cpp#L399-L485and#L714-L789. The source of each property, used to decide which ones need a data file, isuprops_getSource; the data-file loaders areulayout_load(InPC, InSC, vo),emojiprops.cppandloadednormalizer2impl.cpp(NFKC, NFKC_CF).ustrcase.cpp(argument checks L1356-L1377,u_strFoldCaseL1423,_cmpFold, which reads both strings without checks, L1481, public wrappers L1854-L1905); the locale stepustrcase_locale.cpp#L31-L65, where a null locale meansuloc_getDefault().AnyPS5 lines that change (at
b11c7bf84f5d):Export.cpp#L92-L413and their declarations fromIdentified.hpp#L22-L79.CMakeLists.txt#L662(newinclude(cmake/Icu.cmake)after Opus),.gitmodules#L78,libSceIcu/CMakeLists.txt#L11and#L164,libSceIcu.hpp#L17.Properties.cppandCaseMapping.cppinclude only their own headers, because the guestUCharIteratorthatIterator.hppdeclares clashes with ICU's.libSceIcu.md#L54(new entry before it) and#L56-L57(93 stubs become 44, 73 named stubs become 24).Space.cpp.What this adds
ICU becomes a submodule at
3rdparty/icu, pinned to the commit above (shallow = true, since the repository history is large).cmake/Icu.cmakebuildsicu_common, a static library made of the 53icu4c/source/commonfiles these exports reach plusstubdata/stubdata.cpp, withUCONFIG_NO_FILE_IO=1. The property, case, bidi, name and NFC tables are compiled-in*_data.hheaders, so no.datfile, data directory or host ICU is involved, and nothing is read from the host file system. ICU keeps its versioned symbol names (u_charType_78), is built with hidden visibility and is linked with--exclude-libslike Opus, solibSceIcu.prxexports none of it (nm -Dshows 0 ICU symbols). Windows adds 19 files (wintz.cppand the locale and resource-bundle code it andputil.cppreach only on Win32).49 exports call ICU and keep the guest ABI the library already uses:
UChar32isint32_t,UBoolone byte, enumsint32_t,UErrorCode*anint32_t*,UCharchar16_t:u_charType,u_enumCharTypes,u_islower,u_isupper,u_istitle,u_isdigit,u_isxdigit,u_isalpha,u_isalnum,u_isdefined,u_isbase,u_iscntrl,u_ispunct,u_isgraph,u_isprint,u_isIDStart,u_isIDPart,u_isIDIgnorable,u_isJavaIDStart,u_isJavaIDPart,u_isUAlphabetic,u_isULowercase,u_isUUppercaseu_charDigitValue,u_digit,u_getNumericValue,u_charAge,u_getCombiningClass,u_charDirection,u_isMirrored,u_charMirror,u_getBidiPairedBracket,u_foldCaseu_getPropertyEnum,u_getPropertyName,u_getPropertyValueEnum,u_getPropertyValueName,u_getIntPropertyMinValue,u_getIntPropertyMaxValue,u_hasBinaryProperty,u_getIntPropertyValueu_strFoldCase,u_strToLower,u_strToUpper,u_strCaseCompare,u_strcasecmp,u_strncasecmp,u_memcasecmp,u_caseInsensitivePrefixMatchBehaviour is ICU's for every input, with these exceptions:
u_hasBinaryPropertyandu_getIntPropertyValuethrowNotImplemented_nid_no_patchfor properties whose source is a data file (emoji properties, NFKD/NFKC_Inert, NFKD/NFKC_Quick_Check, Changes_When_NFKC_Casefolded, Indic_Positional_Category, Indic_Syllabic_Category, Vertical_Orientation), andu_getIntPropertyMaxValuefor the last three. Without the file ICU would answer 0 or false, which would be made up.u_strToLowerandu_strToUpperthrowNotImplemented_nid_no_patchfor a null locale when ICU would map the string, because the result then depends on the device's default locale. An empty locale is root, as in ICU, and failed input codes and invalid arguments return as in ICU. A named locale goes throughucase_getCaseLocaleandustrcase_mapWithOverlap, which is whatu_strToLowerdoes for a non-null locale; calling them directly keepsuloc_getDefaultand the locale and resource-bundle code out of the Linux build.UErrorCode*in the string mappers and the prefix match, a null string with a nonzero length in the comparisons, a null second match length) or receives a length below -1, which ICU turns into pointer arithmetic before the string, the export throwsstd::invalid_argument.u_enumCharTypesforwards ICU's callback to the guest function through a System V ABI trampoline, so a guest callback works on Windows.These five exports therefore still count as stubs; the other 44 no longer reach
NotImplemented_nid_no_patch.Why ICU's tables instead of hand-written sets
#12630 implemented
u_isspaceand four similar classifiers with fixed ranges, and those stay as they are. That works for a few short sets, but these exports need the General_Category, numeric value, age, case mappings and folding, bidi class, mirroring and paired brackets, combining class and the property and value names for all 1,114,112 code points, about 100 properties in all. Hand-transcribing them would mean several hundred kilobytes of tables that nobody can review against the Unicode Character Database, and they would drift from the 17.0 data that ICU 78.1 ships. The submodule uses the tables ICU generates from that data, at the release the library already follows, compiled from source as AGENTS.md asks for third-party code.Excluded exports
u_getUnicodeVersionandu_strCompareIter: feat(libSceIcu): implement u_strCompareIter and u_getUnicodeVersion #13457.u_isISOControl,u_forDigitand the other 18 exports feat(libSceIcu): implement UTF-32, Java UTF-8, comparison and configuration exports #12320 implements.u_charName,u_charFromName,u_enumCharNames: needunames.icu.u_getFC_NFKC_Closure: needs NFKC data.u_getBinaryPropertySet,u_getIntPropertyMap: they return ICUUSetandUCPMapobjects that onlyuset_*anducpmap_*functions can read, and this library exports none of those.locale_getKeywordsStartstubs: not property or case data.Overlap (
tools/pr_overlap.py --branch feat/icu-submodule-properties)u_charDigitValueandu_digitwith its own decimal-range table. Here both call ICU, andu_charDigitValueis checked againstUnicodeData.txtfor every code point. Whichever PR merges second has to drop the other's version.Export.cpp,Identified.hpporlibSceIcu.hpp) and feat(libSceIcu): implement locale_getKeywordsStart #14050, feat(libSceIcu): implement 481 data-free ICU 68 exports #14144, feat(libSceIcu): implement 458 more data-free ICU 68 exports #14340, feat(libSceIcu): implement the string trie builders, data swappers and the ICU service cache #14443, feat(icu): implement converter name and signature helpers #12738, feat(libSceIcu): implement UTF-16 comparisons #10963–feat(libSceIcu): implement UTF-16 search and memory primitives #10967 (libSceIcu/CMakeLists.txt, debt file): other ICU exports. None implements any of these 49; only the appended lines conflict..gitmodules) and feat(libs): let relinked Linux games opt into mimalloc #14264, feat(agc): look up table images at run time through a device image table on the GPU heap (APS5_GPU_PATH) #7642, perf(agc): GPU descriptor heap on VK_EXT_descriptor_heap (APS5_GPU_HEAP) #9508, fix(tools): merge only the pull requests that share files or exports with the branch in pr_overlap #9539, feat(agc): serve lane-keyed, pointer and runtime image-table columns #10193, feat(agc): resolve table samplers through device image keys #13928, feat(agc): resolve runtime sampler addresses through device image keys #13929, docs(debt): separate entries with blank lines and drop derived export totals #14212 (CMakeLists.txt): they add other submodules or libraries, none of them ICU; only neighbouring lines conflict.Tested
Linux (GitHub Codespace, AMD EPYC 7763, 4 cores, Ubuntu 24.04, g++ 14, Ninja, Release), on
mainatb11c7bf84f5dwith the submodule checked out bygit submodule update --init --depth 1 3rdparty/icu(commit049e0d6a4206):ctest -R "^guest_icu_": 18/18 passed, the 15 ICU tests onmainand the newguest_icu_properties,guest_icu_case_mappingandguest_icu_unicode_data.guest_icu_unicode_datausesicu4c/source/data/unidata/UnicodeData.txtfrom the submodule as the oracle. CMake passes the path in, and the test reads the file when it runs and fails if the file is missing or malformed. For all 1,114,112 code points (299,382 listed, withFirst/Lastranges expanded) it comparesu_charType,u_getCombiningClass,u_charDigitValueandu_charDirectionwith the file, and theu_enumCharTypesranges with the same table. Unlisted code points must be Cn, class 0 and no digit. Bidi is not compared for them, because the submodule has noDerivedBidiClass.txt.INT32_MIN, -1, 0x110000 andINT32_MAXmust return Cn, 0, -1 and L, which is what ICU returns outside its tries.guest_icu_propertiesandguest_icu_case_mappingcheck values from the Unicode Character Database (for example U+20C1 SAUDI RIYAL SIGN, new in 17.0: category Sc and age 17.0), locale mappings (tr,az,lt,el, final sigma), preflighting, in-place mapping, error codes and the cases that throw.guest_icu_unicode_data: changing a single code point in the middle of a range (U+7A00 foru_charType, U+0F72 foru_getCombiningClass, U+0AE9 foru_charDigitValue, U+05D3 foru_charDirection, the surrogate U+DB80 foru_charType) gives exactly one mismatch each, whileguest_icu_propertiesandguest_icu_case_mappingstill pass with it. Shifting one block for each export (U+8000 foru_charType, U+1DC0 foru_getCombiningClass, U+1D7CE foru_charDigitValue, U+10900 foru_charDirection) gives 182, 19, 50 and 204 mismatches. Mapping out-of-range input to U+0041 gives 4. The 49 bodies replaced byNotImplemented_nid_no_patch(__func__)fail withu_charType_nid_postfix not implemented; on the earlier head the same control failedguest_icu_propertiesandguest_icu_case_mapping(u_strToUpper_nid_postfix not implemented). WithUnicodeData.txtmoved away the test fails withcannot open, and with the file truncated it fails on the cut line. With the change restored all pass.icu_commoncold build without ccache: 19 s on 4 cores (54 files). The depth-1 checkout is 329 MB of working tree plus 95 MB under.git/modules.tools/check_conventions.py --base origin/mainandtools/check_test_registration.pypass.tools/progress.py --compare: libSceIcu 75/2661 → 119/2661, +44 implemented, 5 still counted as stubs (u_hasBinaryProperty,u_getIntPropertyValue,u_getIntPropertyMaxValue,u_strToLower,u_strToUpper).d8ee336with WinLibs GCC 15.2.0 (MinGW, UCRT); the 8 ICU tests of that head passed (comment).Not tested: this head on Windows (MinGW), including
guest_icu_unicode_data, and macOS. The 19 Windows-only sources were first found by forcing every ICU symbol thatputil.cpp,wintz.cpp,locmap.cpp,udata.cppandumapfile.cppreference to be undefined in a Linux link and reading the linker map, not by building with MinGW. Not compared with a console.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)