Commit 5ab43b74 authored by David Van Cleve's avatar David Van Cleve Committed by Commit Bot

TimeDelta::FromDouble now uses saturated_cast

This change resolves the issue described in crbug.com/612601 (internal)
by changing the existing ternary implementation of TimeDelta::FromDouble
into a call to saturated_cast.

Back in 2016, when TimeDelta::FromDouble's current control flow
was written (https://crrev.com/1976703005), there was some concern
about using saturated_cast here because TimeDelta's permitted values
ranged from Max() to -Max() at the time. jyasskin implemented
a safe version [1] that met the [-Max(), Max()] requirement, but it
looks like the CL got lost somewhere before landing.

As of February 2018 (crrev.com/c/539058), the internal
value of TimeDelta can now range over all int64_t's, so the desired
semantics of TimeDelta::FromDouble are now those of
saturated_cast<int64_t>.

[1]
https://codereview.chromium.org/1976703005/diff/100001/base/time/time.h#newcode627

Test: Expanded time_unittest
Bug: 612601
Change-Id: Id00bdad9e732a22f0765c228b65ac6d65aa8b3d4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1830114Reviewed-by: default avatarAsanka Herath <asanka@chromium.org>
Reviewed-by: default avatardanakj <danakj@chromium.org>
Commit-Queue: Asanka Herath <asanka@chromium.org>
Commit-Queue: David Van Cleve <davidvc@chromium.org>
Auto-Submit: David Van Cleve <davidvc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701704}
parent 3b39e796
...@@ -837,13 +837,7 @@ constexpr TimeDelta TimeDelta::Min() { ...@@ -837,13 +837,7 @@ constexpr TimeDelta TimeDelta::Min() {
// static // static
constexpr TimeDelta TimeDelta::FromDouble(double value) { constexpr TimeDelta TimeDelta::FromDouble(double value) {
// TODO(crbug.com/612601): Use saturated_cast<int64_t>(value) once we sort out return TimeDelta(saturated_cast<int64_t>(value));
// the Min() behavior.
return value > std::numeric_limits<int64_t>::max()
? Max()
: value < std::numeric_limits<int64_t>::min()
? Min()
: TimeDelta(static_cast<int64_t>(value));
} }
// static // static
......
...@@ -1352,6 +1352,10 @@ TEST(TimeDelta, MaxConversions) { ...@@ -1352,6 +1352,10 @@ TEST(TimeDelta, MaxConversions) {
.is_max(), .is_max(),
""); "");
static_assert(
TimeDelta::FromMicrosecondsD(max_d).is_max(),
"Make sure that 2^63 correctly gets clamped to `max` (crbug.com/612601)");
// Floating point arithmetic resulting in infinity isn't constexpr in C++14. // Floating point arithmetic resulting in infinity isn't constexpr in C++14.
EXPECT_TRUE( EXPECT_TRUE(
TimeDelta::FromMillisecondsD(std::numeric_limits<double>::infinity()) TimeDelta::FromMillisecondsD(std::numeric_limits<double>::infinity())
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment