Repository navigation
Delete FString and coreclr PAL's wchar.cpp - #137164
Merged
Merged
Conversation
Follow-up to dotnet#136973. * Replace the FString wrapper with direct minipal UTF-8 conversions in SString, CQuickBytes and JitHost. * Delete pal/src/cruntime/wchar.cpp. Its users within the PAL are inlined; the u16_strtoul, u16_strtoui64 and u16_strtod implementations move to coreclr minipal (dn-u16). * Add u16_stricmp and u16_strnicmp to dn-u16 and use them in place of _wcsicmp/_wcsnicmp. * Delete the corresponding PAL tests. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Azure Pipelines: Successfully started running 5 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
dotnet-policy-service
Bot
added
the
community-contribution
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new numeric parsers lack replacement coverage for deleted tests, leaving range checks, overflow, and incomplete-exponent handling unverified across return values, errno, and end pointers.
1 open issue
What changed in this PR
Continues #136973’s removal of CoreCLR’s Unicode wrapper layers by moving callers to minipal and deleting FString and the PAL’s wchar.cpp.
Changes:
- Replace
FStringconversions with direct minipal UTF-8 conversion calls. - Move numeric parsing into
dn-u16, add case-insensitive comparisons, and migrate callers. - Remove obsolete PAL declarations, build entries, and tests; update remaining tests to use
dn-u16.
| File | Description |
|---|---|
| src/coreclr/vm/jithost.cpp | Convert JIT configuration strings through minipal. |
| src/coreclr/vm/interoputil.cpp | Parse standard DISPIDs with u16_strtoul. |
| src/coreclr/vm/excep.cpp | Use u16_stricmp for configuration comparison. |
| src/coreclr/vm/eeconfig.cpp | Migrate configuration comparisons to dn-u16. |
| src/coreclr/vm/codeman.cpp | Migrate target OS comparisons to dn-u16. |
| src/coreclr/vm/classhash.cpp | Remove the obsolete fstring.h include. |
| src/coreclr/utilcode/sstring.cpp | Convert UTF-16 directly through minipal. |
| src/coreclr/utilcode/fstring.cpp | Delete the conversion wrapper implementation. |
| src/coreclr/utilcode/CMakeLists.txt | Remove fstring.cpp from compilation. |
| src/coreclr/tools/metainfo/mdobj.cpp | Migrate filename comparisons to dn-u16. |
| src/coreclr/pal/tests/palsuite/paltestlist.txt | Remove deleted wide-character test registrations. |
| src/coreclr/pal/tests/palsuite/miscellaneous/SetEnvironmentVariableW/test4/test4.cpp | Use dn-u16 comparison and length helpers. |
| src/coreclr/pal/tests/palsuite/miscellaneous/SetEnvironmentVariableW/test3/test3.cpp | Use dn-u16 string comparisons. |
| src/coreclr/pal/tests/palsuite/miscellaneous/SetEnvironmentVariableW/test1/test.cpp | Use dn-u16 string lengths. |
| src/coreclr/pal/tests/palsuite/miscellaneous/GetEnvironmentVariableW/test6/test6.cpp | Use dn-u16 comparison and length helpers. |
| src/coreclr/pal/tests/palsuite/miscellaneous/GetEnvironmentVariableW/test5/test5.cpp | Use dn-u16 comparison and length helpers. |
| src/coreclr/pal/tests/palsuite/miscellaneous/GetEnvironmentVariableW/test4/test.cpp | Use dn-u16 comparison and length helpers. |
| src/coreclr/pal/tests/palsuite/miscellaneous/GetEnvironmentStringsW/test1/test.cpp | Migrate environment-block searching to dn-u16. |
| src/coreclr/pal/tests/palsuite/miscellaneous/FormatMessageW/test6/test.cpp | Use u16_strlen for output validation. |
| src/coreclr/pal/tests/palsuite/miscellaneous/FormatMessageW/test5/test.cpp | Use u16_strlen for output validation. |
| src/coreclr/pal/tests/palsuite/miscellaneous/FormatMessageW/test4/test.cpp | Use u16_strlen for output validation. |
| src/coreclr/pal/tests/palsuite/miscellaneous/FormatMessageW/test3/test.cpp | Use u16_strlen for output validation. |
| src/coreclr/pal/tests/palsuite/miscellaneous/FormatMessageW/test2/test.cpp | Use u16_strlen for output validation. |
| src/coreclr/pal/tests/palsuite/miscellaneous/FormatMessageW/test1/test.cpp | Use u16_strlen for output validation. |
| src/coreclr/pal/tests/palsuite/compilableTests.txt | Remove deleted wide-character tests. |
| src/coreclr/pal/tests/palsuite/common/palsuite.h | Remove PAL string aliases and unused helper declaration. |
| src/coreclr/pal/tests/palsuite/common/palsuite.cpp | Migrate length lookup and delete unused wide-path helper. |
| src/coreclr/pal/tests/palsuite/CMakeLists.txt | Remove deleted test sources from compilation. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcstoul/test6/test6.cpp | Delete numeric-base parsing tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcstoul/test5/test5.cpp | Delete overflow and negative-input tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcstoul/test4/test4.cpp | Delete maximum-value parsing test. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcstoul/test3/test3.cpp | Delete invalid-input parsing test. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcstoul/test2/test2.cpp | Delete decimal parsing test. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcstoul/test1/test1.cpp | Delete base-four parsing test. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcstod/test2/test2.cpp | Delete floating-point overflow tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcstod/test1/test1.cpp | Delete floating-point parsing cases. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcsstr/test1/test1.cpp | Delete PAL substring-search tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcsrchr/test1/test1.cpp | Delete PAL reverse-search tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcsncpy/test1/test1.cpp | Delete PAL bounded-copy tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcsncmp/test1/test1.cpp | Delete PAL bounded-comparison tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcslen/test1/test1.cpp | Delete PAL string-length tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcscpy/test1/test1.cpp | Delete PAL string-copy test. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcscmp/test1/test1.cpp | Delete PAL string-comparison tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcschr/test1/test1.cpp | Delete PAL character-search tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/wcscat/test1/test1.cpp | Delete PAL concatenation tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/sscanf_s/sscanf_s.h | Use u16_strlen in scanning assertions. |
| src/coreclr/pal/tests/palsuite/c_runtime/_wtoi/test1/test1.cpp | Delete PAL integer-conversion tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/_wcsnicmp/test1/test1.cpp | Delete bounded case-insensitive comparison tests. |
| src/coreclr/pal/tests/palsuite/c_runtime/_wcslwr_s/test1/test1.cpp | Use u16_strlen in lowercase validation. |
| src/coreclr/pal/tests/palsuite/c_runtime/_wcsicmp/test1/test1.cpp | Delete case-insensitive comparison tests. |
| src/coreclr/pal/src/thread/thread.cpp | Use minipal lengths for thread descriptions. |
| src/coreclr/pal/src/objmgr/listedobjectmanager.cpp | Replace PAL length and comparison helpers. |
| src/coreclr/pal/src/misc/utils.cpp | Use minipal length lookup before conversion. |
| src/coreclr/pal/src/misc/fmtmessage.cpp | Replace PAL string helpers with direct operations. |
| src/coreclr/pal/src/misc/environ.cpp | Use minipal lengths for environment conversions. |
| src/coreclr/pal/src/misc/dbgmsg.cpp | Remove the obsolete runtime-header include. |
| src/coreclr/pal/src/loader/module.cpp | Use minipal lengths for module paths. |
| src/coreclr/pal/src/include/pal/palinternal.h | Include minipal string helpers. |
| src/coreclr/pal/src/include/pal/cruntime.h | Delete the obsolete runtime utility header. |
| src/coreclr/pal/src/include/pal/corunix.hpp | Use minipal length lookup in string storage. |
| src/coreclr/pal/src/eventprovider/lttngprovider/eventproviderhelpers.cpp | Calculate UTF-16 length locally. |
| src/coreclr/pal/src/debug/debug.cpp | Use minipal lengths in debug operations. |
| src/coreclr/pal/src/cruntime/wchar.cpp | Delete PAL wide-character implementations. |
| src/coreclr/pal/src/CMakeLists.txt | Remove wchar.cpp from compilation. |
| src/coreclr/pal/inc/pal.h | Remove deleted wide-character API declarations. |
| src/coreclr/minipal/Windows/dn-u16.cpp | Add CRT-backed case-insensitive comparisons. |
| src/coreclr/minipal/Unix/dn-u16.cpp | Implement comparisons and numeric parsing locally. |
| src/coreclr/minipal/dn-u16.h | Declare case-insensitive comparison helpers. |
| src/coreclr/inc/sstring.inl | Use dn-u16 in comparison verification assertions. |
| src/coreclr/inc/fstring.h | Delete the conversion wrapper declarations. |
| src/coreclr/inc/eventtracebase.h | Migrate trace parsing and provider comparisons. |
| src/coreclr/inc/corhlprpriv.h | Replace quick-buffer conversions with minipal calls. |
| src/coreclr/ildasm/dman.cpp | Migrate resource-name comparisons to dn-u16. |
| src/coreclr/ildasm/dasm.cpp | Migrate file-extension comparisons to dn-u16. |
| src/coreclr/ilasm/main.cpp | Migrate debug-option comparisons to dn-u16. |
| src/coreclr/dlls/mscordac/mscordac_unixexports.src | Remove deleted PAL string exports. |
Review effort: Balanced
* u16_strtoul truncates negative values instead of saturating them, and u16_strtod sets the end pointer past an incomplete exponent, like the deleted PAL wcstoul and wcstod did. * u16_strnicmp lowers characters with towlower, like the deleted PAL _wcsnicmp did. * Keep calling _wtoi in the Windows-only COM interop code. * Port the deleted PAL wide string tests to src/tests/minipal. Co-authored-by: Cursor <cursoragent@cursor.com>
jkotas
reviewed
Nov 8, 2026
* Use minipal_u16_strlen in the LTTng event provider helpers. * Don't route u16_strcmp and u16_stricmp through the bounded variants. * Pass all leading ASCII characters to strtod, like to strtoul and strtoull, which makes u16_strtod match the Windows wcstod. * Fix the u16_strtoul comment. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unchecked UTF-16 allocation-size multiplication can overflow on 32-bit builds, allocating a buffer smaller than the destination capacity passed to the converter.
1 open issue
1 resolved since last review
Review effort: Balanced
Add overflow check for WCHAR buffer allocation Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Changes to Unix parsing semantics and shared UTF conversion require maintainer review and cross-platform validation, with native builds and the new u16 suite still unexecuted.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
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.
Follow-up to #136973.