This is an archive of the discontinued LLVM Phabricator instance.

[libc++][chrono] Add sys_time formatter.
ClosedPublic

Authored by Mordante on Feb 24 2023, 8:23 AM.

Details

Reviewers
ldionne
vitaut
Group Reviewers
Restricted Project
Commits
rG2c1d79596fe7: [libc++][chrono] Add sys_time formatter.
Summary

Partially implements:

  • P1361 Integration of chrono with text formatting
  • P2372 Fixing locale handling in chrono formatters

Diff Detail

Event Timeline

Mordante created this revision.Feb 24 2023, 8:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2023, 8:23 AM
Mordante updated this revision to Diff 500417.Feb 25 2023, 4:44 AM

CI fixes.

Mordante updated this revision to Diff 500589.Feb 26 2023, 8:21 AM

CI fixes and temporary disable green CI jobs.

Mordante updated this revision to Diff 501172.Feb 28 2023, 9:13 AM

CI fixes.

Mordante updated this revision to Diff 501541.Mar 1 2023, 9:00 AM

CI fixes.

Mordante updated this revision to Diff 501554.Mar 1 2023, 9:50 AM

CI fixes.

Mordante updated this revision to Diff 501632.Mar 1 2023, 12:27 PM

CI fixes.

Mordante updated this revision to Diff 501648.Mar 1 2023, 1:01 PM

CI fixes.

Mordante updated this revision to Diff 501924.Mar 2 2023, 10:56 AM

CI fixes.

Mordante updated this revision to Diff 501936.Mar 2 2023, 11:35 AM

CI fixes.

Mordante updated this revision to Diff 501953.Mar 2 2023, 12:58 PM

CI fixes.

Mordante updated this revision to Diff 502141.Mar 3 2023, 8:29 AM

Rebased and enable the entire CI again.

Mordante updated this revision to Diff 502183.Mar 3 2023, 10:39 AM

Disable Windows for now.

Mordante published this revision for review.Mar 4 2023, 5:01 AM
Mordante added reviewers: ldionne, vitaut.
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2023, 5:01 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante added inline comments.Mar 22 2023, 9:25 AM
libcxx/test/std/time/time.clock/time.clock.system/ostream.pass.cpp
21

Review all test to match the availablity as changed in D134598.

ldionne accepted this revision.Mar 28 2023, 9:28 AM
ldionne added inline comments.
libcxx/include/__chrono/convert_to_tm.h
102

It would be nice to implement this code path for the clocks that we do implement in libc++ for LLVM 17, to avoid the library being inconsistent with the clocks that it provides.

libcxx/include/__chrono/ostream.h
43
This revision is now accepted and ready to land.Mar 28 2023, 9:28 AM
Mordante updated this revision to Diff 511742.Apr 7 2023, 11:11 AM
Mordante marked an inline comment as done.

Rebased and address review comments.

Mordante marked an inline comment as done.Apr 7 2023, 11:18 AM
Mordante added inline comments.
libcxx/include/__chrono/convert_to_tm.h
102

I agree that would be nice. This is on my TODO list. Note that we still miss parts of chrono for the clocks and TZDB. Also the there is no support for parse. So in general C++20 chrono is not in a great shape in libc++. (Note all chrono things are quite high on my priority list, but not at the top. I hope to get some things in LLVM 17, everything seems unlikely due to how much time I have to work on libc++.)

Mordante updated this revision to Diff 511747.Apr 7 2023, 11:23 AM
Mordante marked an inline comment as done.

CI fixes

Mordante updated this revision to Diff 511864.Apr 8 2023, 1:55 AM

CI fixes.

Mordante updated this revision to Diff 511874.Apr 8 2023, 3:53 AM

CI fixes.

This revision was landed with ongoing or failed builds.Apr 8 2023, 6:23 AM
This revision was automatically updated to reflect the committed changes.
bjope added a subscriber: bjope.EditedApr 12 2023, 2:22 AM

Started to see failures as shown below, downstream, after merging this patch.

So it seems like in my environment it says "GMT" instead of "UTC" (or perhaps the other way around).
Not sure exactly what might cause that and how to debug this problem. Any clues to what might cause this or what to look for?

FAIL: llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.sys_time.pass.cpp (79693 of 80953)
******************** TEST 'llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.sys_time.pass.cpp' FAILED ********************
Script:
--
: 'COMPILED WITH';  /repo/llvm-project/llvm/build-all-builtins/./bin/clang++ /repo/llvm-project/libcxx/test/std/time/time.syn/formatter.sys_time.pass.cpp  --target=x86_64-unknown-linux-gnu -nostdinc++ -I /repo/llvm-project/llvm/build-all-builtins/include/c++/v1 -I /repo/llvm-project/llvm/build-all-builtins/include/x86_64-unknown-linux-gnu/c++/v1 -I /repo/llvm-project/libcxx/test/support -std=c++2b -Werror -Wall -Wctad-maybe-unsupported -Wextra -Wshadow -Wundef -Wunused-template -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-noexcept-type -Wno-atomic-alignment -Wno-user-defined-literals -Wno-tautological-compare -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D_LIBCPP_ENABLE_EXPERIMENTAL -Werror=thread-safety -Wuser-defined-warnings  -lc++experimental -nostdlib++ -L /repo/llvm-project/llvm/build-all-builtins/./lib/x86_64-unknown-linux-gnu -Wl,-rpath,/repo/llvm-project/llvm/build-all-builtins/./lib/x86_64-unknown-linux-gnu -lc++ -pthread -o /repo/llvm-project/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/test/std/time/time.syn/Output/formatter.sys_time.pass.cpp.dir/t.tmp.exe
: 'EXECUTED AS';  "/python/3.8.0/bin/python3.8" /repo/llvm-project/libcxx/test/../utils/run.py --execdir /repo/llvm-project/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/test/std/time/time.syn/Output/formatter.sys_time.pass.cpp.dir --  /repo/llvm-project/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/test/std/time/time.syn/Output/formatter.sys_time.pass.cpp.dir/t.tmp.exe
--
Exit Code: 250

Command Output (stdout):
--
$ ":" "COMPILED WITH"
note: command had no output on stdout or stderr
$ "/repo/llvm-project/llvm/build-all-builtins/./bin/clang++" "/repo/llvm-project/libcxx/test/std/time/time.syn/formatter.sys_time.pass.cpp" "--target=x86_64-unknown-linux-gnu" "-nostdinc++" "-I" "/repo/llvm-project/llvm/build-all-builtins/include/c++/v1" "-I" "/repo/llvm-project/llvm/build-all-builtins/include/x86_64-unknown-linux-gnu/c++/v1" "-I" "/repo/llvm-project/libcxx/test/support" "-std=c++2b" "-Werror" "-Wall" "-Wctad-maybe-unsupported" "-Wextra" "-Wshadow" "-Wundef" "-Wunused-template" "-Wno-unused-command-line-argument" "-Wno-attributes" "-Wno-pessimizing-move" "-Wno-c++11-extensions" "-Wno-noexcept-type" "-Wno-atomic-alignment" "-Wno-user-defined-literals" "-Wno-tautological-compare" "-Wsign-compare" "-Wunused-variable" "-Wunused-parameter" "-Wunreachable-code" "-Wno-unused-local-typedef" "-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER" "-D_LIBCPP_ENABLE_EXPERIMENTAL" "-Werror=thread-safety" "-Wuser-defined-warnings" "-lc++experimental" "-nostdlib++" "-L" "/repo/llvm-project/llvm/build-all-builtins/./lib/x86_64-unknown-linux-gnu" "-Wl,-rpath,/repo/llvm-project/llvm/build-all-builtins/./lib/x86_64-unknown-linux-gnu" "-lc++" "-pthread" "-o" "/repo/llvm-project/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/test/std/time/time.syn/Output/formatter.sys_time.pass.cpp.dir/t.tmp.exe"
note: command had no output on stdout or stderr
$ ":" "EXECUTED AS"
note: command had no output on stdout or stderr
$ "/python/3.8.0/bin/python3.8" "/repo/llvm-project/libcxx/test/../utils/run.py" "--execdir" "/repo/llvm-project/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/test/std/time/time.syn/Output/formatter.sys_time.pass.cpp.dir" "--" "/repo/llvm-project/llvm/build-all-builtins/runtimes/runtimes-x86_64-unknown-linux-gnu-bins/test/std/time/time.syn/Output/formatter.sys_time.pass.cpp.dir/t.tmp.exe"
# command stderr:

Format string   {:L%%c='%c'%t%%Ec='%Ec'%n}
Expected output %c='jeu. 01 janv. 1970 00:00:00 GMT'	%Ec='jeu. 01 janv. 1970 00:00:00 GMT'

Actual output   %c='jeu. 01 janv. 1970 00:00:00 UTC'	%Ec='jeu. 01 janv. 1970 00:00:00 UTC'

t.tmp.exe: /repo/llvm-project/libcxx/test/std/time/time.syn/formatter_tests.h :41 : void check(std::basic_string_view<CharT>, test_format_string<CharT, Args...>, Args &&...) [CharT = char, Args = <std::chrono::time_point<std::chrono::system_clock, std::chrono::duration<long long>>>]:  l'assertion « out == expected » a échoué.

error: command failed with exit status: 250

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

1 warning(s) in tests
********************
Failed Tests (1):
  llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.sys_time.pass.cpp

Started to see failures as shown below, downstream, after merging this patch.

So it seems like in my environment it says "GMT" instead of "UTC" (or perhaps the other way around).
Not sure exactly what might cause that and how to debug this problem. Any clues to what might cause this or what to look for?

# command stderr:

Format string   {:L%%c='%c'%t%%Ec='%Ec'%n}
Expected output %c='jeu. 01 janv. 1970 00:00:00 GMT'	%Ec='jeu. 01 janv. 1970 00:00:00 GMT'

Actual output   %c='jeu. 01 janv. 1970 00:00:00 UTC'	%Ec='jeu. 01 janv. 1970 00:00:00 UTC'

t.tmp.exe: /repo/llvm-project/libcxx/test/std/time/time.syn/formatter_tests.h :41 : void check(std::basic_string_view<CharT>, test_format_string<CharT, Args...>, Args &&...) [CharT = char, Args = <std::chrono::time_point<std::chrono::system_clock, std::chrono::duration<long long>>>]:  l'assertion « out == expected » a échoué.

error: command failed with exit status: 250

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

1 warning(s) in tests
********************
Failed Tests (1):
  llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.sys_time.pass.cpp

These test highly depend on the platform specific libc's output. Which platform and libc are you using?
(Unfortunately it seems several platforms have other ideas how something is a specific locale should be.)

If you look at the failed test in this patch you already see quite some #ifdefs. The easiest solution for me would be to get a patch for your specific platform.

bjope added a comment.Apr 12 2023, 9:20 AM

Started to see failures as shown below, downstream, after merging this patch.

So it seems like in my environment it says "GMT" instead of "UTC" (or perhaps the other way around).
Not sure exactly what might cause that and how to debug this problem. Any clues to what might cause this or what to look for?

# command stderr:

Format string   {:L%%c='%c'%t%%Ec='%Ec'%n}
Expected output %c='jeu. 01 janv. 1970 00:00:00 GMT'	%Ec='jeu. 01 janv. 1970 00:00:00 GMT'

Actual output   %c='jeu. 01 janv. 1970 00:00:00 UTC'	%Ec='jeu. 01 janv. 1970 00:00:00 UTC'

t.tmp.exe: /repo/llvm-project/libcxx/test/std/time/time.syn/formatter_tests.h :41 : void check(std::basic_string_view<CharT>, test_format_string<CharT, Args...>, Args &&...) [CharT = char, Args = <std::chrono::time_point<std::chrono::system_clock, std::chrono::duration<long long>>>]:  l'assertion « out == expected » a échoué.

error: command failed with exit status: 250

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

1 warning(s) in tests
********************
Failed Tests (1):
  llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.sys_time.pass.cpp

These test highly depend on the platform specific libc's output. Which platform and libc are you using?
(Unfortunately it seems several platforms have other ideas how something is a specific locale should be.)

If you look at the failed test in this patch you already see quite some #ifdefs. The easiest solution for me would be to get a patch for your specific platform.

Right. I've noticed :-)

I'm on linux, RHEL7.9, targeting x86_64-unknown-linux-gnu. But not sure exactly what to look for. I just tried using a similar cmd line and adding "-dM", and it looks like I've got GLIBC 2.17. But no idea if this problem is specific to GLIBC or something else, so I wouldn't dare putting up a patch that expect UTC specifically for GLIBC 2.17.

For now I've just disabled the test case, so it is not causing me that much trouble. But I guess there might be others out there with similar problems.

Started to see failures as shown below, downstream, after merging this patch.

So it seems like in my environment it says "GMT" instead of "UTC" (or perhaps the other way around).
Not sure exactly what might cause that and how to debug this problem. Any clues to what might cause this or what to look for?

# command stderr:

Format string   {:L%%c='%c'%t%%Ec='%Ec'%n}
Expected output %c='jeu. 01 janv. 1970 00:00:00 GMT'	%Ec='jeu. 01 janv. 1970 00:00:00 GMT'

Actual output   %c='jeu. 01 janv. 1970 00:00:00 UTC'	%Ec='jeu. 01 janv. 1970 00:00:00 UTC'

t.tmp.exe: /repo/llvm-project/libcxx/test/std/time/time.syn/formatter_tests.h :41 : void check(std::basic_string_view<CharT>, test_format_string<CharT, Args...>, Args &&...) [CharT = char, Args = <std::chrono::time_point<std::chrono::system_clock, std::chrono::duration<long long>>>]:  l'assertion « out == expected » a échoué.

error: command failed with exit status: 250

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

1 warning(s) in tests
********************
Failed Tests (1):
  llvm-libc++-shared.cfg.in :: std/time/time.syn/formatter.sys_time.pass.cpp

These test highly depend on the platform specific libc's output. Which platform and libc are you using?
(Unfortunately it seems several platforms have other ideas how something is a specific locale should be.)

If you look at the failed test in this patch you already see quite some #ifdefs. The easiest solution for me would be to get a patch for your specific platform.

Right. I've noticed :-)

I'm on linux, RHEL7.9, targeting x86_64-unknown-linux-gnu. But not sure exactly what to look for. I just tried using a similar cmd line and adding "-dM", and it looks like I've got GLIBC 2.17. But no idea if this problem is specific to GLIBC or something else, so I wouldn't dare putting up a patch that expect UTC specifically for GLIBC 2.17.

For now I've just disabled the test case, so it is not causing me that much trouble. But I guess there might be others out there with similar problems.

According to https://sourceware.org/glibc/wiki/Glibc%20Timeline GLIBC 2.17 is over 10 years old. We don't have a fixed policy on which versions of GLIBC we support. Since most users don't run our tests and you have solved it locally I prefer to keep the code as is. If there are more bug reports with newer GLIBC versions I can look at another #ifdef.

I'm actually surprised there are not more parts of the C library, we depend on, missing with such an old GLIBC version.

According to https://sourceware.org/glibc/wiki/Glibc%20Timeline GLIBC 2.17 is over 10 years old. We don't have a fixed policy on which versions of GLIBC we support. Since most users don't run our tests and you have solved it locally I prefer to keep the code as is. If there are more bug reports with newer GLIBC versions I can look at another #ifdef.

I'm actually surprised there are not more parts of the C library, we depend on, missing with such an old GLIBC version.

We've actually started to only claim support for 2.24 and newer.

According to https://sourceware.org/glibc/wiki/Glibc%20Timeline GLIBC 2.17 is over 10 years old. We don't have a fixed policy on which versions of GLIBC we support. Since most users don't run our tests and you have solved it locally I prefer to keep the code as is. If there are more bug reports with newer GLIBC versions I can look at another #ifdef.

I'm actually surprised there are not more parts of the C library, we depend on, missing with such an old GLIBC version.

We've actually started to only claim support for 2.24 and newer.

Thanks it seems I missed that change. (I know we had a review about it, but missed we landed it.)