Normative: Prevent indefinite loops in NormalizedTimeDurationToDays

It's possible to make at least the second loop continue indefinitely with a contrived calendar and time zone. DRAFT: Still to be determined if this precludes any non-contrived use cases. If so, we will keep the loops, but still put an upper limit on the number of iterations. Includes a few more tests in the NYSE time zone cookbook example to make sure that a time zone transition of >24h continues to work.
tc39 · Nov 14, 2023 · 7a2ea4c · 7a2ea4c
1 parent 3e38f70
commit 7a2ea4c
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 35 deletions.
diff --git a/docs/cookbook/stockExchangeTimeZone.mjs b/docs/cookbook/stockExchangeTimeZone.mjs
@@ -222,7 +222,30 @@ assert.equal(monday.hoursInDay, 24);
 const friday = monday.add({ days: 4 });
 assert.equal(friday.hoursInDay, 72);
 
-// Adding 1 day to Friday gets you the next Monday
+// Adding 1 day to Friday gets you the next Monday (disambiguates forward)
 assert.equal(friday.add({ days: 1 }).toString(), '2022-08-29T09:30:00-04:00[NYSE]');
 // Adding 3 days to Friday also gets you the next Monday
 assert.equal(friday.add({ days: 3 }).toString(), '2022-08-29T09:30:00-04:00[NYSE]');
+
+const nextMonday = monday.add({ weeks: 1 });
+
+// Subtracting 1 day from Monday gets you the same day (disambiguates forward)
+assert.equal(nextMonday.subtract({ days: 1 }).toString(), '2022-08-29T09:30:00-04:00[NYSE]');
+// Subtracting 3 days from Monday gets you the previous Friday
+assert.equal(nextMonday.subtract({ days: 3 }).toString(), '2022-08-26T09:30:00-04:00[NYSE]');
+
+// Difference between Friday and Monday is 72 hours or 3 days
+const fridayUntilMonday = friday.until(nextMonday);
+assert.equal(fridayUntilMonday.toString(), 'PT72H');
+assert.equal(fridayUntilMonday.total('hours'), 72);
+assert.equal(fridayUntilMonday.total('days'), 3);
+
+const mondaySinceFriday = nextMonday.since(friday);
+assert.equal(mondaySinceFriday.toString(), 'PT72H');
+assert.equal(mondaySinceFriday.total('hours'), 72);
+assert.equal(mondaySinceFriday.total('days'), 3);
+
+// One week is still 7 days
+const oneWeek = Temporal.Duration.from({ weeks: 1 });
+assert.equal(oneWeek.total({ unit: 'days', relativeTo: monday }), 7);
+assert.equal(oneWeek.total({ unit: 'days', relativeTo: friday }), 7);
diff --git a/polyfill/lib/ecmascript.mjs b/polyfill/lib/ecmascript.mjs
@@ -3255,20 +3255,34 @@ export function NormalizedTimeDurationToDays(norm, zonedRelativeTo, timeZoneRec,
   // back inside the period where it belongs. Note that this case only can
   // happen for positive durations because the only direction that
   // `disambiguation: 'compatible'` can change clock time is forwards.
-  if (sign === 1) {
-    while (days > 0 && relativeResult.epochNs.greater(endNs)) {
-      days--;
-      relativeResult = AddDaysToZonedDateTime(start, dtStart, timeZoneRec, calendar, days);
-      // may do disambiguation
+  if (sign === 1 && days > 0 && relativeResult.epochNs.greater(endNs)) {
+    days--;
+    relativeResult = AddDaysToZonedDateTime(start, dtStart, timeZoneRec, calendar, days);
+    // may do disambiguation
+    if (days > 0 && relativeResult.epochNs.greater(endNs)) {
+      throw new RangeError('inconsistent result from custom time zone getInstantFor()');
     }
   }
   norm = TimeDuration.fromEpochNsDiff(endNs, relativeResult.epochNs);
 
-  let isOverflow = false;
-  let dayLengthNs;
-  do {
-    // calculate length of the next day (day that contains the time remainder)
-    const oneDayFarther = AddDaysToZonedDateTime(
+  // calculate length of the next day (day that contains the time remainder)
+  let oneDayFarther = AddDaysToZonedDateTime(
+    relativeResult.instant,
+    relativeResult.dateTime,
+    timeZoneRec,
+    calendar,
+    sign
+  );
+  let dayLengthNs = TimeDuration.fromEpochNsDiff(oneDayFarther.epochNs, relativeResult.epochNs);
+  const oneDayLess = norm.subtract(dayLengthNs);
+  let isOverflow = oneDayLess.sign() * sign >= 0;
+  if (isOverflow) {
+    norm = oneDayLess;
+    relativeResult = oneDayFarther;
+    days += sign;
+
+    // ensure there was no more overflow
+    oneDayFarther = AddDaysToZonedDateTime(
       relativeResult.instant,
       relativeResult.dateTime,
       timeZoneRec,
@@ -3277,14 +3291,9 @@ export function NormalizedTimeDurationToDays(norm, zonedRelativeTo, timeZoneRec,
     );
 
     dayLengthNs = TimeDuration.fromEpochNsDiff(oneDayFarther.epochNs, relativeResult.epochNs);
-    const oneDayLess = norm.subtract(dayLengthNs);
-    isOverflow = oneDayLess.sign() * sign >= 0;
-    if (isOverflow) {
-      norm = oneDayLess;
-      relativeResult = oneDayFarther;
-      days += sign;
-    }
-  } while (isOverflow);
+    isOverflow = norm.subtract(dayLengthNs).sign() * sign >= 0;
+    if (isOverflow) throw new RangeError('inconsistent result from custom time zone getPossibleInstantsFor()');
+  }
   if (days !== 0 && MathSign(days) != sign) {
     throw new RangeError('Time zone or calendar converted nanoseconds into a number of days with the opposite sign');
   }

diff --git a/spec/zoneddatetime.html b/spec/zoneddatetime.html
@@ -1446,23 +1446,22 @@ <h1>
         1. Else if _days_ &lt; 0 and _timeSign_ &lt; 0, then
           1. Set _days_ to _days_ + 1.
         1. Let _relativeResult_ be ? AddDaysToZonedDateTime(_startInstant_, _startDateTime_, _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _days_).
-        1. If _sign_ is 1, then
-          1. Repeat, while _days_ &gt; 0 and ℝ(_relativeResult_.[[EpochNanoseconds]]) &gt; _endNs_,
-            1. Set _days_ to _days_ - 1.
-            1. Set _relativeResult_ to ? AddDaysToZonedDateTime(_startInstant_, _startDateTime_, _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _days_).
+        1. If _sign_ = 1, and _days_ &gt; 0, and ℝ(_relativeResult_.[[EpochNanoseconds]]) &gt; _endNs_, then
+          1. Set _days_ to _days_ - 1.
+          1. Set _relativeResult_ to ? AddDaysToZonedDateTime(_startInstant_, _startDateTime_, _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _days_).
+          1. If _days_ &gt; 0 and ℝ(_relativeResult_.[[EpochNanoseconds]]) &gt; _endNs_, throw a *RangeError* exception.
         1. Set _norm_ to NormalizedTimeDurationFromEpochNanosecondsDifference(_endNs_, _relativeResult_.[[EpochNanoseconds]]).
-        1. Let _done_ be *false*.
-        1. Let _dayLengthNs_ be ~unset~.
-        1. Repeat, while _done_ is *false*,
-          1. Let _oneDayFarther_ be ? AddDaysToZonedDateTime(_relativeResult_.[[Instant]], _relativeResult_.[[DateTime]], _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _sign_).
-          1. Set _dayLengthNs_ to NormalizedTimeDurationFromEpochNanosecondsDifference(_oneDayFarther_.[[EpochNanoseconds]], _relativeResult_.[[EpochNanoseconds]]).
-          1. Let _oneDayLess_ be ? SubtractNormalizedTimeDuration(_norm_, _dayLengthNs_).
-          1. If NormalizedTimeDurationSign(_oneDayLess_) &times; _sign_ &ge; 0, then
-            1. Set _norm_ to _oneDayLess_.
-            1. Set _relativeResult_ to _oneDayFarther_.
-            1. Set _days_ to _days_ + _sign_.
-          1. Else,
-            1. Set _done_ to *true*.
+        1. Let _oneDayFarther_ be ? AddDaysToZonedDateTime(_relativeResult_.[[Instant]], _relativeResult_.[[DateTime]], _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _sign_).
+        1. Let _dayLengthNs_ be NormalizedTimeDurationFromEpochNanosecondsDifference(_oneDayFarther.[[EpochNanoseconds]], _relativeResult_.[[EpochNanoseconds]]).
+        1. Let _oneDayLess_ be ? SubtractNormalizedTimeDuration(_norm_, _dayLengthNs_).
+        1. If NormalizedTimeDurationSign(_oneDayLess_) &times; _sign_ &ge; 0, then
+          1. Set _norm_ to _oneDayLess_.
+          1. Set _relativeResult_ to _oneDayFarther_.
+          1. Set _days_ to _days_ + _sign_.
+          1. Set _oneDayFarther_ to ? AddDaysToZonedDateTime(_relativeResult_.[[Instant]], _relativeResult_.[[DateTime]], _timeZoneRec_, _zonedRelativeTo_.[[Calendar]], _sign_).
+          1. Set _dayLengthNs_ to NormalizedTimeDurationFromEpochNanosecondsDifference(_oneDayFarther.[[EpochNanoseconds]], _relativeResult_.[[EpochNanoseconds]]).
+          1. If NormalizedTimeDurationSign(? SubtractNormalizedTimeDuration(_norm_, _dayLengthNs_)) &times; _sign_ &ge; 0, then
+            1. Throw a *RangeError* exception.
         1. If _days_ &lt; 0 and _sign_ = 1, throw a *RangeError* exception.
         1. If _days_ &gt; 0 and _sign_ = -1, throw a *RangeError* exception.
         1. If NormalizedTimeDurationSign(_norm_) = -1, then