Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce std::time_base::mdy for "C" locale #4437

Merged
merged 5 commits into from
Mar 8, 2024

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Mar 3, 2024

Fixes #3346

  1. I found out that the tests work on libc++.
  2. I looked how libc++ detect date_order. It has a some date and run std::strftime(mbstr, sizeof(mbstr), "%x", std::localtime(&t)); and analyse its output.
  3. I looked at strftime implementation. It creates _LocaleUpdate object. and passes nullptr to it.
    _LocaleUpdate constructor looks like that (pseudo-code)
if (locale)
{
    _locale_pointers = *locale;
}
else if (is_it_C_locale())
{
    _locale_pointers = some_predefined_values;
}
else
{
   query runtime data to fill _locale_pointers from the current locale.
}
  1. So std::strftime uses mdy for "C" locale and we can do too. I found another places where we did a special case for "C" locale.

    STL/stl/src/xstrcoll.cpp

    Lines 57 to 69 in 8b081e2

    if (locale_name == nullptr) {
    int ans = memcmp(string1, string2, n1 < n2 ? n1 : n2);
    ret = (ans != 0 || n1 == n2 ? ans : n1 < n2 ? -1 : +1);
    } else {
    ret = __crtCompareStringA(locale_name, SORT_STRINGSORT, string1, n1, string2, n2, codepage);
    if (ret == 0) {
    errno = EINVAL;
    ret = _NLSCMPERROR;
    } else {
    ret -= 2;
    }
    }

    STL/stl/src/_tolower.cpp

    Lines 52 to 58 in 8b081e2

    if (locale_name == nullptr) {
    if (c >= 'A' && c <= 'Z') {
    c = c + ('a' - 'A');
    }
    return c;
    }

    STL/stl/src/_toupper.cpp

    Lines 51 to 57 in 8b081e2

    if (locale_name == nullptr) {
    if (c >= 'a' && c <= 'z') {
    c = c - ('a' - 'A');
    }
    return c;
    }

@fsb4000 fsb4000 requested a review from a team as a code owner March 3, 2024 18:27
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Mar 3, 2024
@StephanTLavavej StephanTLavavej self-assigned this Mar 3, 2024
stl/src/xdateord.cpp Outdated Show resolved Hide resolved
stl/src/xdateord.cpp Outdated Show resolved Hide resolved
stl/src/xdateord.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Mar 5, 2024
@StephanTLavavej
Copy link
Member

I believe this behavioral change is desirable - we'll properly detect when we should be using the "C" locale, instead of picking up LOCALE_NAME_USER_DEFAULT for the date order - but as it affects legacy code in my magical weak point, I'd like a second maintainer review here.

@StephanTLavavej StephanTLavavej self-assigned this Mar 6, 2024
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit 0613950 into microsoft:main Mar 8, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for investigating and fixing this longstanding bug that was affecting several contributors! 🌎 🌍 🌏

@fsb4000 fsb4000 deleted the fix3346 branch March 8, 2024 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Tests failing for DMY locales
2 participants