Base: Make TimeDelta constructors deal with overflow.

base/time/time.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Time represents an absolute point in coordinated universal time (UTC),
 // internally represented as microseconds (s/1,000,000) since the Windows epoch
 // (1601-01-01 00:00:00 UTC). System-dependent clock interface routines are
 // defined in time_PLATFORM.cc. Note that values for Time may skew and jump
 // around as the operating system makes adjustments to synchronize (e.g., with
 // NTP servers). Thus, client code that uses the Time class must account for
@@ skipping 246 matching lines @@
  private:
   friend int64 time_internal::SaturatedAdd(TimeDelta delta, int64 value);
   friend int64 time_internal::SaturatedSub(TimeDelta delta, int64 value);
 
   // Constructs a delta given the duration in microseconds. This is private
   // to avoid confusion by callers with an integer constructor. Use
   // FromSeconds, FromMilliseconds, etc. instead.
   explicit TimeDelta(int64 delta_us) : delta_(delta_us) {
   }
 
+  // Private method to build a delta from a double.
+  static TimeDelta FromDouble(double value);
+
   // Delta in microseconds.
   int64 delta_;
 };
 
 template<typename T>
 inline TimeDelta operator*(T a, TimeDelta td) {
   return td * a;
 }
 
 // For logging use only.
@@ skipping 313 matching lines @@
   // timezone is not specified.
   static bool FromStringInternal(const char* time_string,
                                  bool is_local,
                                  Time* parsed_time);
 };
 
 // Inline the TimeDelta factory methods, for fast TimeDelta construction.
 
 // static
 inline TimeDelta TimeDelta::FromDays(int days) {
-  // Preserve max to prevent overflow.
+  // Special-case max.

[Nico, 2015/07/17 17:00:23]

The previous comment answered "why?". From what I understand, it did so
incorrectly, but the intention was good.

[rvargas (doing something else), 2015/07/17 19:18:10]

yeah... I actually don't like this. FromDays(max_int) returns Max, but
FromDays(max_int - 1) doesn't, and neither value is really overflowing anything.
However I'm afraid of code somewhere relying on that behavior :(

So the real reason of "why" today is "backwards compatibility" (or actually,
fear of breaking things), even though that was never part of the contract.

I could go through the 746 call sites that I see. (+388 FromHours + 464
FromMinutes)

[Nico, 2015/07/17 20:21:20]

Hm, maybe just remove the comment then I guess. As is, it just repeats the line
below.

   if (days == std::numeric_limits<int>::max())
     return Max();
   return TimeDelta(days * Time::kMicrosecondsPerDay);

[Nico, 2015/07/17 17:00:23]

doesn't days * Time::kMicrosecondsPerDay overflow too if days is large but less
than max()?

…ah, kMicrosecondsPerDay is a int64, I see.

 }
 
 // static
 inline TimeDelta TimeDelta::FromHours(int hours) {
-  // Preserve max to prevent overflow.
+  // Special-case max.
   if (hours == std::numeric_limits<int>::max())
     return Max();
   return TimeDelta(hours * Time::kMicrosecondsPerHour);
 }
 
 // static
 inline TimeDelta TimeDelta::FromMinutes(int minutes) {
-  // Preserve max to prevent overflow.
+  // Special-case max.
   if (minutes == std::numeric_limits<int>::max())
     return Max();
   return TimeDelta(minutes * Time::kMicrosecondsPerMinute);
 }
 
 // static
 inline TimeDelta TimeDelta::FromSeconds(int64 secs) {
-  // Preserve max to prevent overflow.
-  if (secs == std::numeric_limits<int64>::max())
-    return Max();
-  return TimeDelta(secs * Time::kMicrosecondsPerSecond);
+  return TimeDelta(secs) * Time::kMicrosecondsPerSecond;

[Nico, 2015/07/17 17:00:23]

Does TimeDelta have a saturating operator*?

[rvargas (doing something else), 2015/07/17 19:18:10]

Yes, all TimeDelta operators should do the right thing on overflow.

 }
 
 // static
 inline TimeDelta TimeDelta::FromMilliseconds(int64 ms) {
-  // Preserve max to prevent overflow.
-  if (ms == std::numeric_limits<int64>::max())
-    return Max();
-  return TimeDelta(ms * Time::kMicrosecondsPerMillisecond);
+  return TimeDelta(ms) * Time::kMicrosecondsPerMillisecond;
 }
 
 // static
 inline TimeDelta TimeDelta::FromSecondsD(double secs) {
-  // Preserve max to prevent overflow.
-  if (secs == std::numeric_limits<double>::infinity())
-    return Max();
-  return TimeDelta(static_cast<int64>(secs * Time::kMicrosecondsPerSecond));
+  return FromDouble(secs * Time::kMicrosecondsPerSecond);
 }
 
 // static
 inline TimeDelta TimeDelta::FromMillisecondsD(double ms) {
-  // Preserve max to prevent overflow.
-  if (ms == std::numeric_limits<double>::infinity())
-    return Max();
-  return TimeDelta(static_cast<int64>(ms * Time::kMicrosecondsPerMillisecond));
+  return FromDouble(ms * Time::kMicrosecondsPerMillisecond);
 }
 
 // static
 inline TimeDelta TimeDelta::FromMicroseconds(int64 us) {
-  // Preserve max to prevent overflow.
-  if (us == std::numeric_limits<int64>::max())
-    return Max();
   return TimeDelta(us);
 }
 
+// static
+inline TimeDelta TimeDelta::FromDouble(double value) {
+  double max_magnitude = std::numeric_limits<int64>::max();
+  TimeDelta delta = TimeDelta(static_cast<int64>(value));
+  if (value > max_magnitude)
+    delta = Max();
+  else if (value < -max_magnitude)
+    delta = -Max();
+  return delta;
+}
+
 // For logging use only.
 BASE_EXPORT std::ostream& operator<<(std::ostream& os, Time time);
 
 // TimeTicks ------------------------------------------------------------------
 
 // Represents monotonically non-decreasing clock time.
 class BASE_EXPORT TimeTicks : public time_internal::TimeBase<TimeTicks> {
  public:
   TimeTicks() : TimeBase(0) {
   }
@@ skipping 129 matching lines @@
   explicit TraceTicks(int64 us) : TimeBase(us) {
   }
 };
 
 // For logging use only.
 BASE_EXPORT std::ostream& operator<<(std::ostream& os, TraceTicks time_ticks);
 
 }  // namespace base
 
 #endif  // BASE_TIME_TIME_H_