Remove narrow-to-wide formatter specializations#41143
Draft
benhillis wants to merge 2 commits into
Draft
Conversation
Copilot-Session: a404d085-142f-448f-b7fa-7ccc6528e844
Contributor
There was a problem hiding this comment.
Pull request overview
Removes the custom std::formatter narrow→wide specializations and makes UTF-8→UTF-16 conversions explicit at formatting boundaries (localization helpers, user-error paths, reporters, and tests), avoiding collisions with libc++’s deleted cross-encoding formatters.
Changes:
- Removed
char→wchar_tstd::formatterspecializations and updated thestd::source_locationwide formatter to explicitly convertfunction_name()/file_name(). - Updated generated localization helpers to materialize converted arguments before
make_wformat_args/vformat. - Updated Windows WSLC code paths and tests to explicitly call
wsl::shared::string::MultiByteToWidewhere wide formatting/user errors/reporting consume narrow values.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/generateLocalizationHeader.ps1 | Generates WIN32-only format-argument adapters that materialize narrow→wide conversions before vformat. |
| src/shared/inc/stringshared.h | Removes narrow→wide std::formatter specializations; updates std::source_location wide formatter to explicitly convert. |
| src/windows/wslcsession/WSLCSession.cpp | Converts Docker error strings to wide before THROW_HR_WITH_USER_ERROR* / warnings. |
| src/windows/wslcsession/WSLCContainer.cpp | Converts Docker error strings to wide before THROW_HR_WITH_USER_ERROR*. |
| src/windows/wslcsession/DockerHTTPClient.h | Converts Docker exception message to wide before user-error macro. |
| src/windows/wslc/services/ImageProgressCallback.cpp | Explicit narrow→wide conversions for reporter output and formatted status lines. |
| src/windows/wslc/services/BuildImageCallback.cpp | Explicit narrow→wide conversions for reporter output. |
| src/windows/wslc/services/ContainerModel.cpp | Makes user-error messages wide by converting narrow strings explicitly. |
| src/windows/wslc/services/ContainerService.cpp | Removes implicit formatting conversion by passing narrow args to localized helper directly. |
| src/windows/wslc/services/SessionService.cpp | Removes implicit formatting conversion by passing narrow args to localized helper directly. |
| src/windows/wslc/services/WinCredStorage.cpp | Removes implicit formatting conversion by passing narrow args to localized helper directly. |
| src/windows/wslc/services/FileCredStorage.cpp | Removes implicit formatting conversion by passing narrow args to localized helper directly. |
| src/windows/wslc/tasks/ImageTasks.cpp | Converts image IDs to wide before wide reporter output. |
| src/windows/wslc/tasks/NetworkTasks.cpp | Removes implicit formatting conversion by passing narrow args to localized helper directly. |
| src/windows/wslc/tasks/RegistryTasks.cpp | Removes implicit formatting conversion by passing narrow args to localized helper directly. |
| src/windows/wslc/tasks/VolumeTasks.cpp | Removes implicit formatting conversion by passing narrow args to localized helper directly. |
| src/windows/wslc/commands/VersionCommand.cpp | Converts package version to wide for wide reporter output. |
| src/windows/common/WSLCUserSettings.cpp | Removes implicit formatting conversion by passing e.what() directly to localized helper. |
| test/windows/Common.h | Updates unordered-vector mismatch formatting to explicitly convert std::string to wide for diagnostics. |
| test/windows/Common.cpp | Updates pattern-mismatch diagnostics to explicitly convert narrow strings to wide. |
| test/windows/WSLCTests.cpp | Converts narrow values to wide when formatting test diagnostics / expected strings. |
| test/windows/UnitTests.cpp | Converts narrow values to wide where wide formatting is used. |
| test/windows/SimpleTests.cpp | Converts narrow values to wide where wide formatting is used. |
| test/windows/DrvFsTests.cpp | Converts narrow paths to wide where wide formatting is used. |
| test/windows/wslc/e2e/WSLCE2EWarningTests.cpp | Removes extra conversion at call site now handled by localized helper / explicit conversions. |
| test/windows/wslc/e2e/WSLCE2EImageDeleteTests.cpp | Converts container/image IDs to wide when formatting expected stderr. |
| test/windows/wslc/e2e/WSLCE2EHelpers.cpp | Converts narrow error messages/IDs to wide for logging/expected output. |
| test/windows/wslc/e2e/WSLCE2EGlobalTests.cpp | Converts WSL_PACKAGE_VERSION to wide for formatted output. |
| test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp | Converts prompt to wide when formatting interactive command line. |
| test/windows/wslc/e2e/WSLCE2EContainerExecTests.cpp | Converts prompt/IDs to wide when formatting command lines and expected stderr. |
| test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp | Converts registry address/prompt to wide when formatting command lines. |
| test/windows/wslc/e2e/WSLCE2EContainerAttachTests.cpp | Converts prompt to wide when formatting command line. |
benhillis
marked this pull request as draft
July 22, 2026 22:26
Copilot-Session: a404d085-142f-448f-b7fa-7ccc6528e844
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
tools/generateLocalizationHeader.ps1:53
- FormatArgument(const std::basic_string<...>&) currently constructs a temporary std::string and then calls MultiByteToWide(const std::string&), which immediately calls c_str() and ignores the explicit size. This adds an unnecessary allocation/copy for non-std::string basic_string types without improving correctness.
src/windows/wslc/services/ContainerModel.cpp:29 - std::all_of(..., ::isdigit) is undefined behavior for chars with the high bit set because ::isdigit expects an unsigned char value (or EOF). This can mis-validate non-ASCII input depending on signedness/locale.
static auto parsePort = [](const std::string& value, const std::wstring& errorMessage) -> uint16_t {
try
{
// Ensure the value is not empty and contains only digits before parsing
if (value.empty() || !std::all_of(value.begin(), value.end(), ::isdigit))
{
|
|
||
| VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"{} {} {}", WSL_EXPORT_ARG, LXSS_DISTRO_NAME_TEST, tar.wstring()).c_str()), (DWORD)0); | ||
| VERIFY_ARE_EQUAL( | ||
| LxsstuLaunchWsl(std::format(L"{} {} {}", WSL_EXPORT_ARG, wsl::shared::string::MultiByteToWide(LXSS_DISTRO_NAME_TEST), tar.wstring()) |
| VERIFY_ARE_EQUAL(LxsstuLaunchWsl(std::format(L"mkdir -p '{}'", mountPoint)), 0); | ||
| const auto mountCommand = std::format(L"mount -t drvfs {}'{}' '{}'", readOnly ? L"-o ro " : L"", sourceDir.string(), mountPoint); | ||
| const auto mountCommand = std::format( | ||
| L"mount -t drvfs {}'{}' '{}'", readOnly ? L"-o ro " : L"", wsl::shared::string::MultiByteToWide(sourceDir.string()), mountPoint); |
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.
Summary
Follow-up to #41071. Remove the custom narrow-to-wide
std::formatterspecializations entirely and make cross-encoding explicit at the appropriate boundary.Generated localization helpers now adapt
char*, character arrays,std::basic_string<char>, andstd::basic_string_view<char>arguments throughwsl::shared::string::MultiByteToWide. Adapted values are materialized as local variables beforemake_wformat_argsso their storage remains valid throughvformat.Direct wide
std::format, Reporter, and user-error paths convert narrow values explicitly.Validation
WSLCTests::InvalidNamesWSLCVolumeMountUnitTests::VolumeMount_IsValidNamedVolumeName_InvalidNamesUnitTests::WslInfo