[libcxx-commits] [PATCH] D65339: Implement std::condition_variable via pthread_cond_clockwait() where available
Tom via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Aug 13 16:21:56 PDT 2019
tomcherry marked an inline comment as done.
tomcherry added inline comments.
Comment at: include/__mutex_base:476
+ typedef time_point<steady_clock, duration<long double, nano> > __c_tpf;
+ typedef time_point<steady_clock, nanoseconds> __c_tpi;
+ __c_tpf _Max = __c_tpi::max();
> tomcherry wrote:
> > EricWF wrote:
> > > This assumes the resolution of the `steady_clock`, no?
> > Sort of. It mostly assumes that steady_clock::now() can be converted into nanoseconds safely, which I think is a reasonable assumption to make. The old code assumed the resolution of system_clock too, so it's not a regression from there.
> > I'm happy to hear a better solution, because both this code and the original have never sit well with me. What we really want is a safe_duration_cast, where we get a ceiling effect if there would be overflow. Since that is not how std::chrono was created, we're left with two options:
> > 1) This function ends up with signed integer overflow if someone passes milliseconds::max() to it and steady/system_clock.
> > 2) Work-arounds like this
> > What we really want is a safe_duration_cast, where we get a ceiling effect if there would be overflow. Since that is not how std::chrono was created, we're left with two options:
> I suggest a third option: Write `__safe_duration_cast`
I intentionally didn't suggest that since that full solution would be out of the scope of this change, and I find it strange that we'd implement a private version, when we really do need a public safe_duration_cast.
But I did write something similar today: a safe_nanosecond_cast that only handles the cases needed for this code.
I assume that I cannot use __builtin_mul_overflow or similar in libcxx, so I avoided those, though would happily add them back if it were acceptable.
CHANGES SINCE LAST ACTION
More information about the libcxx-commits