[libc++] Implement file_clock::{to,from}_sys

[ldionne]

Summary

This is part of https://wg21.link/P0355R7. I am adding these methods
to provide an alternative for the {from,to}_time_t methods that were
removed in https://llvm.org/D113027.

[ldionne]

libcxx/include/chrono

2822

IIUC, both the system_clock and file_clock have the same Epochs (the Unix one) on all the platforms we support. So we shouldn't need to do any additional adjustment here AFAIU. Same for from_sys.

[• Quuxplusone]

libcxx/docs/ReleaseNotes.rst

83–86

Instead, in C++20 you can use...?
But what about prior to C++20? Even if the answer is "go jump in a lake," I think it's worth release-noting that. :)

libcxx/test/std/utilities/time/time.clock/time.clock.file/to_from_sys.pass.cpp

34–37

Naming nit: When I see various variables named foo_now, it fires my red-flag detector for TOCTTOU vulnerabilities. In this case, file_now is arguably a "now," but sys_now is clearly a "then," because by the time we've computed it, "now" is later than before. ;) I suggest just ft and st, here and below. (You can still initialize ft with file_clock::now() if you're confident you won't mind whatever's going to happen to this test on DST day or whatever. I just don't like the variable's own name to include the word now.)

47–58

Here, this is a good use of variables named foo_now. :)
I'd recommend testing that

assert(std::chrono::milliseconds(-500) < diff && diff < std::chrono::milliseconds(500));

just to be on the safe side. (And notice my usual preference for () in object-factory-style constructors, idiomatically reserving {} for braced initializer lists.)

60–67

In many cases we'll have sys_diff == file_diff == 0, right?
I'm not sure I understand the fundamental point of this test; it might be clearer if we had any way of generating constant time_points other than the racey now().
Do we intend that

LIBCPP_ASSERT(std::is_same_v<decltype(sys_diff), decltype(file_diff)>);

?

[ldionne]

Address review comments. Thanks!

libcxx/docs/ReleaseNotes.rst

83–86

Oh, yeah, somehow I thought the from_time_t we removed was C++20 only. Will adjust to basically say "move to C++20", since this whole file_clock thing is supposed to be C++20 anyways. For some reason I would have bet that it was C++17 since it is tied to filesystem -- oh well.

libcxx/test/std/utilities/time/time.clock/time.clock.file/to_from_sys.pass.cpp

60–67

Do we intend that LIBCPP_ASSERT(std::is_same_v<decltype(sys_diff), decltype(file_diff)>);?

No, we don't. Or at least I don't. In my mind, decltype(sys_diff) is chrono::duration<representation-of-system-clock, period-of-system-clock> whereas decltype(file_diff) is chrono::duration<representation-of-file-clock, period-of-file-clock>, and both don't need to be the same. And in fact they aren't in our implementation (system clock has rep = long long, period = milliseconds and file clock has rep = int128_t, period = nanoseconds).

Both should be "equal", in the sense that if you convert both to the least granular measurement, you should get the same answer. However, both are not necessarily going to equal 0 ticks, because the two time stamps were not taken at the same time. In practice I expect the difference will be very small because there's almost no time between the two calls to now(), but we still should not assume it is 0.

[• Quuxplusone]

libcxx/test/std/utilities/time/time.clock/time.clock.file/to_from_sys.pass.cpp

30

Btw, IMO it'd be nice to get rid of this line and use std::chrono:: everywhere, for greppability.

60–67

In practice I expect the difference will be very small because there's almost no time between the two calls to now(), but we still should not assume it is 0.

Right; what I meant was more in the line of "flaky tests." Here we have a test where — maybe not 99% of the time, but some significant fraction of the time — it's testing that 0 == 0; and then some small-and-nonreproducible remainder of the time, it's testing something-other-than-that. This is a "testing code smell."

It would be strictly better if we could do something like

auto const ft = chrono::file_clock::time_point("2021-01-01 01:23:45");
auto const st = chrono::system_clock::time_point("2021-01-01 01:23:45");
auto sys_diff = chrono::file_clock::to_sys(ft) - st;
auto file_diff = ft - chrono::file_clock::from_sys(st);
assert(sys_diff == file_diff);

but I don't know that such a primitive for generating timepoints exists (or if it does, it's probably in the date stuff we haven't implemented yet).

[ldionne]

Remove namespace chrono = std::chrono and add XFAIL on 32 bits.

[ldionne]

libcxx/test/std/utilities/time/time.clock/time.clock.file/to_from_sys.pass.cpp

60–67

I agree and would 100% have written the test that way, but as you say, we don't have a good way of generating a time point besides now().

[ldionne]

Rebase onto main

libcxx/docs/ReleaseNotes.rst

 - The functions ``std::allocator<T>::allocate`` and
   ``std::experimental::pmr::polymorphic_allocator<T>::allocate`` now throw
   an exception of type ``std::bad_array_new_length`` when the requested size
   exceeds the maximum supported size, as required by the C++ standard.
   Previously the type ``std::length_error`` was used.
 
 - Removed the nonstandard methods ``std::chrono::file_clock::to_time_t`` and
   ``std::chrono::file_clock::from_time_t``; neither libstdc++ nor MSVC STL
-  had such methods.
+  had such methods. Instead, in C++20, you can use ``std::chrono::file_clock::from_sys``
+  and ``std::chrono::file_clock::to_sys``, which are specified in the Standard.
+  If you are not using C++20, you should move to it.
 
 ABI Changes
 -----------
 
 - The C++17 variable templates ``is_error_code_enum_v`` and
   ``is_error_condition_enum_v`` are now of type ``bool`` instead of ``size_t``.
 
 Build System Changes
 --------------------

libcxx/include/chrono

public:
     static time_point from_time_t(time_t __t) noexcept;
 };
 
 template <class Duration>
   using sys_time  = time_point<system_clock, Duration>; // C++20
 using sys_seconds = sys_time<seconds>;                  // C++20
 using sys_days    = sys_time<days>;                     // C++20
 
-class file_clock;                                       // C++20
+class file_clock                                        // C++20
+{
+public:
+    typedef see-below                      rep;
+    typedef nano                           period;
+    typedef chrono::duration<rep, period>  duration;
+    typedef chrono::time_point<file_clock> time_point;
+    static constexpr bool is_steady =      false;
+
+    static time_point now() noexcept;
+
+    template<class Duration>
+    static sys_time<see-below> to_sys(const file_time<Duration>&);
+
+    template<class Duration>
+    static file_time<see-below> from_sys(const sys_time<Duration>&);
+};
 
 template<class Duration>
   using file_time = time_point<file_clock, Duration>;   // C++20
 
 class steady_clock
 {
 public:
     typedef nanoseconds                                   duration;
#endif
 
   typedef chrono::duration<rep, period> duration;
   typedef chrono::time_point<_FilesystemClock> time_point;
 
   _LIBCPP_EXPORTED_FROM_ABI
   static _LIBCPP_CONSTEXPR_AFTER_CXX11 const bool is_steady = false;
 
   _LIBCPP_AVAILABILITY_FILESYSTEM _LIBCPP_FUNC_VIS static time_point now() noexcept;
+
+#if _LIBCPP_STD_VER > 17
+  template <class _Duration>
+  _LIBCPP_HIDE_FROM_ABI
+  static chrono::sys_time<_Duration> to_sys(const chrono::file_time<_Duration>& __t) {
+    return chrono::sys_time<_Duration>(__t.time_since_epoch());
+  }
+
+  template <class _Duration>
+  _LIBCPP_HIDE_FROM_ABI
+  static chrono::file_time<_Duration> from_sys(const chrono::sys_time<_Duration>& __t) {
+    return chrono::file_time<_Duration>(__t.time_since_epoch());
+  }
+#endif // _LIBCPP_STD_VER > 17
 };
 _LIBCPP_END_NAMESPACE_FILESYSTEM
 #endif // !_LIBCPP_CXX03_LANG
 
 _LIBCPP_POP_MACROS
 
 #endif // _LIBCPP_CHRONO

libcxx/test/std/utilities/time/time.clock/time.clock.file/to_from_sys.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-has-no-filesystem-library
+
+// Filesystem is supported on Apple platforms starting with macosx10.15.
+// UNSUPPORTED: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11|12|13|14}}
+
+// TODO(ldionne): This test fails on Ubuntu Focal on our CI nodes (and only there), in 32 bit mode.
+// UNSUPPORTED: linux && 32bits-on-64bits
+
+// <chrono>
+//
+// file_clock
+//
+// template<class Duration>
+// static sys_time<see-below> to_sys(const file_time<Duration>&);
+//
+// template<class Duration>
+// static file_time<see-below> from_sys(const sys_time<Duration>&);
+
+#include <chrono>
+#include <cassert>
+
+int main(int, char**) {
+  // Test round-trip through the system clock, starting from file_clock::now()
+  {
+    std::chrono::file_clock::time_point const ft = std::chrono::file_clock::now();
+    auto st = std::chrono::file_clock::to_sys(ft);
+    assert(ft == std::chrono::file_clock::from_sys(st));
+  }
+
+  // Test round-trip through the system clock, starting from system_clock::now()
+  {
+    std::chrono::system_clock::time_point const st = std::chrono::system_clock::now();
+    auto ft = std::chrono::file_clock::from_sys(st);
+    assert(st == std::chrono::file_clock::to_sys(ft));
+  }
+
+  // Make sure the value we get is in the ballpark of something reasonable
+  {
+    std::chrono::file_clock::time_point const file_now = std::chrono::file_clock::now();
+    std::chrono::system_clock::time_point const sys_now = std::chrono::system_clock::now();
+    {
+      auto diff = sys_now - std::chrono::file_clock::to_sys(file_now);
+      assert(std::chrono::milliseconds(-500) < diff && diff < std::chrono::milliseconds(500));
+    }
+    {
+      auto diff = std::chrono::file_clock::from_sys(sys_now) - file_now;
+      assert(std::chrono::milliseconds(-500) < diff && diff < std::chrono::milliseconds(500));
+    }
+  }
+
+  // Make sure to_sys and from_sys are consistent with each other
+  {
+    std::chrono::file_clock::time_point const ft = std::chrono::file_clock::now();
+    std::chrono::system_clock::time_point const st = std::chrono::system_clock::now();
+    auto sys_diff = std::chrono::file_clock::to_sys(ft) - st;
+    auto file_diff = ft - std::chrono::file_clock::from_sys(st);
+    assert(sys_diff == file_diff);
+  }
+
+  return 0;
+}