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

[libc++][test] Streaming out floating-point sys_time and local_time is bogus #73849

Closed
StephanTLavavej opened this issue Nov 29, 2023 · 1 comment · Fixed by #76456
Closed
Assignees
Labels
confirmed Verified by a second party libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@StephanTLavavej
Copy link
Member

N4964 [time.clock.system.nonmembers]/1:

template<class charT, class traits, class Duration>
  basic_ostream<charT, traits>&
    operator<<(basic_ostream<charT, traits>& os, const sys_time<Duration>& tp);

Constraints: treat_as_floating_point_v<typename Duration::rep> is false, and Duration{1} < days{1} is true.

forbids:

assert(stream_c_locale<CharT>(std::chrono::sys_time<std::chrono::duration<float, std::ratio<1, 1>>>{
std::chrono::duration<float, std::ratio<1, 1>>{123.456}}) == SV("1970-01-01 00:02:03"));
assert(stream_c_locale<CharT>(std::chrono::sys_time<std::chrono::duration<double, std::ratio<1, 10>>>{
std::chrono::duration<double, std::ratio<1, 10>>{123.456}}) == SV("1970-01-01 00:00:12.3"));
assert(stream_c_locale<CharT>(std::chrono::sys_time<std::chrono::duration<long double, std::ratio<1, 100>>>{
std::chrono::duration<long double, std::ratio<1, 100>>{123.456}}) == SV("1970-01-01 00:00:01.23"));

assert(stream_fr_FR_locale<CharT>(std::chrono::sys_time<std::chrono::duration<float, std::ratio<1, 1>>>{
std::chrono::duration<float, std::ratio<1, 1>>{123.456}}) == SV("1970-01-01 00:02:03"));
assert(stream_fr_FR_locale<CharT>(std::chrono::sys_time<std::chrono::duration<double, std::ratio<1, 10>>>{
std::chrono::duration<double, std::ratio<1, 10>>{123.456}}) == SV("1970-01-01 00:00:12,3"));
assert(stream_fr_FR_locale<CharT>(std::chrono::sys_time<std::chrono::duration<long double, std::ratio<1, 100>>>{
std::chrono::duration<long double, std::ratio<1, 100>>{123.456}}) == SV("1970-01-01 00:00:01,23"));


And [time.clock.local]/2:

template<class charT, class traits, class Duration>
  basic_ostream<charT, traits>&
    operator<<(basic_ostream<charT, traits>& os, const local_time<Duration>& lt);

Effects: os << sys_time<Duration>{lt.time_since_epoch()};

forbids:

assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::duration<float, std::ratio<1, 1>>>{
std::chrono::duration<float, std::ratio<1, 1>>{123.456}}) == SV("1970-01-01 00:02:03"));
assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::duration<double, std::ratio<1, 10>>>{
std::chrono::duration<double, std::ratio<1, 10>>{123.456}}) == SV("1970-01-01 00:00:12.3"));
assert(stream_c_locale<CharT>(std::chrono::local_time<std::chrono::duration<long double, std::ratio<1, 100>>>{
std::chrono::duration<long double, std::ratio<1, 100>>{123.456}}) == SV("1970-01-01 00:00:01.23"));

assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::duration<float, std::ratio<1, 1>>>{
std::chrono::duration<float, std::ratio<1, 1>>{123.456}}) == SV("1970-01-01 00:02:03"));
assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::duration<double, std::ratio<1, 10>>>{
std::chrono::duration<double, std::ratio<1, 10>>{123.456}}) == SV("1970-01-01 00:00:12,3"));
assert(stream_fr_FR_locale<CharT>(std::chrono::local_time<std::chrono::duration<long double, std::ratio<1, 100>>>{
std::chrono::duration<long double, std::ratio<1, 100>>{123.456}}) == SV("1970-01-01 00:00:01,23"));

@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 29, 2023
@mordante mordante added confirmed Verified by a second party and removed test-suite labels Dec 24, 2023
@mordante
Copy link
Member

Thanks! I had a look and it seems I didn't implement the required constraint. So it's not just the test suite, the implementation has issues too.

mordante added a commit to mordante/llvm-project that referenced this issue Dec 28, 2023
- The sys_time formatter is constrained, which was not implemented.
- There is a sys_days formatter which was not implemented.
- The local_time formatter uses the sys_time formatter in its
  implementation so "inherited" the same issues.

Fixes: llvm#73849
Fixes: llvm#67983
mordante added a commit to mordante/llvm-project that referenced this issue Jan 21, 2024
- The sys_time formatter is constrained, which was not implemented.
- There is a sys_days formatter which was not implemented.
- The local_time formatter uses the sys_time formatter in its
  implementation so "inherited" the same issues.

Fixes: llvm#73849
Fixes: llvm#67983
mordante added a commit that referenced this issue Jan 22, 2024
- The sys_time formatter is constrained, which was not implemented.
- There is a sys_days formatter which was not implemented.
- The local_time formatter uses the sys_time formatter in its
implementation so "inherited" the same issues.

Fixes: #73849
Fixes: #67983
blueboxd pushed a commit to blueboxd/libcxx that referenced this issue Feb 7, 2024
- The sys_time formatter is constrained, which was not implemented.
- There is a sys_days formatter which was not implemented.
- The local_time formatter uses the sys_time formatter in its
implementation so "inherited" the same issues.

Fixes: llvm/llvm-project#73849
Fixes: llvm/llvm-project#67983
NOKEYCHECK=True
GitOrigin-RevId: 042a6a1349d512edaaa225380771c64a8d92810a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed Verified by a second party libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants