Skip to content

Commit

Permalink
Post-adjustment: Make time zone method lookups unconditional
Browse files Browse the repository at this point in the history
This is to address a belated review comment from Richard on #2519. For
each operation in which we need to observably look up time zone methods,
make those lookups unconditional for every method that might be called.

See: #2724
  • Loading branch information
ptomato committed Nov 14, 2023
1 parent c040562 commit 8ce1f54
Show file tree
Hide file tree
Showing 12 changed files with 41 additions and 53 deletions.
21 changes: 3 additions & 18 deletions polyfill/lib/ecmascript.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1475,9 +1475,7 @@ export function InterpretISODateTimeOffset(
offsetOpt,
matchMinute
) {
// If offsetBehaviour !== "exact" and offsetOpt !== "use", at least
// getPossibleInstantsFor should be looked up in advance. timeZoneRec may be
// modified by looking up getOffsetNanosecondsFor as needed.
// getPossibleInstantsFor and getOffsetNanosecondsFor should be looked up.
const dt = CreateTemporalDateTime(
year,
month,
Expand Down Expand Up @@ -1522,7 +1520,6 @@ export function InterpretISODateTimeOffset(
// "prefer" or "reject"
const possibleInstants = GetPossibleInstantsFor(timeZoneRec, dt);
if (possibleInstants.length > 0) {
if (!timeZoneRec.hasLookedUp('getOffsetNanosecondsFor')) timeZoneRec.lookup('getOffsetNanosecondsFor');
for (let index = 0; index < possibleInstants.length; index++) {
const candidate = possibleInstants[index];
const candidateOffset = GetOffsetNanosecondsFor(timeZoneRec, candidate);
Expand All @@ -1544,7 +1541,6 @@ export function InterpretISODateTimeOffset(
}
// fall through: offsetOpt === 'prefer', but the offset doesn't match
// so fall back to use the time zone instead.
if (!timeZoneRec.hasLookedUp('getOffsetNanosecondsFor')) timeZoneRec.lookup('getOffsetNanosecondsFor');
const instant = DisambiguatePossibleInstants(possibleInstants, timeZoneRec, dt, disambiguation);
return GetSlot(instant, EPOCHNANOSECONDS);
}
Expand Down Expand Up @@ -1616,10 +1612,7 @@ export function ToTemporalZonedDateTime(item, options) {
}
let offsetNs = 0;
if (offsetBehaviour === 'option') offsetNs = ParseDateTimeUTCOffset(offset);
const timeZoneRec = new TimeZoneMethodRecord(timeZone);
if (offsetBehaviour !== 'exact' && offsetOpt !== 'use') {
timeZoneRec.lookup('getPossibleInstantsFor');
}
const timeZoneRec = new TimeZoneMethodRecord(timeZone, ['getOffsetNanosecondsFor', 'getPossibleInstantsFor']);
const epochNanoseconds = InterpretISODateTimeOffset(
year,
month,
Expand Down Expand Up @@ -2375,16 +2368,8 @@ export function GetPlainDateTimeFor(timeZoneRec, instant, calendar, precalculate
}

export function GetInstantFor(timeZoneRec, dateTime, disambiguation) {
// getPossibleInstantsFor must be looked up already.
// getOffsetNanosecondsFor _may_ be looked up and timeZoneRec may be modified.
// getPossibleInstantsFor and getOffsetNanosecondsFor must be looked up.
const possibleInstants = GetPossibleInstantsFor(timeZoneRec, dateTime);
if (
possibleInstants.length === 0 &&
disambiguation !== 'reject' &&
!timeZoneRec.hasLookedUp('getOffsetNanosecondsFor')
) {
timeZoneRec.lookup('getOffsetNanosecondsFor');
}
return DisambiguatePossibleInstants(possibleInstants, timeZoneRec, dateTime, disambiguation);
}

Expand Down
25 changes: 20 additions & 5 deletions polyfill/lib/intl.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,10 @@ function extractOverrides(temporalObj, main) {
nanosecond,
'iso8601'
);
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], ['getPossibleInstantsFor']);
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], [
'getOffsetNanosecondsFor',
'getPossibleInstantsFor'
]);
return {
instant: ES.GetInstantFor(timeZoneRec, datetime, 'compatible'),
formatter: getPropLazy(main, TIME)
Expand All @@ -383,7 +386,10 @@ function extractOverrides(temporalObj, main) {
);
}
const datetime = ES.CreateTemporalDateTime(isoYear, isoMonth, referenceISODay, 12, 0, 0, 0, 0, 0, calendar);
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], ['getPossibleInstantsFor']);
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], [
'getOffsetNanosecondsFor',
'getPossibleInstantsFor'
]);
return {
instant: ES.GetInstantFor(timeZoneRec, datetime, 'compatible'),
formatter: getPropLazy(main, YM)
Expand All @@ -401,7 +407,10 @@ function extractOverrides(temporalObj, main) {
);
}
const datetime = ES.CreateTemporalDateTime(referenceISOYear, isoMonth, isoDay, 12, 0, 0, 0, 0, 0, calendar);
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], ['getPossibleInstantsFor']);
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], [
'getOffsetNanosecondsFor',
'getPossibleInstantsFor'
]);
return {
instant: ES.GetInstantFor(timeZoneRec, datetime, 'compatible'),
formatter: getPropLazy(main, MD)
Expand All @@ -417,7 +426,10 @@ function extractOverrides(temporalObj, main) {
throw new RangeError(`cannot format PlainDate with calendar ${calendar} in locale with calendar ${main[CAL_ID]}`);
}
const datetime = ES.CreateTemporalDateTime(isoYear, isoMonth, isoDay, 12, 0, 0, 0, 0, 0, main[CAL_ID]);
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], ['getPossibleInstantsFor']);
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], [
'getOffsetNanosecondsFor',
'getPossibleInstantsFor'
]);
return {
instant: ES.GetInstantFor(timeZoneRec, datetime, 'compatible'),
formatter: getPropLazy(main, DATE)
Expand All @@ -431,7 +443,10 @@ function extractOverrides(temporalObj, main) {
`cannot format PlainDateTime with calendar ${calendar} in locale with calendar ${main[CAL_ID]}`
);
}
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], ['getPossibleInstantsFor']);
const timeZoneRec = new TimeZoneMethodRecord(main[TZ_CANONICAL], [
'getOffsetNanosecondsFor',
'getPossibleInstantsFor'
]);
return {
instant: ES.GetInstantFor(timeZoneRec, temporalObj, 'compatible'),
formatter: getPropLazy(main, DATETIME)
Expand Down
2 changes: 1 addition & 1 deletion polyfill/lib/plaindate.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ export class PlainDate {
nanosecond,
calendar
);
const timeZoneRec = new TimeZoneMethodRecord(timeZone, ['getPossibleInstantsFor']);
const timeZoneRec = new TimeZoneMethodRecord(timeZone, ['getOffsetNanosecondsFor', 'getPossibleInstantsFor']);
const instant = ES.GetInstantFor(timeZoneRec, dt, 'compatible');
return ES.CreateTemporalZonedDateTime(GetSlot(instant, EPOCHNANOSECONDS), timeZone, calendar);
}
Expand Down
2 changes: 1 addition & 1 deletion polyfill/lib/plaindatetime.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ export class PlainDateTime {
const timeZone = ES.ToTemporalTimeZoneSlotValue(temporalTimeZoneLike);
options = ES.GetOptionsObject(options);
const disambiguation = ES.ToTemporalDisambiguation(options);
const timeZoneRec = new TimeZoneMethodRecord(timeZone, ['getPossibleInstantsFor']);
const timeZoneRec = new TimeZoneMethodRecord(timeZone, ['getOffsetNanosecondsFor', 'getPossibleInstantsFor']);
const instant = ES.GetInstantFor(timeZoneRec, this, disambiguation);
return ES.CreateTemporalZonedDateTime(GetSlot(instant, EPOCHNANOSECONDS), timeZone, GetSlot(this, CALENDAR));
}
Expand Down
2 changes: 1 addition & 1 deletion polyfill/lib/plaintime.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ export class PlainTime {
nanosecond,
calendar
);
const timeZoneRec = new TimeZoneMethodRecord(timeZone, ['getPossibleInstantsFor']);
const timeZoneRec = new TimeZoneMethodRecord(timeZone, ['getOffsetNanosecondsFor', 'getPossibleInstantsFor']);
const instant = ES.GetInstantFor(timeZoneRec, dt, 'compatible');
return ES.CreateTemporalZonedDateTime(GetSlot(instant, EPOCHNANOSECONDS), timeZone, calendar);
}
Expand Down
2 changes: 1 addition & 1 deletion polyfill/lib/timezone.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class TimeZone {
dateTime = ES.ToTemporalDateTime(dateTime);
options = ES.GetOptionsObject(options);
const disambiguation = ES.ToTemporalDisambiguation(options);
const timeZoneRec = new TimeZoneMethodRecord(this, ['getPossibleInstantsFor']);
const timeZoneRec = new TimeZoneMethodRecord(this, ['getOffsetNanosecondsFor', 'getPossibleInstantsFor']);
return ES.GetInstantFor(timeZoneRec, dateTime, disambiguation);
}
getPossibleInstantsFor(dateTime) {
Expand Down
10 changes: 5 additions & 5 deletions spec/intl.html
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ <h1>HandleDateTimeTemporalDate ( _dateTimeFormat_, _temporalDate_ )</h1>
1. Let _calendar_ be ? ToTemporalCalendarIdentifier(_temporalDate_.[[Calendar]]).
1. If _calendar_ is not _dateTimeFormat_.[[Calendar]] or *"iso8601"*, throw a *RangeError* exception.
1. Let _plainDateTime_ be ? CreateTemporalDateTime(_temporalDate_.[[ISOYear]], _temporalDate_.[[ISOMonth]], _temporalDate_.[[ISODay]], 12, 0, 0, 0, 0, 0, _dateTimeFormat_.[[Calendar]]).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getPossibleInstantsFor~ »).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getOffsetNanosecondsFor~, ~getPossibleInstantsFor~ »).
1. Let _instant_ be ? GetInstantFor(_timeZoneRec_, _plainDateTime_, *"compatible"*).
1. If _pattern_ is *null*, throw a *TypeError* exception.
1. Return the Record {
Expand All @@ -884,7 +884,7 @@ <h1>HandleDateTimeTemporalYearMonth ( _dateTimeFormat_, _temporalYearMonth_ )</h
1. If _calendar_ is not equal to _dateTimeFormat_.[[Calendar]], then
1. Throw a *RangeError* exception.
1. Let _plainDateTime_ be ? CreateTemporalDateTime(_temporalYearMonth_.[[ISOYear]], _temporalYearMonth_.[[ISOMonth]], _temporalYearMonth_.[[ISODay]], 12, 0, 0, 0, 0, 0, _dateTimeFormat_.[[Calendar]]).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getPossibleInstantsFor~ »).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getOffsetNanosecondsFor~, ~getPossibleInstantsFor~ »).
1. Let _instant_ be ? GetInstantFor(_timeZoneRec_, _plainDateTime_, *"compatible"*).
1. If _pattern_ is *null*, throw a *TypeError* exception.
1. Return the Record {
Expand All @@ -911,7 +911,7 @@ <h1>HandleDateTimeTemporalMonthDay ( _dateTimeFormat_, _temporalMonthDay_ )</h1>
1. If _calendar_ is not equal to _dateTimeFormat_.[[Calendar]], then
1. Throw a *RangeError* exception.
1. Let _plainDateTime_ be ? CreateTemporalDateTime(_temporalMonthDay_.[[ISOYear]], _temporalMonthDay_.[[ISOMonth]], _temporalMonthDay_.[[ISODay]], 12, 0, 0, 0, 0, 0, _dateTimeFormat_.[[Calendar]]).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getPossibleInstantsFor~ »).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getOffsetNanosecondsFor~, ~getPossibleInstantsFor~ »).
1. Let _instant_ be ? GetInstantFor(_timeZoneRec_, _plainDateTime_, *"compatible"*).
1. If _pattern_ is *null*, throw a *TypeError* exception.
1. Return the Record {
Expand All @@ -935,7 +935,7 @@ <h1>HandleDateTimeTemporalTime ( _dateTimeFormat_, _temporalTime_ )</h1>
1. Assert: _temporalTime_ has an [[InitializedTemporalTime]] internal slot.
1. Let _pattern_ be _dateTimeFormat_.[[TemporalPlainTimePattern]].
1. Let _plainDateTime_ be ? CreateTemporalDateTime(1970, 1, 1, _temporalTime_.[[ISOHour]], _temporalTime_.[[ISOMinute]], _temporalTime_.[[ISOSecond]], _temporalTime_.[[ISOMillisecond]], _temporalTime_.[[ISOMicrosecond]], _temporalTime_.[[ISONanosecond]], *"iso8601"*).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getPossibleInstantsFor~ »).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getOffsetNanosecondsFor~, ~getPossibleInstantsFor~ »).
1. Let _instant_ be ? GetInstantFor(_timeZoneRec_, _plainDateTime_, *"compatible"*).
1. If _pattern_ is *null*, throw a *TypeError* exception.
1. Return the Record {
Expand All @@ -961,7 +961,7 @@ <h1>HandleDateTimeTemporalDateTime ( _dateTimeFormat_, _dateTime_ )</h1>
1. Let _calendar_ be ? ToTemporalCalendarIdentifier(_dateTime_.[[Calendar]]).
1. If _calendar_ is not *"iso8601"* and not equal to _dateTimeFormat_.[[Calendar]], then
1. Throw a *RangeError* exception.
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getPossibleInstantsFor~ »).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_dateTimeFormat_.[[TimeZone]], « ~getOffsetNanosecondsFor~, ~getPossibleInstantsFor~ »).
1. Let _instant_ be ? GetInstantFor(_timeZoneRec_, _dateTime_, *"compatible"*).
1. If _pattern_ is *null*, throw a *TypeError* exception.
1. Return the Record {
Expand Down
2 changes: 1 addition & 1 deletion spec/plaindate.html
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ <h1>Temporal.PlainDate.prototype.toZonedDateTime ( _item_ )</h1>
1. Else,
1. Set _temporalTime_ to ? ToTemporalTime(_temporalTime_).
1. Let _temporalDateTime_ be ? CreateTemporalDateTime(_temporalDate_.[[ISOYear]], _temporalDate_.[[ISOMonth]], _temporalDate_.[[ISODay]], _temporalTime_.[[ISOHour]], _temporalTime_.[[ISOMinute]], _temporalTime_.[[ISOSecond]], _temporalTime_.[[ISOMillisecond]], _temporalTime_.[[ISOMicrosecond]], _temporalTime_.[[ISONanosecond]], _temporalDate_.[[Calendar]]).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_timeZone_, « ~getPossibleInstantsFor~ »).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_timeZone_, « ~getOffsetNanosecondsFor~, ~getPossibleInstantsFor~ »).
1. Let _instant_ be ? GetInstantFor(_timeZoneRec_, _temporalDateTime_, *"compatible"*).
1. Return ! CreateTemporalZonedDateTime(_instant_.[[Nanoseconds]], _timeZone_, _temporalDate_.[[Calendar]]).
</emu-alg>
Expand Down
2 changes: 1 addition & 1 deletion spec/plaindatetime.html
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ <h1>Temporal.PlainDateTime.prototype.toZonedDateTime ( _temporalTimeZoneLike_ [
1. Let _timeZone_ be ? ToTemporalTimeZoneSlotValue(_temporalTimeZoneLike_).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _disambiguation_ be ? ToTemporalDisambiguation(_options_).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_timeZone_, « ~getPossibleInstantsFor~ »).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_timeZone_, « ~getOffsetNanosecondsFor~, ~getPossibleInstantsFor~ »).
1. Let _instant_ be ? GetInstantFor(_timeZoneRec_, _dateTime_, _disambiguation_).
1. Return ! CreateTemporalZonedDateTime(_instant_.[[Nanoseconds]], _timeZone_, _dateTime_.[[Calendar]]).
</emu-alg>
Expand Down
2 changes: 1 addition & 1 deletion spec/plaintime.html
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ <h1>Temporal.PlainTime.prototype.toZonedDateTime ( _item_ )</h1>
1. Throw a *TypeError* exception.
1. Let _timeZone_ be ? ToTemporalTimeZoneSlotValue(_temporalTimeZoneLike_).
1. Let _temporalDateTime_ be ? CreateTemporalDateTime(_temporalDate_.[[ISOYear]], _temporalDate_.[[ISOMonth]], _temporalDate_.[[ISODay]], _temporalTime_.[[ISOHour]], _temporalTime_.[[ISOMinute]], _temporalTime_.[[ISOSecond]], _temporalTime_.[[ISOMillisecond]], _temporalTime_.[[ISOMicrosecond]], _temporalTime_.[[ISONanosecond]], _temporalDate_.[[Calendar]]).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_timeZone_, « ~getPossibleInstantsFor~ »).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_timeZone_, « ~getOffsetNanosecondsFor~, ~getPossibleInstantsFor~ »).
1. Let _instant_ be ? GetInstantFor(_timeZoneRec_, _temporalDateTime_, *"compatible"*).
1. Return ! CreateTemporalZonedDateTime(_instant_.[[Nanoseconds]], _timeZone_, _temporalDate_.[[Calendar]]).
</emu-alg>
Expand Down
10 changes: 3 additions & 7 deletions spec/timezone.html
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ <h1>Temporal.TimeZone.prototype.getInstantFor ( _dateTime_ [ , _options_ ] )</h1
1. Set _dateTime_ to ? ToTemporalDateTime(_dateTime_).
1. Set _options_ to ? GetOptionsObject(_options_).
1. Let _disambiguation_ be ? ToTemporalDisambiguation(_options_).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_timeZone_, « ~getPossibleInstantsFor~ »).
1. Let _timeZoneRec_ be ? CreateTimeZoneMethodsRecord(_timeZone_, « ~getOffsetNanosecondsFor~, ~getPossibleInstantsFor~ »).
1. Return ? GetInstantFor(_timeZoneRec_, _dateTime_, _disambiguation_).
</emu-alg>
</emu-clause>
Expand Down Expand Up @@ -887,14 +887,10 @@ <h1>
If that call returns an empty array and _disambiguation_ is not *"reject"*, it calls _timeZoneRec_'s `getOffsetNanosecondsFor` method twice, and `getPossibleInstantsFor` an additional time.
</dd>
</dl>
<p>
_timeZoneRec_ must have looked up at least the `getPossibleInstantsFor` method.
`getOffsetNanosecondsFor` will be looked up as needed, since this is one of the few operations that cannot determine in advance whether it will be used.
</p>
<emu-alg>
1. Assert: TimeZoneMethodsRecordHasLookedUp(_timeZoneRec_, ~getOffsetNanosecondsFor~) is *true*.
1. Assert: TimeZoneMethodsRecordHasLookedUp(_timeZoneRec_, ~getPossibleInstantsFor~) is *true*.
1. Let _possibleInstants_ be ? GetPossibleInstantsFor(_timeZoneRec_, _dateTime_).
1. If _possibleInstants_ is empty, and _disambiguation_ is not *"reject"*, and TimeZoneMethodsRecordHasLookedUp(_timeZoneRec_, ~getOffsetNanosecondsFor~) is *false*, then
1. Perform ? TimeZoneMethodsRecordLookup(_timeZoneRec_, ~getOffsetNanosecondsFor~).
1. Return ? DisambiguatePossibleInstants(_possibleInstants_, _timeZoneRec_, _dateTime_, _disambiguation_).
</emu-alg>
</emu-clause>
Expand Down
Loading

0 comments on commit 8ce1f54

Please sign in to comment.