[libcxx] r290806 - chrono: address post commit comments from Howard

[Howard Hinnant via cfe-commits]

Very close!  Comment below:

On Jan 1, 2017, at 5:04 PM, Saleem Abdulrasool via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> Author: compnerd
> Date: Sun Jan  1 16:04:38 2017
> New Revision: 290806
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=290806&view=rev
> Log:
> chrono: address post commit comments from Howard
> 
> Drawing some inspiration from code from Bill O'Neal as pointed out by
> Howard, rework the code to avoid an overflow in the duration.  Adjust
> the style to match libc++ style as well.
> 
> Create a local typedef for the FILETIME duration (100-ns units).  Use
> this to define the difference between the NT and the UNIX epochs (which
> previously overflowed due to the representation limits due to the
> bouncing to ns).  Return the FILETIME duration biased by the NT-to-UNIX
> epoch conversion.
> 
> Use of the custom duration makes it easier to read and reason about the
> code.
> 
> Modified:
>    libcxx/trunk/src/chrono.cpp
> 
> Modified: libcxx/trunk/src/chrono.cpp
> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/chrono.cpp?rev=290806&r1=290805&r2=290806&view=diff
> ==============================================================================
> --- libcxx/trunk/src/chrono.cpp (original)
> +++ libcxx/trunk/src/chrono.cpp Sun Jan  1 16:04:38 2017
> @@ -46,26 +46,30 @@ system_clock::time_point
> system_clock::now() _NOEXCEPT
> {
> #if defined(_WIN32)
> -  // The Windows epoch is Jan 1 1601, the Unix epoch Jan 1 1970.  The difference
> -  // in nanoseconds is the windows epoch offset.
> -  static const constexpr __int64 kWindowsEpochOffset = 0x19db1ded53e8000;
> +  // FILETIME is in 100ns units
> +  using filetime_duration =
> +      _VSTD::chrono::duration<__int64,
> +                              _VSTD::ratio_multiply<_VSTD::ratio<100, 1>,
> +                                                    nanoseconds::period>>;
> 
> -  FILETIME ftSystemTime;
> +  // The Windows epoch is Jan 1 1601, the Unix epoch Jan 1 1970.
> +  static _LIBCPP_CONSTEXPR const filetime_duration
> +      nt_to_unix_epoch{11644473600};

If you give nt_to_unix_epoch units of seconds, instead of filetime_duration, then I _believe_ this will be correct.  But I have not tested it.

I like this direction much better as all of your computations have units now.

Howard

> +
> +  FILETIME ft;
> #if _WIN32_WINNT >= _WIN32_WINNT_WIN8
> #if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)
> -  GetSystemTimePreciseAsFileTime(&ftSystemTime);
> +  GetSystemTimePreciseAsFileTime(&ft);
> #else
> -  GetSystemTimeAsFileTime(&ftSystemTime);
> +  GetSystemTimeAsFileTime(&ft);
> #endif
> #else
> -  GetSystemTimeAsFileTime(&ftSystemTime);
> +  GetSystemTimeAsFileTime(&ft);
> #endif
> -  __int64 llWinTimeNS =
> -      ((static_cast<__int64>(ftSystemTime.dwHighDateTime) << 32) |
> -       static_cast<__int64>(ftSystemTime.dwLowDateTime)) *
> -      100;
> -  return time_point(duration_cast<duration>(
> -      (nanoseconds(llWinTimeNS - kWindowsEpochOffset))));
> +
> +  filetime_duration d{(static_cast<__int64>(ft.dwHighDateTime) << 32) |
> +                       static_cast<__int64>(ft.dwLowDateTime)};
> +  return time_point(duration_cast<duration>(d - nt_to_unix_epoch));
> #else
> #ifdef CLOCK_REALTIME
>     struct timespec tp;
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits