From 5fc78a94cb3fb1751e530f4cf70ff2bc8d122158 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Mon, 10 Jul 2023 09:27:33 -0700
Subject: [PATCH] [time] update DistantFuture/Past() to exclude value half
 range apart (#9263)

This commit changes the `Time::GetDistantFuture/Past()` methods to
exclude the ambiguous time value which is half range `(1 << 31)`
apart. The returned value is now `(1 << 31) - 1` apart. This ensures
that `Min(now.GetDistantFuture(), now)` is well-defined and the same
as `Min(now, now.GetDistantFuture())`. This commit also updates
`test_timer` to validate this.
---
 src/core/common/time.hpp  | 20 +++++++++++---------
 tests/unit/test_timer.cpp | 15 +++++++++++----
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/src/core/common/time.hpp b/src/core/common/time.hpp
index 12c16f8b1..89c22c239 100644
--- a/src/core/common/time.hpp
+++ b/src/core/common/time.hpp
@@ -217,26 +217,28 @@ public:
     /**
      * Returns a new `Time` instance which is in distant future relative to current `Time` object.
      *
-     * The returned distance future `Time` is guaranteed to be equal or after (as defined by comparison operator `<=`)
-     * any other `Time` which is after this `Time` object, i.e., for any `t` for which we have `*this <= t`, it is
-     * ensured that `t <= this->GetGetDistantFuture()`.
+     * The distant future is the largest time that is ahead of `Time`. For any time `t`, if `(*this <= t)`, then
+     * `t <= this->GetGetDistantFuture()`, except for the ambiguous `t` value which is half range `(1 << 31)` apart.
+     *
+     * When comparing `GetDistantFuture()` with a time `t` the caller must ensure that `t` is already ahead of `*this`.
      *
      * @returns A new `Time` in distance future relative to current `Time` object.
      *
      */
-    Time GetDistantFuture(void) const { return Time(mValue + kDistantFuture); }
+    Time GetDistantFuture(void) const { return Time(mValue + kDistantInterval); }
 
     /**
      * Returns a new `Time` instance which is in distant past relative to current `Time` object.
      *
-     * The returned distance past `Time` is guaranteed to be equal or before (as defined by comparison operator `>=`)
-     * any other `Time` which is before this `Time` object, i.e., for any `t` for which we have `*this >= t`, it is
-     * ensured that `t >= this->GetDistantPast()`.
+     * The distant past is the smallest time that is before `Time`. For any time `t`, if `(t <= *this )`, then
+     * `this->GetGetDistantPast() <= t`, except for the ambiguous `t` value which is half range `(1 << 31)` apart.
+     *
+     * When comparing `GetDistantPast()` with a time `t` the caller must ensure that the `t` is already before `*this`.
      *
      * @returns A new `Time` in distance past relative to current `Time` object.
      *
      */
-    Time GetDistantPast(void) const { return Time(mValue - kDistantFuture); }
+    Time GetDistantPast(void) const { return Time(mValue - kDistantInterval); }
 
     /**
      * Converts a given number of seconds to milliseconds.
@@ -259,7 +261,7 @@ public:
     static uint32_t constexpr MsecToSec(uint32_t aMilliseconds) { return aMilliseconds / 1000u; }
 
 private:
-    static constexpr uint32_t kDistantFuture = (1UL << 31);
+    static constexpr uint32_t kDistantInterval = (1UL << 31) - 1;
 
     uint32_t mValue;
 };
diff --git a/tests/unit/test_timer.cpp b/tests/unit/test_timer.cpp
index 26b55d8ce..b3509f833 100644
--- a/tests/unit/test_timer.cpp
+++ b/tests/unit/test_timer.cpp
@@ -32,6 +32,7 @@
 #include "common/code_utils.hpp"
 #include "common/debug.hpp"
 #include "common/instance.hpp"
+#include "common/num_utils.hpp"
 #include "common/timer.hpp"
 
 enum
@@ -650,14 +651,20 @@ int TestTimerTime(void)
             VerifyOrQuit(t1 - t2 == duration, "Time difference failed");
 
             t2 = t1.GetDistantFuture();
-            VerifyOrQuit((t1 < t2), "GetDistanceFuture() failed");
+            VerifyOrQuit((t1 < t2) && !(t1 > t2), "GetDistanceFuture() failed");
             t2 += 1;
-            VerifyOrQuit(!(t1 < t2), "GetDistanceFuture() failed");
+            VerifyOrQuit(!(t1 < t2) || (t1 > t2), "GetDistanceFuture() failed");
 
             t2 = t1.GetDistantPast();
-            VerifyOrQuit((t1 > t2), "GetDistantPast() failed");
+            VerifyOrQuit((t1 > t2) && !(t1 < t2), "GetDistantPast() failed");
             t2 -= 1;
-            VerifyOrQuit(!(t1 > t2), "GetDistantPast() failed");
+            VerifyOrQuit(!(t1 > t2) || (t1 < t2), "GetDistantPast() failed");
+
+            VerifyOrQuit(Min(t1, t1.GetDistantFuture()) == t1);
+            VerifyOrQuit(Min(t1.GetDistantFuture(), t1) == t1);
+
+            VerifyOrQuit(Max(t1, t1.GetDistantPast()) == t1);
+            VerifyOrQuit(Max(t1.GetDistantPast(), t1) == t1);
 
             printf("--> PASSED\n");
         }