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

<locale>: time_put.put("%r") prints a 24-hour time rather than a 12-hour time #371

Open
BillyONeal opened this issue Dec 11, 2019 · 3 comments
Labels
bug Something isn't working

Comments

@BillyONeal
Copy link
Member

BillyONeal commented Dec 11, 2019

Describe the bug
The C++ standard time_put defers to strftime, and the C standard says that %r is intended to print a 12-hour time. However, we appear to be printing a 24-hour time.

Command-line test case

C:\Users\bion\Desktop>type repro.cpp
#include <iostream>

#include <locale>

using namespace std;

class ctst : public std::time_put<char>{};

int main() {
    ctst stdtp;
    struct tm _tm {
        0, 0, 23, 1, 0, 0, 0, 0, 0
    };
    char format[]{'%', 'r', 0};
    stdtp.put(cout, cout, cout.fill(), &_tm, &format[0], &format[2]);
    cout << '\n';
    stdtp.put(cout, cout, cout.fill(), &_tm, format[1]);
    cout << '\n';
    return 0;
}

C:\Users\bion\Desktop>cl /EHsc .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.24.28314 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

repro.cpp
Microsoft (R) Incremental Linker Version 14.24.28314.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
repro.obj

C:\Users\bion\Desktop>.\repro.exe
23:00:00
23:00:00

Expected behavior
The output should have been a 12-hour time rather than a 24-hour time.

Additional context
It looks like we get the CRT rather than the STL to do most of this processing, so the result may be reducing this to a UCRT test case and submitting that against Windows.

if (0 < (_Count = _Strftime(&_Str[0], _Str.size(), _Fmt, _Pt, _Tnames._Getptr()))) {

This item is also tracked on Developer Community as DevCom-758960 and by Microsoft-internal VSO-998596 / AB#998596.

@BillyONeal BillyONeal added the bug Something isn't working label Dec 11, 2019
@StephanTLavavej StephanTLavavej changed the title <locale>: std::time_put.put("%r") prints a 24 hour time rather than a 12 hour time <locale>: time_put.put("%r") prints a 24-hour time rather than a 12-hour time Dec 11, 2019
@chuggafan
Copy link

I checked the UCRT library on my machine from C:\Program Files (x86)\Windows Kits\10\Source\10.0.18362.0 and the function that gets eventually called to expand the %r processing (expand_time) has a comment that states:

    // In the C locale, %r is equivalent to "%I:%M:%S %p".  This is the only
    // locale in which we guarantee that %r is a 12-hour time; in all other
    // locales we only have one time format which may or may not be a 12-hour
    // format.

So I'm assuming that the reason for the 24 hour time is that when it's not in the C locale it uses the WW_TIMEFORMAT, which on your machine drops you at a 24 hour time format. If it's verifyable that you're on C standard locale when you're testing this and you're still getting 24 hour time I'd be very confused since expand_time calls itself recursively for the values of: %I:%M:%S %p as given in the comments, meaning that expand_time failing for doing 12 hour time here would be the fault of %I, which mods the current time by 12 before printing it in the first place, making it impossible for %I to be the failure point.

TL;DR because I know I'm rambling: Check if you're on the C standard locale for this, but AFAICT the issue at hand is that the system deferring to local locale is causing the issue and that adding a forced 12-hour local locale for all locales will be the solution here, because the C standard locale is fine.

@BillyONeal
Copy link
Member Author

@chuggafan In the original repro the locale has not been changed, so it should be in the default (that is, "C") locale setting. The bug is that we are apparently not propagating that to all the right places correctly.

@MattStephanson
Copy link
Contributor

I took a look at this, and it appears to be a UCRT issue. STL calls _W_Gettnames to get the locale's date/time info, which returns a deep copy of such in the form of a __crt_lc_time_data* (cast to void*). expand_time receives this pointer and compares it with __lc_time_c, the pointer to the internal "C" locale data, which always yields false. There doesn't seem to be anything STL can do, unless it can get the actual __lc_time_c pointer.

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
None yet
Development

No branches or pull requests

3 participants