Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

features.py: Recognize the'Czech_Czechia.1250' locale#5480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
StephanTLavavej merged 2 commits intomicrosoft:mainfromStephanTLavavej:czechia
May 10, 2025

Conversation

@StephanTLavavej
Copy link
Member

Noticed by@muellerj2 in#5444 (comment). Windows 11 24H2 recognizes the locale name'Czech_Czechia.1250' and doesn't recognize'Czech_Czech Republic.1250'. I wasn't able to figure outwhen this change happened in Windows (a moderate amount of searching turned up nothing, and I even tried GitHub Copilot but it neither provided consistent answers nor cited any authoritative sources I could confirm), but it seems to have happened a while ago. (Presumably after our Python-powered test harness was added in 2020.)

We can assume/require that contributors and maintainers are running modern builds of Win11, and our CI infrastructure is now using Windows Server 2025, so we should switch to looking for the new locale name here. I believe our MSVC-internal infrastructure isn't using Server 2025 yet, but it doesn't use the Python-powered test harness, so this is irrelevant for it. (It doesn't even understand "REQUIRES" lines.)

Additionally, I have a logic cleanup to simplify how we add locale features. This just uses an if-statement instead of a when-lambda. It also uses a formatted string literal for clarity.

To validate this, I ran the test directories that require thecs_CZ.ISO8859-2 locale:

python tests\utils\stl-lit\stl-lit.py -o testing_x64.log -Dnotags=ASAN --order=random %STL%\llvm-project\libcxx\test\std\re\re.traits %STL%\llvm-project\libcxx\test\std\re\re.alg\re.alg.search %STL%\llvm-project\libcxx\test\std\re\re.alg\re.alg.match

There are 132 total discovered tests here. I then comparedmain,#5444, this PR, and#5444 merged with this PR:

Resultmain#5444This PRBoth PRs
Skipped44444444
Unsupported222200
Passed46464850
Expectedly Failed20204038

This just uses an if-statement instead of a when-lambda. It also uses a formatted string literal for clarity.
@muellerj2
Copy link
Contributor

I see one potential argument to resolve this differently: The locale setup in the test harness seems to have been adapted almost verbatim from libc++'s, so libc++'s test harness suffers from the same bug. If we resolve this issue in the same way as libc++, it might make it a little bit easier to sync changes from upstream in the future.

But I guess it's also a valid argument that the test setups have accumulated so many differences that another minor one doesn't matter.

@StephanTLavavej
Copy link
MemberAuthor

I'm open to a "better" fix, I just don't know what it would look like. Our feature-setup logic is very different.

@muellerj2
Copy link
Contributor

muellerj2 commentedMay 8, 2025
edited
Loading

If you think they have diverged too much, then there isn't much point in trying to keep the code as closely aligned as possible. Your code certainly looks cleaner.

The MSVC STL test harness also appears to be missing some locale-related compile flags that were recently added to libcxx tests, like this one in std\localization\locale.categories\category.monetary\locale.money.get\locale.money.get.members\get_long_double_fr_FR.pass.cpp:

// ADDITIONAL_COMPILE_FLAGS: -DFR_MON_THOU_SEP=%{LOCALE_CONV_FR_FR_UTF_8_MON_THOUSANDS_SEP}

The necessary logic for these flags was added to the original locale processing code in libc++'s features.pyhere.

We should add some equivalent logic to features.py as well (but probably not in this PR).

StephanTLavavej reacted with thumbs up emoji

Copy link
Member

@MahmoudGSalehMahmoudGSaleh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

LGTM

@StephanTLavavejStephanTLavavej moved this fromFinal Review toReady To Merge inSTL Code ReviewsMay 9, 2025
@StephanTLavavejStephanTLavavej moved this fromReady To Merge toMerging inSTL Code ReviewsMay 9, 2025
@StephanTLavavej
Copy link
MemberAuthor

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

StephanTLavavej added a commit to StephanTLavavej/STL that referenced this pull requestMay 9, 2025
@StephanTLavavejStephanTLavavej merged commita9ae27f intomicrosoft:mainMay 10, 2025
39 checks passed
@github-project-automationgithub-project-automationbot moved this fromMerging toDone inSTL Code ReviewsMay 10, 2025
@StephanTLavavejStephanTLavavej deleted the czechia branchMay 10, 2025 12:14
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@MahmoudGSalehMahmoudGSalehMahmoudGSaleh approved these changes

Assignees

No one assigned

Labels

testRelated to test code

Projects

Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@StephanTLavavej@muellerj2@MahmoudGSaleh

[8]ページ先頭

©2009-2025 Movatter.jp